refcount_release_take_##lock
Mateusz Guzik
mjguzik at gmail.com
Sat Mar 14 00:49:24 UTC 2015
On Sat, Mar 14, 2015 at 01:15:26AM +0100, Mateusz Guzik wrote:
> 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. :>
>
One of the things i don't like is the fact that we don't have a
refcount_t.
If we did, we could do the following: with debug enabled it would be
extended with a function pointer.
On init you would:
refcount_init(&refc, asserting_func);
So that if you use 0->1 transition, you can provide a function which
asserts that appropriate locks are held if you happen to trigger such a
case.
I doubt there is any runtime relible way to check that you could ref in
general.
Would this help with your concerns?
In genearl, I don't see how you can go around 0->1 transitions in some
cases without introducing a bunch of ugly code which only increases
complexity.
If you are so concerned that *_if functions can encourage refcount
mismanage, we can put a big fat warning no problem.
--
Mateusz Guzik <mjguzik gmail.com>
More information about the freebsd-arch
mailing list