From e09206bac2e4f398dbb398726bdd6023ebeb9cc3 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Thu, 18 Nov 2004 20:04:28 +0000 Subject: [PATCH] Cleanup some dangling issues with cache_inval(). A lot of hard work went into guarenteeing that the namecache topology would remain connected, but there were two cases (basically rmdir and rename-over-empty-target-dir) which disconnected a portion of the hierarchy. This fixes the remaining cases by having cache_inval() simply mark the namecache entry as destroyed without actually disconnecting it from the topology. The flag tells cache_nlookup() and ".." handlers that a node has been destroyed and is no longer connected to any parent directory. The new cache_inval() also now has the ability to mark an entire subhierarchy as being unresolved, which can be a useful feature to have. In-discussion-with: Richard Nyberg --- sys/kern/vfs_cache.c | 121 +++++++++++++++++++++++--------------- sys/kern/vfs_default.c | 4 +- sys/kern/vfs_nlookup.c | 8 ++- sys/kern/vfs_subr.c | 4 +- sys/sys/namecache.h | 9 +-- sys/vfs/nfs/nfs_nqlease.c | 4 +- sys/vfs/nfs/nfs_socket.c | 4 +- 7 files changed, 95 insertions(+), 59 deletions(-) diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index 844bdee3d1..3c7ff791b5 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -67,7 +67,7 @@ * * @(#)vfs_cache.c 8.5 (Berkeley) 3/22/95 * $FreeBSD: src/sys/kern/vfs_cache.c,v 1.42.2.6 2001/10/05 20:07:03 dillon Exp $ - * $DragonFly: src/sys/kern/vfs_cache.c,v 1.42 2004/11/12 00:09:24 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_cache.c,v 1.43 2004/11/18 20:04:24 dillon Exp $ */ #include @@ -565,13 +565,33 @@ cache_setunresolved(struct namecache *ncp) } /* - * Invalidate portions of a namecache entry. The passed ncp should be - * referenced and locked but we might not adhere to that rule during the - * old api -> new api transition period. + * Invalidate portions of the namecache topology given a starting entry. + * The passed ncp is set to an unresolved state and: * - * CINV_PARENT - disconnect the ncp from its parent - * CINV_SELF - same as cache_setunresolved(ncp) - * CINV_CHILDREN - disconnect children of the ncp from the ncp + * The passed ncp must be locked. + * + * CINV_DESTROY - Set a flag in the passed ncp entry indicating + * that the physical underlying nodes have been + * destroyed... as in deleted. For example, when + * a directory is removed. This will cause record + * lookups on the name to no longer be able to find + * the record and tells the resolver to return failure + * rather then trying to resolve through the parent. + * + * The topology itself, including ncp->nc_name, + * remains intact. + * + * This only applies to the passed ncp, if CINV_CHILDREN + * is specified the children are not flagged. + * + * CINV_CHILDREN - Set all children (recursively) to an unresolved + * state as well. + * + * Note that this will also have the side effect of + * cleaning out any unreferenced nodes in the topology + * from the leaves up as the recursion backs out. + * + * Note that the topology for any referenced nodes remains intact. */ void cache_inval(struct namecache *ncp, int flags) @@ -579,59 +599,53 @@ cache_inval(struct namecache *ncp, int flags) struct namecache *kid; struct namecache *nextkid; - if (flags & CINV_SELF) - cache_setunresolved(ncp); - if (flags & CINV_PARENT) - cache_unlink_parent(ncp); + KKASSERT(ncp->nc_exlocks); +again: + cache_setunresolved(ncp); + if (flags & CINV_DESTROY) + ncp->nc_flag |= NCF_DESTROYED; - /* - * Children are invalidated when the parent is destroyed. This - * basically disconnects the children from the parent. Anyone - * CD'd into a child will no longer be able to ".." back up. - * - * Any unresolved or negative cache-hit children with a ref count - * of 0 must be immediately and recursively destroyed or this - * disconnection may leave them dangling forever. XXX this recursion - * could run the kernel out of stack, the children should be placed - * on a to-destroy list instead. - */ - if (flags & CINV_CHILDREN) { - if ((kid = TAILQ_FIRST(&ncp->nc_list)) != NULL) - cache_hold(kid); + if ((flags & CINV_CHILDREN) && + (kid = TAILQ_FIRST(&ncp->nc_list)) != NULL + ) { + cache_hold(kid); + cache_unlock(ncp); while (kid) { if ((nextkid = TAILQ_NEXT(kid, nc_entry)) != NULL) cache_hold(nextkid); - if (kid->nc_refs == 0 && - ((kid->nc_flag & NCF_UNRESOLVED) || - kid->nc_vp == NULL) + if ((kid->nc_flag & NCF_UNRESOLVED) == 0 || + TAILQ_FIRST(&kid->nc_list) ) { - cache_inval(kid, CINV_PARENT); + cache_lock(kid); + cache_inval(kid, flags & ~CINV_DESTROY); + cache_unlock(kid); } - cache_unlink_parent(kid); cache_drop(kid); kid = nextkid; } + cache_lock(ncp); + + /* + * Someone could have gotten in there while ncp was unlocked, + * retry if so. + */ + if ((ncp->nc_flag & NCF_UNRESOLVED) == 0) + goto again; } } +/* + * Invalidate a vnode's namecache associations. + */ void cache_inval_vp(struct vnode *vp, int flags) { struct namecache *ncp; - if (flags & CINV_SELF) { - while ((ncp = TAILQ_FIRST(&vp->v_namecache)) != NULL) { - cache_hold(ncp); - KKASSERT((ncp->nc_flag & NCF_UNRESOLVED) == 0); - cache_inval(ncp, flags); - cache_drop(ncp); - } - } else { - TAILQ_FOREACH(ncp, &vp->v_namecache, nc_vnode) { - cache_hold(ncp); - cache_inval(ncp, flags); - cache_drop(ncp); - } + while ((ncp = TAILQ_FIRST(&vp->v_namecache)) != NULL) { + cache_get(ncp); + cache_inval(ncp, flags); + cache_put(ncp); } } @@ -642,6 +656,10 @@ cache_inval_vp(struct vnode *vp, int flags) * 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) ? + * * After we return the caller has the option of calling cache_setvp() if * the vnode of the new target ncp is known. * @@ -1185,11 +1203,13 @@ restart: /* * Break out if we find a matching entry. Note that - * UNRESOLVED entries may match. + * UNRESOLVED entries may match, but DESTROYED entries + * do not. */ if (ncp->nc_parent == par && ncp->nc_nlen == nlc->nlc_namelen && - bcmp(ncp->nc_name, nlc->nlc_nameptr, ncp->nc_nlen) == 0 + bcmp(ncp->nc_name, nlc->nlc_nameptr, ncp->nc_nlen) == 0 && + (ncp->nc_flag & NCF_DESTROYED) == 0 ) { if (cache_get_nonblock(ncp) == 0) { if (new_ncp) @@ -1300,6 +1320,7 @@ restart: * - due to NFS being stupid about tracking the namespace and * destroys the namespace for entire directories quite often. * - due to forced unmounts. + * - due to an rmdir (parent will be marked DESTROYED) * * When this occurs we have to track the chain backwards and resolve * it, looping until the resolver catches up to the current node. We @@ -1309,6 +1330,14 @@ restart: * many nodes to resolve the ncp. */ while (ncp->nc_parent->nc_vp == NULL) { + /* + * This case can occur if a process is CD'd into a + * directory which is then rmdir'd. If the parent is marked + * destroyed there is no point trying to resolve it. + */ + if (ncp->nc_parent->nc_flag & NCF_DESTROYED) + return(ENOENT); + par = ncp->nc_parent; while (par->nc_parent && par->nc_parent->nc_vp == NULL) par = par->nc_parent; @@ -1549,7 +1578,7 @@ cache_purge(struct vnode *vp) { static u_long nextid; - cache_inval_vp(vp, CINV_PARENT | CINV_SELF | CINV_CHILDREN); + cache_inval_vp(vp, CINV_DESTROY | CINV_CHILDREN); /* * Calculate a new unique id for ".." handling diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c index 98f677e7eb..f98d48eca8 100644 --- a/sys/kern/vfs_default.c +++ b/sys/kern/vfs_default.c @@ -37,7 +37,7 @@ * * * $FreeBSD: src/sys/kern/vfs_default.c,v 1.28.2.7 2003/01/10 18:23:26 bde Exp $ - * $DragonFly: src/sys/kern/vfs_default.c,v 1.22 2004/11/12 00:09:24 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_default.c,v 1.23 2004/11/18 20:04:24 dillon Exp $ */ #include @@ -959,7 +959,7 @@ vop_compat_nrmdir(struct vop_nrmdir_args *ap) * back out. */ if (error == 0) - cache_inval(ncp, CINV_SELF|CINV_PARENT); + cache_inval(ncp, CINV_DESTROY); } if (vp) { if (dvp == vp) diff --git a/sys/kern/vfs_nlookup.c b/sys/kern/vfs_nlookup.c index 25c858ce86..b74e4420fd 100644 --- a/sys/kern/vfs_nlookup.c +++ b/sys/kern/vfs_nlookup.c @@ -31,7 +31,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/kern/vfs_nlookup.c,v 1.7 2004/11/12 00:09:24 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_nlookup.c,v 1.8 2004/11/18 20:04:24 dillon Exp $ */ /* * nlookup() is the 'new' namei interface. Rather then return directory and @@ -357,10 +357,16 @@ nlookup(struct nlookupdata *nd) ncp = cache_get(ncp); } else { while ((ncp->nc_flag & NCF_MOUNTPT) && ncp != nd->nl_rootncp) { + if (ncp->nc_parent->nc_flag & NCF_DESTROYED) + break; ncp = ncp->nc_parent; /* get to underlying node */ KKASSERT(ncp != NULL && 1); } if (ncp != nd->nl_rootncp) { + if (ncp->nc_parent->nc_flag & NCF_DESTROYED) { + error = EINVAL; + break; + } ncp = ncp->nc_parent; if (ncp == NULL) { error = EINVAL; diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c index 2aa0e9512d..25c1a16299 100644 --- a/sys/kern/vfs_subr.c +++ b/sys/kern/vfs_subr.c @@ -37,7 +37,7 @@ * * @(#)vfs_subr.c 8.31 (Berkeley) 5/26/95 * $FreeBSD: src/sys/kern/vfs_subr.c,v 1.249.2.30 2003/04/04 20:35:57 tegge Exp $ - * $DragonFly: src/sys/kern/vfs_subr.c,v 1.47 2004/11/12 10:58:59 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_subr.c,v 1.48 2004/11/18 20:04:24 dillon Exp $ */ /* @@ -827,7 +827,7 @@ vclean(struct vnode *vp, int flags, struct thread *td) /* * Scrap the vfs cache */ - cache_inval_vp(vp, CINV_SELF); + cache_inval_vp(vp, 0); /* * Check to see if the vnode is in use. If so we have to reference it diff --git a/sys/sys/namecache.h b/sys/sys/namecache.h index ffd0645dda..504d0f8863 100644 --- a/sys/sys/namecache.h +++ b/sys/sys/namecache.h @@ -62,7 +62,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $DragonFly: src/sys/sys/namecache.h,v 1.15 2004/11/12 00:09:27 dillon Exp $ + * $DragonFly: src/sys/sys/namecache.h,v 1.16 2004/11/18 20:04:26 dillon Exp $ */ #ifndef _SYS_NAMECACHE_H_ @@ -130,13 +130,14 @@ typedef struct namecache *namecache_t; #define NCF_UNUSED080 0x0080 #define NCF_ISSYMLINK 0x0100 /* represents a symlink */ #define NCF_ISDIR 0x0200 /* represents a directory */ +#define NCF_DESTROYED 0x0400 /* name association is considered destroyed */ /* * cache_inval[_vp]() flags */ -#define CINV_PARENT 0x0001 /* disconnect from parent in namecache */ -#define CINV_SELF 0x0002 /* disconnect vp from namecache */ -#define CINV_CHILDREN 0x0004 /* disconnect children in namecache */ +#define CINV_DESTROY 0x0001 /* flag so cache_nlookup ignores the ncp */ +#define CINV_UNUSED02 0x0002 +#define CINV_CHILDREN 0x0004 /* recursively set children to unresolved */ #ifdef _KERNEL diff --git a/sys/vfs/nfs/nfs_nqlease.c b/sys/vfs/nfs/nfs_nqlease.c index b7b7a8231d..24dd7f684d 100644 --- a/sys/vfs/nfs/nfs_nqlease.c +++ b/sys/vfs/nfs/nfs_nqlease.c @@ -35,7 +35,7 @@ * * @(#)nfs_nqlease.c 8.9 (Berkeley) 5/20/95 * $FreeBSD: src/sys/nfs/nfs_nqlease.c,v 1.50 2000/02/13 03:32:05 peter Exp $ - * $DragonFly: src/sys/vfs/nfs/Attic/nfs_nqlease.c,v 1.20 2004/11/12 00:09:37 dillon Exp $ + * $DragonFly: src/sys/vfs/nfs/Attic/nfs_nqlease.c,v 1.21 2004/11/18 20:04:28 dillon Exp $ */ @@ -1062,7 +1062,7 @@ nqnfs_clientd(struct nfsmount *nmp, struct ucred *cred, struct nfsd_cargs *ncd, if (np->n_flag & NQNFSEVICTED) { if (vp->v_type == VDIR) nfs_invaldir(vp); - cache_inval_vp(vp, CINV_SELF); + cache_inval_vp(vp, 0); (void) nfs_vinvalbuf(vp, V_SAVE, td, 0); np->n_flag &= ~NQNFSEVICTED; diff --git a/sys/vfs/nfs/nfs_socket.c b/sys/vfs/nfs/nfs_socket.c index d50d638289..1b1c8a4610 100644 --- a/sys/vfs/nfs/nfs_socket.c +++ b/sys/vfs/nfs/nfs_socket.c @@ -35,7 +35,7 @@ * * @(#)nfs_socket.c 8.5 (Berkeley) 3/30/95 * $FreeBSD: src/sys/nfs/nfs_socket.c,v 1.60.2.6 2003/03/26 01:44:46 alfred Exp $ - * $DragonFly: src/sys/vfs/nfs/nfs_socket.c,v 1.21 2004/11/12 00:09:37 dillon Exp $ + * $DragonFly: src/sys/vfs/nfs/nfs_socket.c,v 1.22 2004/11/18 20:04:28 dillon Exp $ */ /* @@ -1166,7 +1166,7 @@ tryagain: * lookup cache, just in case. */ if (error == ESTALE) - cache_inval_vp(vp, CINV_SELF); + cache_inval_vp(vp, CINV_CHILDREN); if (nmp->nm_flag & NFSMNT_NFSV3) { *mrp = mrep; *mdp = md; -- 2.41.0