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 <[email protected]>
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 <[email protected]>
---
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
