rename patch (was Re: renaming the current working directory (cwd))
Matthew Dillon
dillon at apollo.backplane.com
Sat Jan 12 14:22:37 PST 2008
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:
More information about the Users
mailing list