kernel - add cache_unlink(), fix a rename issue.
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 24 Aug 2012 23:26:31 +0000 (16:26 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 24 Aug 2012 23:26:31 +0000 (16:26 -0700)
* Add a rollup function called cache_unlink() to handle namecache
  effects when unlinking a file.

* Change namecache semantics a bit.  When a namecache entry is unlinked
  we allow it to be left in the topology as long as the vnode survives,
  but marked NCF_DESTROYED so it does not conflict with any new entries
  that might be named the same, and does not get returned in lookup results.

* This will solve the issue when renaming a file over an empty directory
  (destroying the directory) when one or more processes are chdir'd into
  that directory.  This would cause the process nchdir stuff to get out
  of sync with the retained directory vnode because the same namecache
  entry would get reused.

sys/kern/vfs_cache.c
sys/kern/vfs_default.c
sys/kern/vfs_nlookup.c
sys/sys/namecache.h

index 19031fc..bbd46e8 100644 (file)
@@ -186,16 +186,17 @@ static void _cache_setunresolved(struct namecache *ncp);
 static void _cache_cleanneg(int count);
 static void _cache_cleanpos(int count);
 static void _cache_cleandefered(void);
+static void _cache_unlink(struct namecache *ncp);
 
 /*
  * The new name cache statistics
  */
 SYSCTL_NODE(_vfs, OID_AUTO, cache, CTLFLAG_RW, 0, "Name cache statistics");
 static int numneg;
-SYSCTL_ULONG(_vfs_cache, OID_AUTO, numneg, CTLFLAG_RD, &numneg, 0,
+SYSCTL_INT(_vfs_cache, OID_AUTO, numneg, CTLFLAG_RD, &numneg, 0,
     "Number of negative namecache entries");
 static int numcache;
-SYSCTL_ULONG(_vfs_cache, OID_AUTO, numcache, CTLFLAG_RD, &numcache, 0,
+SYSCTL_INT(_vfs_cache, OID_AUTO, numcache, CTLFLAG_RD, &numcache, 0,
     "Number of namecaches entries");
 static u_long numcalls;
 SYSCTL_ULONG(_vfs_cache, OID_AUTO, numcalls, CTLFLAG_RD, &numcalls, 0,
@@ -1372,14 +1373,26 @@ cache_rename(struct nchandle *fnch, struct nchandle *tnch)
        struct nchash_head *nchpp;
        u_int32_t hash;
        char *oname;
+       char *nname;
+
+       if (tncp->nc_nlen) {
+               nname = kmalloc(tncp->nc_nlen + 1, M_VFSCACHE, M_WAITOK);
+               bcopy(tncp->nc_name, nname, tncp->nc_nlen);
+               nname[tncp->nc_nlen] = 0;
+       } else {
+               nname = NULL;
+       }
 
        /*
         * Rename fncp (unlink)
         */
        _cache_unlink_parent(fncp);
        oname = fncp->nc_name;
-       fncp->nc_name = tncp->nc_name;
+       fncp->nc_name = nname;
        fncp->nc_nlen = tncp->nc_nlen;
+       if (oname)
+               kfree(oname, M_VFSCACHE);
+
        tncp_par = tncp->nc_parent;
        _cache_hold(tncp_par);
        _cache_lock(tncp_par);
@@ -1400,13 +1413,24 @@ cache_rename(struct nchandle *fnch, struct nchandle *tnch)
        /*
         * Get rid of the overwritten tncp (unlink)
         */
-       _cache_setunresolved(tncp);
-       _cache_unlink_parent(tncp);
-       tncp->nc_name = NULL;
-       tncp->nc_nlen = 0;
+       _cache_unlink(tncp);
+}
 
-       if (oname)
-               kfree(oname, M_VFSCACHE);
+/*
+ * Perform actions consistent with unlinking a file.  The namecache
+ * entry is marked DESTROYED so it no longer shows up in searches,
+ * and will be physically deleted when the vnode goes away.
+ */
+void
+cache_unlink(struct nchandle *nch)
+{
+       _cache_unlink(nch->ncp);
+}
+
+static void
+_cache_unlink(struct namecache *ncp)
+{
+       ncp->nc_flag |= NCF_DESTROYED;
 }
 
 /*
index 27c9e60..6b12932 100644 (file)
@@ -840,11 +840,8 @@ vop_compat_nremove(struct vop_nremove_args *ap)
        if (error == 0) {
                KKASSERT((cnp.cn_flags & CNP_PDIRUNLOCK) == 0);
                error = VOP_OLD_REMOVE(dvp, vp, &cnp);
-               if (error == 0) {
-                       cache_setunresolved(nch);
-                       cache_setvp(nch, NULL);
-                       cache_inval_vp(vp, CINV_DESTROY);
-               }
+               if (error == 0)
+                       cache_unlink(nch);
        }
        if (vp) {
                if (dvp == vp)
index e42d06a..d770503 100644 (file)
@@ -556,13 +556,17 @@ nlookup(struct nlookupdata *nd)
        } else {
            /*
             * Must unlock nl_nch when traversing down the path.
+            *
+            * If we race an unlink or rename the ncp might be marked
+            * DESTROYED after resolution, requiring a retry.
             */
            cache_unlock(&nd->nl_nch);
            nd->nl_flags &= ~NLC_NCPISLOCKED;
            nch = cache_nlookup(&nd->nl_nch, &nlc);
            if (nch.ncp->nc_flag & NCF_UNRESOLVED)
                hit = 0;
-           while ((error = cache_resolve(&nch, nd->nl_cred)) == EAGAIN) {
+           while ((error = cache_resolve(&nch, nd->nl_cred)) == EAGAIN ||
+               (nch.ncp->nc_flag & NCF_DESTROYED)) {
                kprintf("[diagnostic] nlookup: relookup %*.*s\n", 
                        nch.ncp->nc_nlen, nch.ncp->nc_nlen, nch.ncp->nc_name);
                cache_put(&nch);
index 4078c5e..8fb09dc 100644 (file)
@@ -211,6 +211,7 @@ void        cache_put(struct nchandle *nch);
 void   cache_drop(struct nchandle *nch);
 void   cache_zero(struct nchandle *nch);
 void   cache_rename(struct nchandle *fnch, struct nchandle *tnch);
+void   cache_unlink(struct nchandle *nch);
 int    cache_vget(struct nchandle *, struct ucred *, int, struct vnode **);
 int    cache_vref(struct nchandle *, struct ucred *, struct vnode **);
 int    cache_fromdvp(struct vnode *, struct ucred *, int, struct nchandle *);