Date: Sat, 14 Mar 2015 01:15:26 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: John-Mark Gurney <jmg@funkthat.com> Cc: freebsd-arch@freebsd.org Subject: Re: refcount_release_take_##lock Message-ID: <20150314001526.GC32157@dft-labs.eu> In-Reply-To: <20150313235838.GM32288@funkthat.com> References: <20141025184448.GA19066@dft-labs.eu> <201410281413.58414.jhb@freebsd.org> <20141028193404.GB12014@dft-labs.eu> <201411111427.15407.jhb@freebsd.org> <20150313231607.GB32157@dft-labs.eu> <20150313235838.GM32288@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Mar 13, 2015 at 04:58:38PM -0700, John-Mark Gurney wrote: > Mateusz Guzik wrote this message on Sat, Mar 14, 2015 at 00:16 +0100: > > In the meantime I wrote a new version. > > > > Apart from locking-handling primitives this time we get > > refcount_acquire_if_greater and refcount_release_if_greater helpers. > > I don't see how this is of any benefit... The smallest value you can > provide is 0, which means the only time a reference can be obtained is > if the caller already has a reference. If you don't have a reference > (making it =0), it isn't safe to call this function on the object, as > it could be free'd, and point to a different type of object... Even if > you implement type-safe memory (which we shouldn't use more of), it's > less than ideal, since you then have to check if the object is the same > object you were expecting, and need to release it... > > The release_if is even more problematic IMO... > I see I forgot to note the rationale in my e-mail. The kernel already uses 'refing an object with ref = 0' extensively in vfs. For instance entering a name to the namecache does not increase hold count of the vnode. A thread doing lookup locks the cache shared, locks the interlock, unlocks the cache and calls vget which blindly vholds the vnode, which quite often does transition 0->1. What prevents freeing of the vnode is name cache lock and later the interlock. All v_holdcnt manipulation is done with the interlock held. Crucial value changes are 0->1 and 1->0 and we need the lock here to ensure consistency. However, as long as we modify this counter in a way which does not go 0->1 nor 1->0 we don't have take the interlock and not doing so increases scalability. So for instance in aforementioned case of namecache, the vnode is kept stable by namecache lock and if v_holdcnt is >=1, we can increase it without taking the interlock which I plan to do. But in order to do that I need primitives which wrap such functionality. Once more, stability of the object in question has to be ensured in other manners. > After reading the previous discussion, I really don't like this. If > this gets approved (others override my objection), we need some docs > that say this should never be used, and it's use is only in the unsafe > case where the containing data structure does NOT have a reference to > the object. Well it should be quite obvious you can't just ref random objects. :> -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150314001526.GC32157>