Re: Freeing vnodes.

From: David Schultz (das_at_FreeBSD.ORG)
Date: 03/29/05

  • Next message: Colin Percival: "Adding bsdiff to the base system" 
    Date: Tue, 29 Mar 2005 09:18:53 -0500
To: Jeff Roberson <jroberson@chesapeake.net>

    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);
             }
    _______________________________________________
    freebsd-arch@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-arch
    To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"

  • Next message: Colin Percival: "Adding bsdiff to the base system"

    Relevant Pages

    • Re: Is there a need to hold the vnode lock across getnewbuf( ) in getblk?
      ... lock for the vnode. ... getblk(struct vnode * vp, daddr_t blkno, int size, int slpflag, int ... Buffer is not in-core, create new buffer. ... any need to hold the vnode lock across getnewbuf call? ...
      (freebsd-current)
    • Re: md deadlocks on wdrain. Was: [Re: quota and snapshotsin6.1-RELEASE]
      ... as without the machine deadlocks without even trying. ... no choice but to migrate away from using any vnode backed ... retrieving revision 1.513 ... diff -u -r1.513 vfs_bio.c ...
      (freebsd-stable)
    • Re: nfs on zfs panic
      ... the zfs code, zfs_vget always returns zero, even if it failed to ... find a vnode which matches the given 'inode' number. ... I tried to commit it but got the 'need Approved by: ...
      (freebsd-current)
    • Re: nfs on zfs panic
      ... the zfs code, zfs_vget always returns zero, even if it failed to ... find a vnode which matches the given 'inode' number. ... I tried to commit it but got the 'need Approved by: ...
      (freebsd-current)
    • Re: ZFS leaking vnodes (sort of)
      ... Eventually I realised that virtually nothing ever ended up on the vnode ... I looked a bit closer at the ZFS code and poked around with DDB and I ... diff -u -r1.22 zfs_vnops.c ... retrieving revision 1.8 ...
      (freebsd-current)
    • Quantcast