From owner-freebsd-arch@FreeBSD.ORG Tue Mar 29 14:19:04 2005 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B5A1A16A4CE for ; Tue, 29 Mar 2005 14:19:04 +0000 (GMT) Received: from VARK.MIT.EDU (VARK.MIT.EDU [18.95.3.179]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2CB7443D2F for ; Tue, 29 Mar 2005 14:19:04 +0000 (GMT) (envelope-from das@FreeBSD.ORG) Received: from VARK.MIT.EDU (localhost [127.0.0.1]) by VARK.MIT.EDU (8.13.3/8.13.1) with ESMTP id j2TEIrMb013564; Tue, 29 Mar 2005 09:18:53 -0500 (EST) (envelope-from das@FreeBSD.ORG) Received: (from das@localhost) by VARK.MIT.EDU (8.13.3/8.13.1/Submit) id j2TEIr54013563; Tue, 29 Mar 2005 09:18:53 -0500 (EST) (envelope-from das@FreeBSD.ORG) Date: Tue, 29 Mar 2005 09:18:53 -0500 From: David Schultz To: Jeff Roberson Message-ID: <20050329141853.GA13447@VARK.MIT.EDU> Mail-Followup-To: Jeff Roberson , arch@FreeBSD.ORG, Stephan Uphoff 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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050329085512.M54623@mail.chesapeake.net> cc: arch@FreeBSD.ORG cc: Stephan Uphoff Subject: Re: Freeing vnodes. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Mar 2005 14:19:04 -0000 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); }