[PATCH] fix missing wakeup in ipc/sem

[Manfred Spraul]

Hi Andrew,

My patch that removed the spin_lock calls from the tail of
sys_semtimedop introduced a bug:
Before my patch was merged, every operation that altered an array called
update_queue. That call woke up threads that were waiting until a
semaphore value becomes 0. I've accidentially removed that call.

The attached patch fixes that by modifying update_queue: the function
now loops internally and wakes up all threads. The patch also removes
update_queue calls from the error path of sys_semtimedop: failed
operations do not modify the array, no need to rescan the list of
waiting threads.

Signed-Off-By: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>



// $Header$
// Kernel Version:
//  VERSION = 2
//  PATCHLEVEL = 6
//  SUBLEVEL = 10
//  EXTRAVERSION =-rc2-mm4
--- 2.6/include/linux/sem.h	2004-12-05 16:20:31.000000000 +0100
+++ build-2.6/include/linux/sem.h	2004-12-09 18:39:58.000000000 +0100
@@ -109,6 +109,7 @@ struct sem_queue {
 	int			id;	 /* internal sem id */
 	struct sembuf *		sops;	 /* array of pending operations */
 	int			nsops;	 /* number of operations */
+	int			alter;   /* does the operation alter the array? */
 };
 
 /* Each task has a list of undo requests. They are executed automatically
--- 2.6/ipc/sem.c	2004-12-05 16:21:39.000000000 +0100
+++ build-2.6/ipc/sem.c	2004-12-09 19:13:19.000000000 +0100
@@ -358,8 +358,22 @@ static void update_queue (struct sem_arr
 		if (error <= 0) {
 			struct sem_queue *n;
 			remove_from_queue(sma,q);
-			n = q->next;
 			q->status = IN_WAKEUP;
+			/*
+			 * 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.
+			 */
+			if (q->alter)
+				n = sma->sem_pending;
+			else
+				n = q->next;
 			wake_up_process(q->sleeper);
 			/* hands-off: q will disappear immediately after
 			 * writing q->status.
@@ -1119,8 +1133,11 @@ retry_undos:
 		goto out_unlock_free;
 
 	error = try_atomic_semop (sma, sops, nsops, un, current->tgid);
-	if (error <= 0)
-		goto update;
+	if (error <= 0) {
+		if (alter && error == 0)
+			update_queue (sma);
+		goto out_unlock_free;
+	}
 
 	/* We need to sleep on this operation, so we put the current
 	 * task into the pending queue and go to sleep.
@@ -1132,6 +1149,7 @@ retry_undos:
 	queue.undo = un;
 	queue.pid = current->tgid;
 	queue.id = semid;
+	queue.alter = alter;
 	if (alter)
 		append_to_queue(sma ,&queue);
 	else
@@ -1183,9 +1201,6 @@ retry_undos:
 	remove_from_queue(sma,&queue);
 	goto out_unlock_free;
 
-update:
-	if (alter)
-		update_queue (sma);
 out_unlock_free:
 	sem_unlock(sma);
 out_free: