Date: Thu, 16 Jun 2005 00:46:59 -0400 (EDT) From: Jeff Roberson <jroberson@chesapeake.net> To: Jeff Roberson <jeff@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern vfs_subr.c Message-ID: <20050616004203.N66638@mail.chesapeake.net> In-Reply-To: <200506160441.j5G4fg0l078419@repoman.freebsd.org> References: <200506160441.j5G4fg0l078419@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050616004203.N66638>