Messages in this thread Patch in this message | | | Subject | Re: [PATCH 3/5] ipc/sem: optimize perform_atomic_semop() | From | Manfred Spraul <> | Date | Mon, 19 Sep 2016 06:41:47 +0200 |
| |
Hi Davidlohr,
On 09/13/2016 10:33 AM, Davidlohr Bueso wrote: > >> >>> @@ -1751,12 +1820,17 @@ SYSCALL_DEFINE4(semtimedop, int, semid, >>> struct sembuf __user *, tsops, >>> if (sop->sem_num >= max) >>> max = sop->sem_num; >>> if (sop->sem_flg & SEM_UNDO) >>> - undos = 1; >>> + undos = true; >>> if (sop->sem_op != 0) >>> - alter = 1; >>> + alter = true; >>> + if (sop->sem_num < SEMOPM_FAST && !dupsop) { >>> + if (dup & (1 << sop->sem_num)) >>> + dupsop = 1; >>> + else >>> + dup |= 1 << sop->sem_num; >>> + } >>> } >> At least for nsops=2, sops[0].sem_num !=sops[1].sem_num can detect >> absense of duplicated ops regardless of the array size. >> Should we support that? > > There are various individual cases like that (ie obviously nsops == 1, > alter == 0, etc) > where the dup detection would be unnecessary, but it seems like a > stretch to go > at it like this. The above will work on the common case (assuming > lower sem_num > of course). So I'm not particularly worried about being too smart at > the dup detection. > What about the attached dup detection?
-- Manfred From 140340a358dbf66b3bc6f848ca9b860e3e957e84 Mon Sep 17 00:00:00 2001 From: Manfred Spraul <manfred@colorfullife.com> Date: Mon, 19 Sep 2016 06:25:20 +0200 Subject: [PATCH] ipc/sem: Update duplicate sop detection
The duplicated sop detection can be improved: - use uint64_t instead of unsigned long for the bit array storage, otherwise we break 32-bit archs - support large arrays, just interpret the bit array as a hash array (i.e.: an operation that accesses semaphore 0 and 64 would trigger the dupsop code, but that is far better than not trying at all for semnum >=64) - support test-for-zero-and-increase, this case can use the fast codepath.
Untested! S-O-B only for the code, needs testing.
Signed-off-by: Manfred Spraul <manfred@colorfullife.com> --- ipc/sem.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/ipc/sem.c b/ipc/sem.c index d9c743a..eda9e46 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -1784,7 +1784,8 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, int max, locknum; bool undos = false, alter = false, dupsop = false; struct sem_queue queue; - unsigned long dup = 0, jiffies_left = 0; + unsigned long jiffies_left = 0; + uint64_t dup; struct ipc_namespace *ns; ns = current->nsproxy->ipc_ns; @@ -1816,18 +1817,32 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, jiffies_left = timespec_to_jiffies(&_timeout); } max = 0; + + dup = 0; for (sop = sops; sop < sops + nsops; sop++) { + uint64_t mask; + if (sop->sem_num >= max) max = sop->sem_num; if (sop->sem_flg & SEM_UNDO) undos = true; - if (sop->sem_op != 0) + + /* 64: BITS_PER_UNIT64 */ + mask = 1<<((sop->sem_num)%64); + + if (dup & mask) { + /* + * There was a previous alter access that appears + * to have accessed the same semaphore, thus + * use the dupsop logic. + * "appears", because the detection can only check + * % BITS_PER_UNIT64. + */ + dupsop = 1; + } + if (sop->sem_op != 0) { alter = true; - if (sop->sem_num < SEMOPM_FAST && !dupsop) { - if (dup & (1 << sop->sem_num)) - dupsop = 1; - else - dup |= 1 << sop->sem_num; + dup |= mask; } } -- 2.7.4
| |