Date: Tue, 29 Mar 2005 09:18:53 -0500 From: David Schultz <das@FreeBSD.ORG> To: Jeff Roberson <jroberson@chesapeake.net> Cc: Stephan Uphoff <ups@tree.com> Subject: Re: Freeing vnodes. Message-ID: <20050329141853.GA13447@VARK.MIT.EDU> In-Reply-To: <20050329085512.M54623@mail.chesapeake.net> References: <20050314213038.V20708@mail.chesapeake.net> <1110856553.29804.37784.camel@palm> <1110896909.29804.39143.camel@palm> <1111983665.64310.19.camel@palm> <20050329071006.GA10416@VARK.MIT.EDU> <20050329030011.I54623@mail.chesapeake.net> <20050329085512.M54623@mail.chesapeake.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Mar 29, 2005, Jeff Roberson wrote:
> I fixed this. You should wrap vn_fullpath() with Giant and then commit
> your patch. I'll do the VFS_LOCK_GIANT() bit. Do you think we can get
> rid of v_id and v_ddid after you commit your patch?
Yeah, I'll merge in your changes and commit that later today.
I just realized that the patch I sent you isn't the same one
that I sent phk last Monday, so it didn't actually include most
of the v_id changes. Once I merge your changes into the following,
v_id can go away entirely.
BTW, I recently noticed that cache_leaf_test() is no longer used
and can also go away. I'll get to that later unless you beat me
to it.
Index: sys/sys/vnode.h
===================================================================
RCS file: /cvs/src/sys/sys/vnode.h,v
retrieving revision 1.293
diff -u -r1.293 vnode.h
--- sys/sys/vnode.h 16 Mar 2005 11:20:51 -0000 1.293
+++ sys/sys/vnode.h 19 Mar 2005 22:00:54 -0000
@@ -145,7 +145,6 @@
TAILQ_HEAD(, namecache) v_cache_dst; /* c Cache entries to us */
u_long v_id; /* c capability identifier */
struct vnode *v_dd; /* c .. vnode */
- u_long v_ddid; /* c .. capability identifier */
/*
* clustering stuff
Index: sys/kern/vfs_cache.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_cache.c,v
retrieving revision 1.90
diff -u -r1.90 vfs_cache.c
--- sys/kern/vfs_cache.c 10 Feb 2005 12:16:42 -0000 1.90
+++ sys/kern/vfs_cache.c 19 Mar 2005 22:01:23 -0000
@@ -169,6 +169,8 @@
static void cache_zap(struct namecache *ncp);
+static int vn_fullpath1(struct thread *td, struct vnode *vp, struct vnode *rdir,
+ char *buf, char **retbuf, u_int buflen);
static MALLOC_DEFINE(M_VFSCACHE, "vfscache", "VFS name cache entries");
@@ -356,9 +358,8 @@
}
if (cnp->cn_namelen == 2 && cnp->cn_nameptr[1] == '.') {
dotdothits++;
- if (dvp->v_dd->v_id != dvp->v_ddid ||
+ if (dvp->v_dd == NULL ||
(cnp->cn_flags & MAKEENTRY) == 0) {
- dvp->v_ddid = 0;
CACHE_UNLOCK();
return (0);
}
@@ -369,7 +370,7 @@
}
hash = fnv_32_buf(cnp->cn_nameptr, cnp->cn_namelen, FNV1_32_INIT);
- hash = fnv_32_buf(&dvp->v_id, sizeof(dvp->v_id), hash);
+ hash = fnv_32_buf(&dvp, sizeof(dvp), hash);
LIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) {
numchecks++;
if (ncp->nc_dvp == dvp && ncp->nc_nlen == cnp->cn_namelen &&
@@ -456,13 +457,7 @@
return;
}
if (cnp->cn_namelen == 2 && cnp->cn_nameptr[1] == '.') {
- if (vp) {
- dvp->v_dd = vp;
- dvp->v_ddid = vp->v_id;
- } else {
- dvp->v_dd = dvp;
- dvp->v_ddid = 0;
- }
+ dvp->v_dd = vp;
return;
}
}
@@ -477,7 +472,8 @@
ncp->nc_flag = cnp->cn_flags & ISWHITEOUT ? NCF_WHITE : 0;
} else if (vp->v_type == VDIR) {
vp->v_dd = dvp;
- vp->v_ddid = dvp->v_id;
+ } else {
+ vp->v_dd = NULL;
}
/*
@@ -490,7 +486,7 @@
len = ncp->nc_nlen = cnp->cn_namelen;
hash = fnv_32_buf(cnp->cn_nameptr, len, FNV1_32_INIT);
bcopy(cnp->cn_nameptr, ncp->nc_name, len);
- hash = fnv_32_buf(&dvp->v_id, sizeof(dvp->v_id), hash);
+ hash = fnv_32_buf(&dvp, sizeof(dvp), hash);
ncpp = NCHHASH(hash);
LIST_INSERT_HEAD(ncpp, ncp, nc_hash);
if (LIST_EMPTY(&dvp->v_cache_src)) {
@@ -542,16 +538,8 @@
* Invalidate all entries to a particular vnode.
*
* Remove all entries in the namecache relating to this vnode and
- * change the v_id. We take the v_id from a global counter, since
- * it becomes a handy sequence number in crash-dumps that way.
- * No valid vnode will ever have (v_id == 0).
- *
- * XXX: Only time and the size of v_id prevents this from failing:
- * XXX: In theory we should hunt down all (struct vnode*, v_id)
- * XXX: soft references and nuke them, at least on the global
- * XXX: v_id wraparound. The period of resistance can be extended
- * XXX: by incrementing each vnodes v_id individually instead of
- * XXX: using the global v_id.
+ * change the v_id. The counter is per-vnode, so two vnodes may
+ * have the same v_id. No valid vnode will ever have (v_id == 0).
*/
/*
@@ -562,7 +550,6 @@
cache_purge(vp)
struct vnode *vp;
{
- static u_long nextid;
CACHE_LOCK();
while (!LIST_EMPTY(&vp->v_cache_src))
@@ -570,12 +557,9 @@
while (!TAILQ_EMPTY(&vp->v_cache_dst))
cache_zap(TAILQ_FIRST(&vp->v_cache_dst));
- do
- nextid++;
- while (nextid == vp->v_id || !nextid);
- vp->v_id = nextid;
- vp->v_dd = vp;
- vp->v_ddid = 0;
+ MPASS(vp->v_id > 0);
+ vp->v_id++;
+ vp->v_dd = NULL;
CACHE_UNLOCK();
}
@@ -780,14 +764,6 @@
SYSCTL_INT(_debug, OID_AUTO, disablecwd, CTLFLAG_RW, &disablecwd, 0,
"Disable the getcwd syscall");
-/* Various statistics for the getcwd syscall */
-static u_long numcwdcalls; STATNODE(CTLFLAG_RD, numcwdcalls, &numcwdcalls);
-static u_long numcwdfail1; STATNODE(CTLFLAG_RD, numcwdfail1, &numcwdfail1);
-static u_long numcwdfail2; STATNODE(CTLFLAG_RD, numcwdfail2, &numcwdfail2);
-static u_long numcwdfail3; STATNODE(CTLFLAG_RD, numcwdfail3, &numcwdfail3);
-static u_long numcwdfail4; STATNODE(CTLFLAG_RD, numcwdfail4, &numcwdfail4);
-static u_long numcwdfound; STATNODE(CTLFLAG_RD, numcwdfound, &numcwdfound);
-
/* Implementation of the getcwd syscall */
int
__getcwd(td, uap)
@@ -802,94 +778,29 @@
kern___getcwd(struct thread *td, u_char *buf, enum uio_seg bufseg, u_int buflen)
{
char *bp, *tmpbuf;
- int error, i, slash_prefixed;
struct filedesc *fdp;
- struct namecache *ncp;
- struct vnode *vp;
+ int error;
- numcwdcalls++;
if (disablecwd)
return (ENODEV);
if (buflen < 2)
return (EINVAL);
if (buflen > MAXPATHLEN)
buflen = MAXPATHLEN;
- mtx_lock(&Giant);
- error = 0;
- tmpbuf = bp = malloc(buflen, M_TEMP, M_WAITOK);
- bp += buflen - 1;
- *bp = '\0';
+
+ tmpbuf = malloc(buflen, M_TEMP, M_WAITOK);
fdp = td->td_proc->p_fd;
- slash_prefixed = 0;
FILEDESC_LOCK(fdp);
- for (vp = fdp->fd_cdir; vp != fdp->fd_rdir && vp != rootvnode;) {
- if (vp->v_vflag & VV_ROOT) {
- if (vp->v_mount == NULL) { /* forced unmount */
- error = EBADF;
- goto out;
- }
- vp = vp->v_mount->mnt_vnodecovered;
- continue;
- }
- if (vp->v_dd->v_id != vp->v_ddid) {
- numcwdfail1++;
- error = ENOTDIR;
- goto out;
- }
- CACHE_LOCK();
- ncp = TAILQ_FIRST(&vp->v_cache_dst);
- if (!ncp) {
- numcwdfail2++;
- CACHE_UNLOCK();
- error = ENOENT;
- goto out;
- }
- if (ncp->nc_dvp != vp->v_dd) {
- numcwdfail3++;
- CACHE_UNLOCK();
- error = EBADF;
- goto out;
- }
- for (i = ncp->nc_nlen - 1; i >= 0; i--) {
- if (bp == tmpbuf) {
- numcwdfail4++;
- CACHE_UNLOCK();
- error = ENOMEM;
- goto out;
- }
- *--bp = ncp->nc_name[i];
- }
- if (bp == tmpbuf) {
- numcwdfail4++;
- CACHE_UNLOCK();
- error = ENOMEM;
- goto out;
- }
- *--bp = '/';
- slash_prefixed = 1;
- vp = vp->v_dd;
- CACHE_UNLOCK();
- }
- if (!slash_prefixed) {
- if (bp == tmpbuf) {
- numcwdfail4++;
- error = ENOMEM;
- goto out;
- }
- *--bp = '/';
- }
+ error = vn_fullpath1(td, fdp->fd_cdir, fdp->fd_rdir, tmpbuf,
+ &bp, buflen);
FILEDESC_UNLOCK(fdp);
- mtx_unlock(&Giant);
- numcwdfound++;
- if (bufseg == UIO_SYSSPACE)
- bcopy(bp, buf, strlen(bp) + 1);
- else
- error = copyout(bp, buf, strlen(bp) + 1);
- free(tmpbuf, M_TEMP);
- return (error);
-out:
- FILEDESC_UNLOCK(fdp);
- mtx_unlock(&Giant);
+
+ if (!error) {
+ if (bufseg == UIO_SYSSPACE)
+ bcopy(bp, buf, strlen(bp) + 1);
+ else
+ error = copyout(bp, buf, strlen(bp) + 1);
+ }
free(tmpbuf, M_TEMP);
return (error);
}
@@ -907,10 +818,10 @@
SYSCTL_INT(_debug, OID_AUTO, disablefullpath, CTLFLAG_RW, &disablefullpath, 0,
"Disable the vn_fullpath function");
+/* These count for kern___getcwd(), too. */
STATNODE(numfullpathcalls);
STATNODE(numfullpathfail1);
STATNODE(numfullpathfail2);
-STATNODE(numfullpathfail3);
STATNODE(numfullpathfail4);
STATNODE(numfullpathfound);
@@ -921,90 +832,112 @@
int
vn_fullpath(struct thread *td, struct vnode *vn, char **retbuf, char **freebuf)
{
- char *bp, *buf;
- int i, slash_prefixed;
+ char *buf;
struct filedesc *fdp;
- struct namecache *ncp;
- struct vnode *vp;
+ int error;
- numfullpathcalls++;
if (disablefullpath)
return (ENODEV);
if (vn == NULL)
return (EINVAL);
+
buf = malloc(MAXPATHLEN, M_TEMP, M_WAITOK);
- bp = buf + MAXPATHLEN - 1;
- *bp = '\0';
fdp = td->td_proc->p_fd;
- slash_prefixed = 0;
- ASSERT_VOP_LOCKED(vn, "vn_fullpath");
FILEDESC_LOCK(fdp);
- for (vp = vn; vp != fdp->fd_rdir && vp != rootvnode;) {
+ error = vn_fullpath1(td, vn, fdp->fd_rdir, buf, retbuf, MAXPATHLEN);
+ FILEDESC_UNLOCK(fdp);
+
+ if (!error)
+ *freebuf = buf;
+ else
+ free(buf, M_TEMP);
+ return (error);
+}
+
+/*
+ * The magic behind kern___getcwd() and vn_fullpath().
+ */
+static int
+vn_fullpath1(struct thread *td, struct vnode *vp, struct vnode *rdir,
+ char *buf, char **retbuf, u_int buflen)
+{
+ char *bp;
+ int error, i, slash_prefixed;
+ struct namecache *ncp;
+
+ bp = buf + buflen - 1;
+ *bp = '\0';
+ error = 0;
+ slash_prefixed = 0;
+
+ CACHE_LOCK();
+ numfullpathcalls++;
+ if (vp->v_type != VDIR) {
+ ncp = TAILQ_FIRST(&vp->v_cache_dst);
+ if (!ncp) {
+ numfullpathfail2++;
+ CACHE_UNLOCK();
+ return (ENOENT);
+ }
+ for (i = ncp->nc_nlen - 1; i >= 0 && bp > buf; i--)
+ *--bp = ncp->nc_name[i];
+ if (bp == buf) {
+ numfullpathfail4++;
+ CACHE_UNLOCK();
+ return (ENOMEM);
+ }
+ *--bp = '/';
+ slash_prefixed = 1;
+ vp = ncp->nc_dvp;
+ }
+ while (vp != rdir && vp != rootvnode) {
if (vp->v_vflag & VV_ROOT) {
if (vp->v_mount == NULL) { /* forced unmount */
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
- return (EBADF);
+ error = EBADF;
+ break;
}
vp = vp->v_mount->mnt_vnodecovered;
continue;
}
- if (vp != vn && vp->v_dd->v_id != vp->v_ddid) {
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
+ if (vp->v_dd == NULL) {
numfullpathfail1++;
- return (ENOTDIR);
+ error = ENOTDIR;
+ break;
}
- CACHE_LOCK();
ncp = TAILQ_FIRST(&vp->v_cache_dst);
if (!ncp) {
numfullpathfail2++;
- CACHE_UNLOCK();
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
- return (ENOENT);
+ error = ENOENT;
+ break;
}
- if (vp != vn && ncp->nc_dvp != vp->v_dd) {
- numfullpathfail3++;
- CACHE_UNLOCK();
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
- return (EBADF);
- }
- for (i = ncp->nc_nlen - 1; i >= 0; i--) {
- if (bp == buf) {
- numfullpathfail4++;
- CACHE_UNLOCK();
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
- return (ENOMEM);
- }
+ MPASS(ncp->nc_dvp == vp->v_dd);
+ for (i = ncp->nc_nlen - 1; i >= 0 && bp != buf; i--)
*--bp = ncp->nc_name[i];
- }
if (bp == buf) {
numfullpathfail4++;
- CACHE_UNLOCK();
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
- return (ENOMEM);
+ error = ENOMEM;
+ break;
}
*--bp = '/';
slash_prefixed = 1;
vp = ncp->nc_dvp;
+ }
+ if (error) {
CACHE_UNLOCK();
+ return (error);
}
if (!slash_prefixed) {
if (bp == buf) {
numfullpathfail4++;
- FILEDESC_UNLOCK(fdp);
- free(buf, M_TEMP);
+ CACHE_UNLOCK();
return (ENOMEM);
+ } else {
+ *--bp = '/';
}
- *--bp = '/';
}
- FILEDESC_UNLOCK(fdp);
numfullpathfound++;
+ CACHE_UNLOCK();
+
*retbuf = bp;
- *freebuf = buf;
return (0);
}
Index: sys/kern/vfs_subr.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.596
diff -u -r1.596 vfs_subr.c
--- sys/kern/vfs_subr.c 15 Mar 2005 14:38:16 -0000 1.596
+++ sys/kern/vfs_subr.c 19 Mar 2005 21:59:35 -0000
@@ -837,13 +837,12 @@
vp = (struct vnode *) uma_zalloc(vnode_zone, M_WAITOK|M_ZERO);
mtx_init(&vp->v_interlock, "vnode interlock", NULL, MTX_DEF);
- vp->v_dd = vp;
bo = &vp->v_bufobj;
bo->__bo_vnode = vp;
bo->bo_mtx = &vp->v_interlock;
vp->v_vnlock = &vp->v_lock;
lockinit(vp->v_vnlock, PVFS, tag, VLKTIMEOUT, LK_NOPAUSE);
- cache_purge(vp); /* Sets up v_id. */
+ vp->v_id = 1;
LIST_INIT(&vp->v_cache_src);
TAILQ_INIT(&vp->v_cache_dst);
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050329141853.GA13447>