Date: Tue, 29 Mar 2005 00:55:44 -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: <20050329005351.B54623@mail.chesapeake.net> In-Reply-To: <20050329005142.U54623@mail.chesapeake.net> References: <20050314213038.V20708@mail.chesapeake.net> <1110856553.29804.37784.camel@palm> <1110896909.29804.39143.camel@palm> <1111983665.64310.19.camel@palm> <20050329005142.U54623@mail.chesapeake.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050329005351.B54623>