Re: Freeing vnodes.

From: Jeff Roberson (jroberson_at_chesapeake.net)
Date: 03/29/05

  • Next message: David Schultz: "Re: Freeing vnodes."
    Date: Tue, 29 Mar 2005 00:55:44 -0500 (EST)
    To: David Schultz <das@freebsd.org>
    
    

    On Tue, 29 Mar 2005, Jeff Roberson wrote:

    > On Mon, 28 Mar 2005, David Schultz wrote:
    >
    > > On Mon, Mar 28, 2005, Jeff Roberson wrote:
    > > > > > I am worried about the v_dd,v_ddid fields of a directory B that has the
    > > > > > to be released vnode A as parent. (Obviously in this case there is no
    > > > > > namecache entry with the vnode A as the directory (nc_dvp))
    > > > > >
    > > > > > Right now A is type stable - but if A is released, access to B->v_dd
    > > > > > may cause a page fault.
    > > > > >
    > > > > > Stephan
    > > > >
    > > > > Jeff,
    > > > >
    > > > > Do you plan to address the problem now that the code is checked in?
    > > >
    > > > Vnodes with children in the name cache are held with vhold() and not
    > > > recycled.
    > >
    > > Yes, but cache_purge() is called directly in a number of places
    > > where the vnode may have children, e.g. in mount. So dangling
    > > references might still be possible unless cache_purge() fixes up
    > > the children's v_dd pointers appropriately.
    > >
    >
    > ah, indeed. How does this look:

    Also, are the ids really necessary now that we don't reuse vnodes?
    Shouldn't the pointer be sufficient?

    >
    > Index: vfs_cache.c
    > ===================================================================
    > RCS file: /home/ncvs/src/sys/kern/vfs_cache.c,v
    > retrieving revision 1.93
    > diff -u -r1.93 vfs_cache.c
    > --- vfs_cache.c 28 Mar 2005 13:29:48 -0000 1.93
    > +++ vfs_cache.c 29 Mar 2005 05:48:40 -0000
    > @@ -553,22 +553,30 @@
    > * XXX: by incrementing each vnodes v_id individually instead of
    > * XXX: using the global v_id.
    > */
    > -
    > -/*
    > - * XXX This is sometimes called when a vnode may still be re-used, in
    > which
    > - * case v_dd may be invalid. Need to look this up.
    > - */
    > void
    > cache_purge(vp)
    > struct vnode *vp;
    > {
    > + struct namecache *ncp;
    > static u_long nextid;
    >
    > CACHE_LOCK();
    > while (!LIST_EMPTY(&vp->v_cache_src))
    > cache_zap(LIST_FIRST(&vp->v_cache_src));
    > - while (!TAILQ_EMPTY(&vp->v_cache_dst))
    > - cache_zap(TAILQ_FIRST(&vp->v_cache_dst));
    > + while (!TAILQ_EMPTY(&vp->v_cache_dst)) {
    > + struct vnode *cvp;
    > +
    > + ncp = TAILQ_FIRST(&vp->v_cache_dst);
    > + /*
    > + * We must reset v_dd of any children so they don't continue
    > + * to point to us.
    > + */
    > + if ((cvp = ncp->nc_vp) && cvp->v_dd == vp) {
    > + cvp->v_dd = cvp;
    > + cvp->v_ddid = 0;
    > + }
    > + cache_zap(ncp);
    > + }
    >
    > do
    > nextid++;
    >
    > _______________________________________________
    > 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"
    >
    _______________________________________________
    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: David Schultz: "Re: Freeing vnodes."