Module Name:    src
Committed By:   reinoud
Date:           Thu Jun 25 17:16:33 UTC 2009

Modified Files:
        src/sys/fs/udf: udf_subr.c udf_subr.h udf_vnops.c

Log Message:
Rewrite of udf_on_rootpath(), and vop_rename() code that calls it, after the
UFS way. The tree walking is now done the same and the code hasn't locked up
on examples that made it lockup before.


To generate a diff of this commit:
cvs rdiff -u -r1.95 -r1.96 src/sys/fs/udf/udf_subr.c
cvs rdiff -u -r1.15 -r1.16 src/sys/fs/udf/udf_subr.h
cvs rdiff -u -r1.46 -r1.47 src/sys/fs/udf/udf_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/fs/udf/udf_subr.c
diff -u src/sys/fs/udf/udf_subr.c:1.95 src/sys/fs/udf/udf_subr.c:1.96
--- src/sys/fs/udf/udf_subr.c:1.95	Wed Jun 24 17:09:13 2009
+++ src/sys/fs/udf/udf_subr.c	Thu Jun 25 17:16:33 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: udf_subr.c,v 1.95 2009/06/24 17:09:13 reinoud Exp $ */
+/* $NetBSD: udf_subr.c,v 1.96 2009/06/25 17:16:33 reinoud Exp $ */
 
 /*
  * Copyright (c) 2006, 2008 Reinoud Zandijk
@@ -29,7 +29,7 @@
 
 #include <sys/cdefs.h>
 #ifndef lint
-__KERNEL_RCSID(0, "$NetBSD: udf_subr.c,v 1.95 2009/06/24 17:09:13 reinoud Exp $");
+__KERNEL_RCSID(0, "$NetBSD: udf_subr.c,v 1.96 2009/06/25 17:16:33 reinoud Exp $");
 #endif /* not lint */
 
 
@@ -3373,6 +3373,14 @@
 }
 
 
