cvs commit: src/sys/kern vfs_subr.c

[Jeff Roberson]

On Thu, 16 Jun 2005, Jeff Roberson wrote:

> jeff        2005-06-16 04:41:42 UTC

This fixes duplicate frees from the vnode zone, crashes in vnlrureclaim
and vlru_free() and a few other related problems.  Basically, I've
converted the holdcnt to be a proper ref counting system, while use count
maintains the traditional vfs use count semantics, which are not really
compatbile with structure life-time issues.  It looks like a big change
but it really makes things much simpler.  Briefly, the rules are this:

1) vget/grab a use count if you're a normal consumer of the vnode API.
That goes for syscalls, mmaping, nfs, etc.

2) vhold() if you want to prevent a vnode from being recycled while you
hold a reference to it.  The name cache uses this, as does the vm when
pages reference the vnode's vm object.  This is also used for vlrureclaim,
vflush, vtryrecyle, etc. which want to make sure the vnode is not recycled
while it has a local reference.

I'll probably add a comment to vnode.h to describe this in more detail.

>
>   FreeBSD src repository
>
>   Modified files:
>     sys/kern             vfs_subr.c
>   Log:
>    - Change holdcnt use around vnode recycling.  We now always keep a holdcnt
>      ref while we're calling vgone().  This prevents transient refs from
>      re-adding us to the free list.  Previously, a vfree() triggered via
>      vinvalbuf() getting rid of all of a vnode's pages could place a partially
>      destructed vnode on the free list where vtryrecycle() could find it.  The
>      first call to vtryrecycle would hang up on the vnode lock, but when it
>      failed it would place a now dead vnode onto the free list, and another
>      call to vtryrecycle() would free an already free vnode.  There were many
>      complications of having a zero ref count while freeing which can now go
>      away.
>    - Change vdropl() to release the interlock before returning.  All callers
>      now respect this, so vdropl() directly frees VI_DOOMED vnodes once the
>      last ref is dropped.  This means that we'll never have VI_DOOMED vnodes
>      on the free list.
>    - Seperate v_incr_usecount() into v_incr_usecount(), v_decr_usecount() and
>      v_decr_useonly().  The incr/decr split is so that incr usecount can
>      return with the interlock still held while decr drops the interlock so
>      it can call vdropl() which will potentially free the vnode.  The calling
>      function can't drop the lock of an already free'd node.  v_decr_useonly()
>      drops a usecount without droping the hold count.  This is done so the
>      usecount reaches zero in vput() before we recycle, however the holdcount
>      is still 1 which prevents any new references from placing the vnode
>      back on the free list.
>    - Fix vnlrureclaim() to vhold the vnode since it doesn't do a vget().  We
>      wouldn't want vnlrureclaim() to bump the usecount since this has
>      different semantics.  Also change vnlrureclaim() to do a NOWAIT on the
>      vn_lock.  When this function runs we're usually in a desperate situation
>      and we wouldn't want to wait for any specific vnode to be released.
>    - Fix a bunch of misc comments to reflect the new behavior.
>    - Add vhold() and vdrop() to vflush() for the same reasons that we do in
>      vlrureclaim().  Previously we held no reference and a vnode could have
>      been freed while we were waiting on the lock.
>    - Get rid of vlruvp() and vfreehead().  Neither are used.  vlruvp() should
>      really be rethought before it's reintroduced.
>    - vgonel() always returns with the vnode locked now and never puts the
>      vnode back on a free list.  The vnode will be freed as soon as the last
>      reference is released.
>
>   Sponsored by:   Isilon Systems, Inc.
>   Debugging help from:    Kris Kennaway, Peter Holm
>   Approved by:    re (blanket vfs)
>
>   Revision  Changes    Path
>   1.632     +198 -202  src/sys/kern/vfs_subr.c
>