[strongSwan-dev] [PATCH] log: do not acquire lock if nobody is listening
christophe.gouault at 6wind.com
Tue Apr 15 10:31:48 CEST 2014
Thanks for reviewing my patches.
On 04/14/2014 06:31 PM, Tobias Brunner wrote:
> Hi Christoph,
>> + if (this->max_level[group] < level &&
>> + this->max_vlevel[group] < level)
>> + return;
> Unfortunately, this might not be thread-safe. It's essentially the same
> problem I described in my response to your half-open IKE_SA patch.
> Because the log levels/logger registrations can be changed at runtime
> the threads that are calling vlog() might not be aware of these updates.
I agree it is not thread safe, but the only risk I suspect it that a log
listener might lose a message at the time it registers or increases its
loglevel (assuming that reading this->max_level[group] or
this->max_vlevel[group] is atomic), in which case it does not seem to be
a serious problem.
What is your opinion?
>> + 49.87% charon [kernel.kallsyms] [k] do_raw_spin_lock
> I was not able to reproduce numbers that come even close on a x64 Ubuntu
> Linux system. On what platform/architecture are you running this? And
> how exactly did you determine the numbers? For instance, what is charon
> doing during your perf record run? And how is it configured (e.g.
I was running charon on an x86_64, with 8 cores/16 hyperthreads, on a
debian 7.4 system.
Here is my strongswan.conf:
ikesa_table_size = 65536
ikesa_table_segments = 16384
cookie_threshold = 0
block_threshold = 0
threads = 64
default = -1
default = -1
charon replies to IKEv2 negotiations initiated by a remote load-tester
at a high throughput.
> It seems strange that acquiring an *uncontested* read lock (the write
> lock is only very rarely held, in your case probably never) can produce
> such a massive overhead. Acquiring such a lock should be possible
> without syscall, just some atomic compare-and-swap operation should be
I was also surprised by this behavior. I didn't expect spin_locks to be
invoked on a read lock.
> Unless, of course, your architecture does not support such atomic
> operations, hence the library has to resort to using a regular
> (spin-)lock. Or if the library just is not optimized very much and
> always does this.
> It might not be a bad idea to check if there is any potential in
> optimizing the pthread_rwlock_rdlock implementation in the pthread
> library you are using.
I am using debian's libpthread-2.13.so for x86_64. I would reasonably
expect that it supports optimized read locks.
> Since this rwlock is not the only one in the
> strongSwan code base it might increase the overall performance (but this
> is definitely the lock that is most often acquired).
> To reduce the overhead somewhat you could also compile strongSwan with
> reduced log statements (see ).
More information about the Dev