Freeing vnodes.

[Jeff Roberson]

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