[patch 2.5.51] add wait_event() to <linux/completion.h>

[patch 2.5.51] add wait_event() to <linux/completion.h>

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

Folk periodically talk about evolving the "struct completion"
support ... here's one:

   int wait_event (struct completion *x, signed long timeout_jiffies);

Returns zero if the event happened, nonzero if it timed out.
An example timeout action might be to cancel a request then
use wait_for_completion(x) to synchronize with that.

It's been behaving for me so far.  Questions from me:

- Is this appropriate to merge as-is?  The tradeoff I'm
   thinking of is code duplication; this patch avoids it
   ("smaller==better" rule-of-thumb) but maybe someone has
   real tuning knowledge, or a "standard" lk policy applies.
   Or there might be bugs or api transgressions.

- One routine was unavailable to modules; now exported.
   That seemed to just be an oversight.

This doesn't add an "interruptible wait" api mode, which
I'd expect some others might find a use for.  This patch
will let us clean up some dubious code in usbcore which
is more or less trying to do a wait_event().

Thanks in advance for any comments.

- Dave

[-- Attachment #2: sched.patch --]
[-- Type: text/plain, Size: 1730 bytes --]

--- ./include/linux-dist/completion.h	Sun Dec  8 10:57:47 2002
+++ ./include/linux/completion.h	Mon Dec  9 15:11:51 2002
@@ -28,6 +28,7 @@
 }
 
 extern void FASTCALL(wait_for_completion(struct completion *));
+extern int FASTCALL(wait_timeout(struct completion *, signed long jiffies));
 extern void FASTCALL(complete(struct completion *));
 extern void FASTCALL(complete_all(struct completion *));
 
--- ./kernel-dist/ksyms.c	Sun Dec  8 10:57:48 2002
+++ ./kernel/ksyms.c	Mon Dec  9 15:13:47 2002
@@ -404,7 +404,9 @@ EXPORT_SYMBOL(autoremove_wake_function);
 
 /* completion handling */
 EXPORT_SYMBOL(wait_for_completion);
+EXPORT_SYMBOL(wait_timeout);
 EXPORT_SYMBOL(complete);
+EXPORT_SYMBOL(complete_all);
 
 /* The notion of irq probe/assignment is foreign to S/390 */
 
--- ./kernel-dist/sched.c	Sun Dec  8 10:57:48 2002
+++ ./kernel/sched.c	Mon Dec  9 15:34:36 2002
@@ -1204,6 +1204,11 @@ void complete_all(struct completion *x)
 
 void wait_for_completion(struct completion *x)
 {
+	wait_timeout (x, MAX_SCHEDULE_TIMEOUT);
+}
+
+int wait_timeout(struct completion *x, signed long timeout_jiffies)
+{
 	might_sleep();
 	spin_lock_irq(&x->wait.lock);
 	if (!x->done) {
@@ -1214,13 +1219,19 @@ void wait_for_completion(struct completi
 		do {
 			__set_current_state(TASK_UNINTERRUPTIBLE);
 			spin_unlock_irq(&x->wait.lock);
-			schedule();
+			timeout_jiffies = schedule_timeout(timeout_jiffies);
+			if (timeout_jiffies == 0) {
+				__remove_wait_queue(&x->wait, &wait);
+				/* caller should wait again */
+				return 1;
+			}
 			spin_lock_irq(&x->wait.lock);
 		} while (!x->done);
 		__remove_wait_queue(&x->wait, &wait);
 	}
 	x->done--;
 	spin_unlock_irq(&x->wait.lock);
+	return 0;
 }
 
 #define	SLEEP_ON_VAR				\

Re: [patch 2.5.51] add wait_event() to <linux/completion.h>

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 167 bytes --]

Milton D. Miller II wrote:
>  __remove_wait_queue(&x->wait, &wait); 
> 
> should be under 
> spin_lock_irq(&x->wait.lock); 

Duh!  Updated patch is attached.

- Dave


[-- Attachment #2: sched2.patch --]
[-- Type: text/plain, Size: 1724 bytes --]

--- ./include/linux-dist/completion.h	Mon Dec  9 23:49:44 2002
+++ ./include/linux/completion.h	Tue Dec 10 09:35:57 2002
@@ -28,6 +28,7 @@
 }
 
 extern void FASTCALL(wait_for_completion(struct completion *));
+extern int FASTCALL(wait_timeout(struct completion *, signed long jiffies));
 extern void FASTCALL(complete(struct completion *));
 extern void FASTCALL(complete_all(struct completion *));
 
--- ./kernel-dist/ksyms.c	Thu Dec 12 10:24:44 2002
+++ ./kernel/ksyms.c	Tue Dec 10 09:35:57 2002
@@ -404,7 +404,9 @@ EXPORT_SYMBOL(autoremove_wake_function);
 
 /* completion handling */
 EXPORT_SYMBOL(wait_for_completion);
+EXPORT_SYMBOL(wait_timeout);
 EXPORT_SYMBOL(complete);
+EXPORT_SYMBOL(complete_all);
 
 /* The notion of irq probe/assignment is foreign to S/390 */
 
--- ./kernel-dist/sched.c	Thu Dec 12 10:24:44 2002
+++ ./kernel/sched.c	Thu Dec 12 10:15:56 2002
@@ -1204,6 +1204,11 @@ void complete_all(struct completion *x)
 
 void wait_for_completion(struct completion *x)
 {
+	(void) wait_timeout (x, MAX_SCHEDULE_TIMEOUT);
+}
+
+int wait_timeout(struct completion *x, signed long timeout)
+{
 	might_sleep();
 	spin_lock_irq(&x->wait.lock);
 	if (!x->done) {
@@ -1214,13 +1219,18 @@ void wait_for_completion(struct completi
 		do {
 			__set_current_state(TASK_UNINTERRUPTIBLE);
 			spin_unlock_irq(&x->wait.lock);
-			schedule();
+			timeout = schedule_timeout(timeout);
 			spin_lock_irq(&x->wait.lock);
-		} while (!x->done);
+		} while (!x->done && timeout != 0);
 		__remove_wait_queue(&x->wait, &wait);
 	}
-	x->done--;
+	if (x->done) {
+		timeout = 1;
+		x->done--;
+	}
 	spin_unlock_irq(&x->wait.lock);
+	/* nonzero return means we timed out */
+	return timeout == 0;
 }
 
 #define	SLEEP_ON_VAR				\