[strongSwan-dev] [PATCH] log: do not acquire lock if nobody is listening
tobias at strongswan.org
Thu Apr 17 18:09:38 CEST 2014
>> At least we should make sure the read is atomic. Something like this
>> if (ref_cur(&this->max_level[group]) < level &&
>> ref_cur(&this->max_vlevel[group]) < level)
> Yes, right. Even if in most cases, reading an enum is atomic, some
> platforms do not guarantee it.
> In this case, we should perform atomic operation wherever these levels
> are read or manipulated, even under read or write lock.
I pushed a commit  to the atomic-ref branch that implements this
using the two flavors of GCC atomic built-ins directly.
It's unfortunate that reading the value with the __sync built-ins
requires such an expensive operation (and that GCC seems unable to
optimize it even with the second argument set to 0). Well, better safe
than sorry, I guess, and in your case it is probably still faster than
having to acquire the read lock. Plus with GCC 4.7 or newer you get the
faster __atomic built-ins.
The check is disabled completely if these built-ins are not available,
because fallbacks (like those for ref_*) would not improve the
performance. But since even the atomic implementation allows races an
alternative approach would be to use __atomic* if available and just
directly read and write if not (thus avoiding the __sync penalty). We
could make max_[v]level an int32_t array to be reasonably safe.
Let me know what you think.
We should probably also add support for clang's __c11 built-ins (and
check for stdatomic.h). So I guess adding a more generic abstraction
layer for this would make sense.
More information about the Dev