From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934150AbcKIQ10 (ORCPT ); Wed, 9 Nov 2016 11:27:26 -0500 Received: from smtp2.provo.novell.com ([137.65.250.81]:50550 "EHLO smtp2.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753107AbcKIQ1Y (ORCPT ); Wed, 9 Nov 2016 11:27:24 -0500 From: Davidlohr Bueso To: akpm@linux-foundation.org, manfred@colorfullife.com Cc: linux-kernel@vger.kernel.org, dave@stgolabs.net, Davidlohr Bueso Subject: [PATCH -next 1/2] ipc/sem: simplify wait-wake loop Date: Wed, 9 Nov 2016 08:26:13 -0800 Message-Id: <1478708774-28826-2-git-send-email-dave@stgolabs.net> X-Mailer: git-send-email 2.6.6 In-Reply-To: <1478708774-28826-1-git-send-email-dave@stgolabs.net> References: <1478708774-28826-1-git-send-email-dave@stgolabs.net> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. Signed-off-by: Davidlohr Bueso --- ipc/sem.c | 107 ++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 51 insertions(+), 56 deletions(-) diff --git a/ipc/sem.c b/ipc/sem.c index ebd18a7104fd..a5eaf517c8b4 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); -- 2.6.6