Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Mar 2005 00:52:35 -0500 (EST)
From:      Jeff Roberson <jroberson@chesapeake.net>
To:        David Schultz <das@FreeBSD.ORG>
Cc:        Stephan Uphoff <ups@tree.com>
Subject:   Re: Freeing vnodes.
Message-ID:  <20050329005142.U54623@mail.chesapeake.net>
In-Reply-To: <20050329044905.GA9730@VARK.MIT.EDU>
References:  <20050314213038.V20708@mail.chesapeake.net> <1110856553.29804.37784.camel@palm> <1110896909.29804.39143.camel@palm> <1111983665.64310.19.camel@palm> <20050329044905.GA9730@VARK.MIT.EDU>

next in thread | previous in thread | raw e-mail | index | archive | help
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:

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++;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050329005142.U54623>