Hi Davidlohr,
On 09/12/2016 01:53 PM, Davidlohr Bueso wrote:
Hmean sembench-sem-482 965735.00 ( 0.00%) 1040313.00 ( 7.72%)
[...]
Signed-off-by: Davidlohr Bueso <dbu...@suse.de>
---
ipc/sem.c | 268 +++++++++++++++++++-------------------------------------------
1 file changed, 83 insertions(+), 185 deletions(-)
Nice speedup, nice amount of removed duplicated code.
diff --git a/ipc/sem.c b/ipc/sem.c
index a4e8bb2fae38..86467b5b78ad 100644
[...]
- error = get_queue_result(&queue);
-
- if (error != -EINTR) {
- /* fast path: update_queue already obtained all requested
- * resources.
- * Perform a smp_mb(): User space could assume that semop()
- * is a memory barrier: Without the mb(), the cpu could
- * speculatively read in user space stale data that was
- * overwritten by the previous owner of the semaphore.
+ /*
+ * fastpath: the semop has completed, either successfully or not, from
+ * the syscall pov, is quite irrelevant to us at this point; we're done.
+ *
+ * We _do_ care, nonetheless, about being awoken by a signal or
spuriously.
+ * The queue.status is checked again in the slowpath (aka after taking
+ * sem_lock), such that we can detect scenarios where we were awakened
+ * externally, during the window between wake_q_add() and wake_up_q().
+ */
+ if ((error = queue.status) != -EINTR && !signal_pending(current)) {
+ /*
+ * User space could assume that semop() is a memory barrier:
+ * Without the mb(), the cpu could speculatively read in user
+ * space stale data that was overwritten by the previous owner
+ * of the semaphore.
*/
smp_mb();
-
goto out_free;
}
rcu_read_lock();
sma = sem_obtain_lock(ns, semid, sops, nsops, &locknum);
What is the purpose of the !signal_pending()?
Even if there is a signal: If someone has set queue.status, then our
semaphore operations completed and we must return that result code.
Obviously: At syscall return, the syscall return code will notice the
pending signal and immediately the signal handler is called, but I don't
see that this prevents us from using the fast path.
And, at least my opinion: I would avoid placing the error= into the if().
--
Manfred
--
