Re: [PATCH 2/2] ipc/sem: sem_lock with hysteresis

From: Manfred Spraul
Date: Sat Jun 25 2016 - 13:38:03 EST


On 06/21/2016 10:29 PM, Davidlohr Bueso wrote:
On Sat, 18 Jun 2016, Manfred Spraul wrote:

sysv sem has two lock modes: One with per-semaphore locks, one lock mode
with a single big lock for the whole array.
When switching from the per-semaphore locks to the big lock, all
per-semaphore locks must be scanned for ongoing operations.

The patch adds a hysteresis for switching from the big lock to the per
semaphore locks. This reduces how often the per-semaphore locks must
be scanned.

Isn't this very arbitrary depending on the workload? Ie the other way around:
when we have a lot more simple ops going on not so good. While I'm more worried
about combinations that could cause enough complex ops to always delay taking
the finer grained lock, this change also obviously makes simple ops more expensive
on newly created segments.
I

Entering complex mode requires a scan of sem_base[].sem_lock.
for (i = 0; i < sma->sem_nsems; i++) {
sem = sma->sem_base + i;
spin_unlock_wait(&sem->lock);
This is what the patch tries to avoid.

In general I don't trust magic numbers much. What sort of numbers have you seen
with this patch? Is this a real concern (particularly because a lot of the sem->lock
work was because real world workloads were doing a lot more simple ops afaicr)?

With a microbenchmark: As much improvement as you want :-)

- Only simple ops: patch has no impact (the first 10 semops do not matter)
- sleeping complex ops: patch has no impact, we are always in complex mode
- not sleeping complex ops: depends on the size of the array.
With a 4.000 semaphore array, I see an improvement of factor 20.

There is obviously one case where the patch causes a slowdown:
- complex op, then 11 simple ops, then repeat.


Perhaps: set COMPLEX_MODE_ENTER to 1 or 2, then allow to configure it from user space.
Or do not merge the patch and wait until someone come with a profile that shows complexmode_enter().

--
Manfred