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 08:56:32 -0500 (EST)
    To: David Schultz <das@freebsd.org>
    
    

    On Tue, 29 Mar 2005, Jeff Roberson wrote:

    > On Tue, 29 Mar 2005, David Schultz wrote:
    >
    > > On Tue, Mar 29, 2005, Jeff Roberson wrote:
    > > > 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?
    > >
    > > I think so. The patch I sent you a few days ago gets rid of v_id
    > > except in vfs_cache_lookup(), where it is used to guarantee that
    > > the vnode hasn't changed while sleeping in vn_lock(). With vnode
    > > reclamation, that isn't safe anyway, so if you fix vfs_cache_lookup(),
    > > we can kill v_id completely.
    >
    > You're right, cache_lookup() needs to be changed to return a referenced
    > vnode. There are only a few callers outside of vfs_cache.c that I'll have
    > to change. I'll put this on my todo list. After that v_id can go away.
    > I'll look at your patch again soon.

    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?

    >
    > >
    > > Your patch looks okay at a glance, but shouldn't you be iterating over
    > > v_cache_src instead of v_cache_dst?
    > >
    >
    > Yes, I thought I double checked that. I haven't done any name cache stuff
    > in a while, and I always get confused whether src means sources for this
    > vnode, or names this vnode is a source for.
    > _______________________________________________
    > 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."

    Relevant Pages

    • Re: panic: getnewvnode: free vnode isnt
      ... >> Could people seeing this panic try the following patch? ... >> callers of vcleanensure that the vnode is not on the free list, ... > The patch hasn't stopped the panics, but I've tracked down a series of ... I've just started using USB keyboard/mouse and a KVM. ...
      (freebsd-current)
    • Re: jail getfsstat patches.
      ... > The best way to do it is to walk up from the vnode where file system is ... > mounted on to the vnode where jail's root directory starts. ... > patch to commit so we can review it together. ... I'd rather see it built in than to have to keep a separate module in ...
      (freebsd-current)
    • Re: fs/udf: vm pages "overlap" while reading large dir [patch]
      ... Actually the patch is not entirely correct. ... reading via device vnode would work correctly. ... Hunk5 - return EOPNOTSUPP for unusual files, ...
      (freebsd-hackers)
    • Re: panic: getnewvnode: free vnode isnt
      ... > Could people seeing this panic try the following patch? ... > callers of vcleanensure that the vnode is not on the free list, ... The patch hasn't stopped the panics, but I've tracked down a series of ... while the modem was plugged in. ...
      (freebsd-current)
    • Re: Freeing vnodes.
      ... I'll merge in your changes and commit that later today. ... diff -u -r1.90 vfs_cache.c ... * Invalidate all entries to a particular vnode. ...
      (freebsd-arch)