ipc/sem.c: sem use list operations
Nick Piggin [Wed, 16 Dec 2009 00:47:29 +0000 (16:47 -0800)]
Replace the handcoded list operations in update_queue() with the standard
list_for_each_entry macros.

list_for_each_entry_safe() must be used, because list entries can
disappear immediately uppon the wakeup event.

Signed-off-by: Nick Piggin <npiggin@suse.de>
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Cc: Pierre Peiffer <peifferp@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

ipc/sem.c

index cb0070e..d377b3a 100644 (file)
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -403,58 +403,45 @@ undo:
  */
 static void update_queue (struct sem_array * sma)
 {
-       int error;
-       struct sem_queue * q;
+       struct sem_queue *q, *tq;
+
+again:
+       list_for_each_entry_safe(q, tq, &sma->sem_pending, list) {
+               int error;
+               int alter;
 
-       q = list_entry(sma->sem_pending.next, struct sem_queue, list);
-       while (&q->list != &sma->sem_pending) {
                error = try_atomic_semop(sma, q->sops, q->nsops,
                                         q->undo, q->pid);
 
                /* Does q->sleeper still need to sleep? */
-               if (error <= 0) {
-                       struct sem_queue *n;
+               if (error > 0)
+                       continue;
 
-                       /*
-                        * Continue scanning. The next operation
-                        * that must be checked depends on the type of the
-                        * completed operation:
-                        * - if the operation modified the array, then
-                        *   restart from the head of the queue and
-                        *   check for threads that might be waiting
-                        *   for semaphore values to become 0.
-                        * - if the operation didn't modify the array,
-                        *   then just continue.
-                        * The order of list_del() and reading ->next
-                        * is crucial: In the former case, the list_del()
-                        * must be done first [because we might be the
-                        * first entry in ->sem_pending], in the latter
-                        * case the list_del() must be done last
-                        * [because the list is invalid after the list_del()]
-                        */
-                       if (q->alter) {
-                               list_del(&q->list);
-                               n = list_entry(sma->sem_pending.next,
-                                               struct sem_queue, list);
-                       } else {
-                               n = list_entry(q->list.next, struct sem_queue,
-                                               list);
-                               list_del(&q->list);
-                       }
+               list_del(&q->list);
 
-                       /* wake up the waiting thread */
-                       q->status = IN_WAKEUP;
+               /*
+                * The next operation that must be checked depends on the type
+                * of the completed operation:
+                * - if the operation modified the array, then restart from the
+                *   head of the queue and check for threads that might be
+                *   waiting for semaphore values to become 0.
+                * - if the operation didn't modify the array, then just
+                *   continue.
+                */
+               alter = q->alter;
+
+               /* wake up the waiting thread */
+               q->status = IN_WAKEUP;
 
-                       wake_up_process(q->sleeper);
-                       /* hands-off: q will disappear immediately after
-                        * writing q->status.
-                        */
-                       smp_wmb();
-                       q->status = error;
-                       q = n;
-               } else {
-                       q = list_entry(q->list.next, struct sem_queue, list);
-               }
+               wake_up_process(q->sleeper);
+               /* hands-off: q will disappear immediately after
+                * writing q->status.
+                */
+               smp_wmb();
+               q->status = error;
+
+               if (alter)
+                       goto again;
        }
 }