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

[Tobias Brunner]

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.

> +  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.
charon.threads)?

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

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.  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 [1]).

Regards,
Tobias

[1]
http://wiki.strongswan.org/projects/strongswan/wiki/LoggerConfiguration#Compile-time-configuration