rename patch (was Re: renaming the current working directory (cwd))

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

    Here is my proposed patch.  Please try it out.  It needs some testing so
    I'll commit it in a week or so.  The HAMMER bit may wind up getting
    committed sooner but I think it will be compatible with the original
    namecache code (just not as optimal).

    The history of cache_rename() is rather interesting.  Prior to
    October 2004 I was embedding nc_name in struct namecache by allocating
    a variable-length structure.  In vfs_cache.c/1.34 I changed nc_name
    to be a separately allocated string.  The original embedding made it
    impossible for cache_rename() to actually move the namecache structure
    within the topology in response to a rename so it copied it instead,
    which led to this problem.  But now that nc_name is separately allocated
    I can relocate the namecache structure in the topology and assign the
    target name to it without copying.

						-Matt

Index: kern/vfs_cache.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_cache.c,v
retrieving revision 1.85
diff -u -p -r1.85 vfs_cache.c
--- kern/vfs_cache.c	2 Nov 2007 19:52:25 -0000	1.85
+++ kern/vfs_cache.c	12 Jan 2008 22:01:46 -0000
@@ -975,48 +975,35 @@ }
 
 /*
  * The source ncp has been renamed to the target ncp.  Both fncp and tncp
- * must be locked.  Both will be set to unresolved, any children of tncp
- * will be disconnected (the prior contents of the target is assumed to be
- * destroyed by the rename operation, e.g. renaming over an empty directory),
- * and all children of fncp will be moved to tncp.
- *
- * XXX the disconnection could pose a problem, check code paths to make
- * sure any code that blocks can handle the parent being changed out from
- * under it.  Maybe we should lock the children (watch out for deadlocks) ?
+ * must be locked.  The target ncp is destroyed (as a normal rename-over
+ * would destroy the target file or directory).
  *
- * After we return the caller has the option of calling cache_setvp() if
- * the vnode of the new target ncp is known.
- *
- * Any process CD'd into any of the children will no longer be able to ".."
- * back out.  An rm -rf can cause this situation to occur.
+ * Because there may be references to the source ncp we cannot copy its
+ * contents to the target.  Instead the source ncp is relinked as the target
+ * and the target ncp is removed from the namecache topology.
  */
 void
 cache_rename(struct nchandle *fnch, struct nchandle *tnch)
 {
 	struct namecache *fncp = fnch->ncp;
 	struct namecache *tncp = tnch->ncp;
-	struct namecache *scan;
-	int didwarn = 0;
+	char *oname;
 
-	_cache_setunresolved(fncp);
 	_cache_setunresolved(tncp);
-	while (_cache_inval(tncp, CINV_CHILDREN) != 0) {
-		if (didwarn++ % 10 == 0) {
-			kprintf("Warning: cache_rename: race during "
-				"rename %s->%s\n",
-				fncp->nc_name, tncp->nc_name);
-		}
-		tsleep(tncp, 0, "mvrace", hz / 10);
-		_cache_setunresolved(tncp);
-	}
-	while ((scan = TAILQ_FIRST(&fncp->nc_list)) != NULL) {
-		_cache_hold(scan);
-		cache_unlink_parent(scan);
-		cache_link_parent(scan, tncp);
-		if (scan->nc_flag & NCF_HASHED)
-			_cache_rehash(scan);
-		_cache_drop(scan);
-	}
+	cache_unlink_parent(fncp);
+	cache_link_parent(fncp, tncp->nc_parent);
+	cache_unlink_parent(tncp);
+	oname = fncp->nc_name;
+	fncp->nc_name = tncp->nc_name;
+	fncp->nc_nlen = tncp->nc_nlen;
+	tncp->nc_name = NULL;
+	tncp->nc_nlen = 0;
+	if (fncp->nc_flag & NCF_HASHED)
+		_cache_rehash(fncp);
+	if (tncp->nc_flag & NCF_HASHED)
+		_cache_rehash(tncp);
+	if (oname)
+		kfree(oname, M_VFSCACHE);
 }
 
 /*
Index: kern/vfs_default.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_default.c,v
retrieving revision 1.51
diff -u -p -r1.51 vfs_default.c
--- kern/vfs_default.c	28 Aug 2007 01:04:32 -0000	1.51
+++ kern/vfs_default.c	12 Jan 2008 21:51:36 -0000
@@ -1056,10 +1056,8 @@ 		 */
 		KKASSERT(tvp == NULL);
 		KKASSERT((tcnp.cn_flags & CNP_PDIRUNLOCK) == 0);
 		error = VOP_OLD_RENAME(fdvp, fvp, &fcnp, tdvp, tvp, &tcnp);
-		if (error == 0) {
+		if (error == 0)
 			cache_rename(fnch, tnch);
-			cache_setvp(tnch, fvp);
-		}
 	} else if (error == 0) {
 		/*
 		 * Target exists.  VOP_OLD_RENAME should correctly delete the
@@ -1067,10 +1065,8 @@ 		 * target.
 		 */
 		KKASSERT((tcnp.cn_flags & CNP_PDIRUNLOCK) == 0);
 		error = VOP_OLD_RENAME(fdvp, fvp, &fcnp, tdvp, tvp, &tcnp);
-		if (error == 0) {
+		if (error == 0)
 			cache_rename(fnch, tnch);
-			cache_setvp(tnch, fvp);
-		}
 	} else {
 		vrele(fdvp);
 		vrele(fvp);
Index: vfs/hammer/hammer_vnops.c
===================================================================
RCS file: /cvs/src/sys/vfs/hammer/hammer_vnops.c,v
retrieving revision 1.18
diff -u -p -r1.18 hammer_vnops.c
--- vfs/hammer/hammer_vnops.c	11 Jan 2008 01:41:34 -0000	1.18
+++ vfs/hammer/hammer_vnops.c	12 Jan 2008 21:52:37 -0000
@@ -1186,10 +1186,8 @@ 	if (error)
 		goto done;
 	error = hammer_ip_del_directory(&trans, &cursor, fdip, ip);
 
-	if (error == 0) {
+	if (error == 0)
 		cache_rename(ap->a_fnch, ap->a_tnch);
-		cache_setvp(ap->a_tnch, ip->vp);
-	}
 done:
         hammer_done_cursor(&cursor);
 failed: