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:

Reply via email to