+int
+udf_check_icb_equal(struct long_ad *a, struct long_ad *b)
+{
+	return (a->loc.lb_num   == b->loc.lb_num &&
+	    a->loc.part_num == b->loc.part_num);
+}
+
+
 static struct udf_node *
 udf_hash_lookup(struct udf_mount *ump, struct long_ad *icbptr)
 {
@@ -3386,8 +3394,7 @@
 	hashline = udf_calchash(icbptr) & UDF_INODE_HASHMASK;
 	LIST_FOREACH(node, &ump->udf_nodes[hashline], hashchain) {
 		assert(node);
-		if (node->loc.loc.lb_num   == icbptr->loc.lb_num &&
-		    node->loc.loc.part_num == icbptr->loc.part_num) {
+		if (udf_check_icb_equal(&node->loc, icbptr)) {
 			vp = node->vnode;
 			assert(vp);
 			mutex_enter(&vp->v_interlock);

Index: src/sys/fs/udf/udf_subr.h
diff -u src/sys/fs/udf/udf_subr.h:1.15 src/sys/fs/udf/udf_subr.h:1.16
--- src/sys/fs/udf/udf_subr.h:1.15	Wed Jun 24 17:09:13 2009
+++ src/sys/fs/udf/udf_subr.h	Thu Jun 25 17:16:33 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: udf_subr.h,v 1.15 2009/06/24 17:09:13 reinoud Exp $ */
+/* $NetBSD: udf_subr.h,v 1.16 2009/06/25 17:16:33 reinoud Exp $ */
 
 /*
  * Copyright (c) 2006, 2008 Reinoud Zandijk
@@ -183,6 +183,7 @@
 
 /* helpers and converters */
 long udf_calchash(struct long_ad *icbptr);    /* for `inode' numbering */
+int udf_check_icb_equal(struct long_ad *a, struct long_ad *b);
 uint32_t udf_getaccessmode(struct udf_node *node);
 void udf_setaccessmode(struct udf_node *udf_node, mode_t mode);
 void udf_getownership(struct udf_node *udf_node, uid_t *uidp, gid_t *gidp);

Index: src/sys/fs/udf/udf_vnops.c
diff -u src/sys/fs/udf/udf_vnops.c:1.46 src/sys/fs/udf/udf_vnops.c:1.47
--- src/sys/fs/udf/udf_vnops.c:1.46	Wed Jun 24 17:09:13 2009
+++ src/sys/fs/udf/udf_vnops.c	Thu Jun 25 17:16:33 2009
@@ -1,4 +1,4 @@
-/* $NetBSD: udf_vnops.c,v 1.46 2009/06/24 17:09:13 reinoud Exp $ */
+/* $NetBSD: udf_vnops.c,v 1.47 2009/06/25 17:16:33 reinoud Exp $ */
 
 /*
  * Copyright (c) 2006, 2008 Reinoud Zandijk
@@ -32,7 +32,7 @@
 
 #include <sys/cdefs.h>
 #ifndef lint
-__KERNEL_RCSID(0, "$NetBSD: udf_vnops.c,v 1.46 2009/06/24 17:09:13 reinoud Exp $");
+__KERNEL_RCSID(0, "$NetBSD: udf_vnops.c,v 1.47 2009/06/25 17:16:33 reinoud Exp $");
 #endif /* not lint */
 
 
@@ -1825,55 +1825,98 @@
 
 /* --------------------------------------------------------------------- */
 
+/*
+ * Check if source directory is in the path of the target directory.  Target
+ * is supplied locked, source is unlocked. The target is always vput before
+ * returning. Modeled after UFS.
+ *
+ * If source is on the path from target to the root, return error.
+ */
+
 static int
-udf_on_rootpath(struct udf_node *fnode, struct udf_node *tdnode)
+udf_on_rootpath(struct udf_node *source, struct udf_node *target)
 {
-	struct udf_mount *ump = tdnode->ump;
+	struct udf_mount *ump = target->ump;
 	struct udf_node *res_node;
-	struct long_ad icb_loc;
+	struct long_ad icb_loc, *root_icb;
 	const char *name;
 	int namelen;
 	int error, found;
 
-	/* if fnode is on the path from tdnode to the root, return error */
-	name    = "..";
-	namelen = 2;
-	while (fnode != tdnode) {
-		DPRINTF(NODE, ("udf_on_rootpath : fnode %p, tdnode %p\n",
-			fnode, tdnode));
-		if (tdnode->vnode->v_vflag & VV_ROOT) {
-			/* found root, accept */
-			/* DPRINTF(NODE, ("\tCOUGHT, pre-vput\n")); */
-			vput(tdnode->vnode);
-			DPRINTF(NODE, ("\tCOUGHT: valid\n"));
-			return 0;
+	name     = "..";
+	namelen  = 2;
+	error    = 0;
+	res_node = target;
+
+	root_icb   = &ump->fileset_desc->rootdir_icb;
+
+	/* if nodes are equal, it is no use looking */
+	if (udf_check_icb_equal(&source->loc, &target->loc)) {
+		error = EEXIST;
+		goto out;
+	}
+
+	/* nothing can exist before the root */
+	if (udf_check_icb_equal(root_icb, &target->loc)) {
+		error = 0;
+		goto out;
+	}
+
+	for (;;) {
+		DPRINTF(NODE, ("udf_on_rootpath : "
+			"source vp %p, looking at vp %p\n",
+			source->vnode, res_node->vnode));
+
+		/* sanity check */
+		if (res_node->vnode->v_type != VDIR) {
+			error = ENOTDIR;
+			goto out;
 		}
+
 		/* go down one level */
-		error = udf_lookup_name_in_dir(tdnode->vnode, name, namelen,
+		error = udf_lookup_name_in_dir(res_node->vnode, name, namelen,
 			&icb_loc, &found);
-
 		DPRINTF(NODE, ("\tlookup of '..' resulted in error %d, "
 			"found %d\n", error, found));
-		/* DPRINTF(NODE, ("\tvput %p\n", tdnode->vnode)); */
-		vput(tdnode->vnode);
 
-		if (error)	/* now what? bail out */
-			return EINVAL;
-		if (!found)	/* unlikely */
-			return EINVAL;
+		if (!found)
+			error = ENOENT;
+		if (error)
+			goto out;
 
-		DPRINTF(NODE, ("\tgetting .. node\n"));
+		/* did we encounter source node? */
+		if (udf_check_icb_equal(&icb_loc, &source->loc)) {
+			error = EINVAL;
+			goto out;
+		}
+
+		/* did we encounter the root node? */
+		if (udf_check_icb_equal(&icb_loc, root_icb)) {
+			error = 0;
+			goto out;
+		}
+
+		/* push our intermediate node, we're done with it */
+		/* DPRINTF(NODE, ("\tvput %p\n", target->vnode)); */
+		vput(res_node->vnode);
+
+		DPRINTF(NODE, ("\tgetting the .. node\n"));
 		error = udf_get_node(ump, &icb_loc, &res_node);
-		DPRINTF(NODE, ("\treported error %d\n", error));
-		if (error)	/* argh, bail out */
-			return EINVAL;
 
-		tdnode = res_node;
+		if (error) {	/* argh, bail out */
+			KASSERT(res_node == NULL);
+			// res_node = NULL;
+			goto out;
+		}
 	}
-	DPRINTF(NODE, ("\tCOUGHT: invalid\n"));
-	vput(tdnode->vnode);
+out:
+	DPRINTF(NODE, ("\tresult: %svalid, error = %d\n", error?"in":"", error));
 
-	return EINVAL;
+	/* put our last node */
+	if (res_node)
+		vput(res_node->vnode);
+
+	return error;
 }
 
 /* note: i tried to follow the logics of the tmpfs rename code */
@@ -1950,15 +1993,38 @@
 
 	/* check if moving a directory to a new parent is allowed */
 	if ((fdnode != tdnode) && (fvp->v_type == VDIR)) {
+		/* release tvp since we might encounter it and lock up */
+		if (tvp)
+			vput(tvp);
+
+		/* vref tdvp since we lose its ref in udf_on_rootpath */
 		vref(tdvp);
+
+		/* search if fnode is a component of tdnode's path to root */
 		error = udf_on_rootpath(fnode, tdnode);
+
 		DPRINTF(NODE, ("Dir rename allowed ? %s\n", error ? "NO":"YES"));
 
-		/* tdnode is vput()'ed, relock */
-		(void) vn_lock(tdvp, LK_EXCLUSIVE | LK_RETRY);
+		if (error) {
+			/* compensate for our vref earlier */
+			vrele(tdvp);
+			goto out;
+		}
+
+		/* relock tdvp; its still here due to the vref earlier */
+		vn_lock(tdvp, LK_EXCLUSIVE | LK_RETRY);
 
-		if (error)
+		/*
+		 * re-lookup tvp since the parent has been unlocked, so could
+		 * have changed/removed in the meantime.
+		 */
+		tcnp->cn_flags &= ~SAVESTART;
+		error = relookup(tdvp, &tvp, tcnp);
+		if (error) {
+			vput(tdvp);
 			goto out;
+		}
+		tnode  = (tvp == NULL) ? NULL : VTOI(tvp);
 	}
 
 	/* remove existing entry if present */

Reply via email to