refcount_release_take_##lock

[Mateusz Guzik]

On Sat, Oct 25, 2014 at 12:04:07PM -0700, John-Mark Gurney wrote:
> Mateusz Guzik wrote this message on Sat, Oct 25, 2014 at 20:44 +0200:
> > The following idiom is used here and there:
> > 
> > int old;
> > old = obj->ref;
> > if (old > 1 && atomic_cmpset_int(&obj->ref, old, old -1))
> > 	return;
> > lock(&something);
> > if (refcount_release(&obj->ref) == 0) {
> > 	unlock(&something);
> > 	return;
> > }
> > free up
> > unlock(&something);
> > 
> > ==========
> 
> Couldn't this be better written as:
> if (__predict_false(refcount_release(&obj->ref) == 0)) {
> 	lock(&something);
> 	if (__predict_true(!obj->ref)) {
> 		free up
> 	}
> 	unlock(&something);
> }
> 
> The reason I'm asking is that I changed how IPsec SA ref counting was
> handled, and used something similar...
> 
> My code gets rid of a branch, and is better in that it uses refcount
> API properly, instead of using atomic_cmpset_int...
> 

This is used when given obj is kept on a list and code which traverses
it (locked) expects found objects to be valid to ref.

If we were to reach count of 0 and then lock, it would be possible that
other thread refed + unrefed the object and is now trying to lock as
well.

That could be remedied for type stable object by having a generation
counter, but I doubt it's worth it. Not to mention objects we lock here
are freeable :)


> > I decided to implement it as a common function.
> > 
> > We have only refcount.h and I didn't want to bloat all including code
> > with additional definitions and as such I came up with a macro that has
> > to be used in .c file and that will define appropriate inline func.
> > 
> > I'm definitely looking for better names for REFCOUNT_RELEASE_TAKE_USE_
> > macro, assuming it has to stay.
> 
> You could shorten it to REFCNT_REL_TAKE_
> 

All function use full 'refcount_release' and the like, so that would be
inconsistent.

Losing 'take' may be an option, I don't know.

> > Comments?

> 
> Will you update the refcount(9) man page w/ documentation before
> committing?
> 

Sure.

-- 
Mateusz Guzik <mjguzik gmail.com>