refcount_release_take_##lock

John-Mark Gurney jmg at funkthat.com
Sat Oct 25 19:04:10 UTC 2014


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...

> 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_

> Comments?

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

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."


More information about the freebsd-arch mailing list