ipc/sem: simplify wait-wake loop
authorDavidlohr Bueso <dave@stgolabs.net>
Wed, 14 Dec 2016 23:06:46 +0000 (15:06 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 15 Dec 2016 00:04:08 +0000 (16:04 -0800)
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.

[akpm@linux-foundation.org: coding-style fixes]
Link: http://lkml.kernel.org/r/1478708774-28826-2-git-send-email-dave@stgolabs.net
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Cc: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
ipc/sem.c

index 4f5af6e7d6303586f3496de736007ced7cb7a9d2..a82c88d0900f307435985d1ee3504f29188ed2c5 100644 (file)
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1963,71 +1963,67 @@ 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 userspace 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);