From 8a111715cf3a3890f5bb72998488c08e4427a8ce Mon Sep 17 00:00:00 2001 From: Davidlohr Bueso Date: Thu, 24 Nov 2016 09:24:18 +1100 Subject: [PATCH] ipc/sem: simplify wait-wake loop Instead of using the reverse goto, we can simplify the flow and make it more language natural by just doing do-while instead. One would hope this is the standard way (or obviously just with a while bucle) that we do wait/wakeup handling in the kernel. The exact same logic is kept, just more indented. Link: http://lkml.kernel.org/r/1478708774-28826-2-git-send-email-dave@stgolabs.net Signed-off-by: Davidlohr Bueso Cc: Manfred Spraul Signed-off-by: Andrew Morton --- ipc/sem.c | 107 ++++++++++++++++++++++++++---------------------------- 1 file changed, 51 insertions(+), 56 deletions(-) diff --git a/ipc/sem.c b/ipc/sem.c index 77c011e90414c3..c8b9b7204aa7d6 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -1980,71 +1980,66 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, sma->complex_count++; } -sleep_again: - queue.status = -EINTR; - queue.sleeper = current; + do { + queue.status = -EINTR; + queue.sleeper = current; - __set_current_state(TASK_INTERRUPTIBLE); - sem_unlock(sma, locknum); - rcu_read_unlock(); + __set_current_state(TASK_INTERRUPTIBLE); + sem_unlock(sma, locknum); + rcu_read_unlock(); - if (timeout) - jiffies_left = schedule_timeout(jiffies_left); - else - schedule(); + if (timeout) + jiffies_left = schedule_timeout(jiffies_left); + else + schedule(); - /* - * 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(). - */ - error = READ_ONCE(queue.status); - if (error != -EINTR) { /* - * 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(). */ - smp_mb(); - goto out_free; - } - - rcu_read_lock(); - sma = sem_obtain_lock(ns, semid, sops, nsops, &locknum); - error = READ_ONCE(queue.status); + error = READ_ONCE(queue.status); + if (error != -EINTR) { + /* + * 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; + } - /* - * Array removed? If yes, leave without sem_unlock(). - */ - if (IS_ERR(sma)) { - rcu_read_unlock(); - goto out_free; - } + rcu_read_lock(); + sma = sem_obtain_lock(ns, semid, sops, nsops, &locknum); + error = READ_ONCE(queue.status); - /* - * If queue.status != -EINTR we are woken up by another process. - * Leave without unlink_queue(), but with sem_unlock(). - */ - if (error != -EINTR) - goto out_unlock_free; + /* + * Array removed? If yes, leave without sem_unlock(). + */ + if (IS_ERR(sma)) { + rcu_read_unlock(); + goto out_free; + } - /* - * If an interrupt occurred we have to clean up the queue. - */ - if (timeout && jiffies_left == 0) - error = -EAGAIN; + /* + * If queue.status != -EINTR we are woken up by another process. + * Leave without unlink_queue(), but with sem_unlock(). + */ + if (error != -EINTR) + goto out_unlock_free; - /* - * If the wakeup was spurious, just retry. - */ - if (error == -EINTR && !signal_pending(current)) - goto sleep_again; + /* + * If an interrupt occurred we have to clean up the queue. + */ + if (timeout && jiffies_left == 0) + error = -EAGAIN; + } while (error == -EINTR && !signal_pending(current)); /* spurious */ unlink_queue(sma, &queue);