[strongSwan-dev] [PATCH] log: do not acquire lock if nobody is listening

[Tobias Brunner]

Hi Christoph,

>> At least we should make sure the read is atomic.  Something like this
>> perhaps?
>>
>>    if (ref_cur(&this->max_level[group]) < level &&
>>        ref_cur(&this->max_vlevel[group]) < level)
>>    {
>>      return;
>>    }
> 
> 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 [1] 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.

Regards,
Tobias

[1] http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=f2b930bc1