Re: [2/4] consolidate bit waiting code patterns

[William Lee Irwin III <wli@holomorphy.com>]

On Sat, Aug 28, 2004 at 02:34:36AM -0700, Andrew Morton wrote:
> Some comments over this thing would be nice.
> The `wait' argument seems to be misnamed.  It typically points at
> sync_page(), yes?   Maybe it should be called `action' or `kick' or
> something.
> If (*wait)() returns non-zero then it looks to me like the callers will get
> confused, thinking that the page did come unlocked?
> If (*wait)() returns non-zero then we need to run finish_wait(), no?

You're right, that should be a break; and return ret; with ret
initialized to 0. Mysteriously, I never hit this even in combination
testing with aio.

Index: mm1-2.6.9-rc1/kernel/fork.c
===================================================================
--- mm1-2.6.9-rc1.orig/kernel/fork.c	2004-08-28 01:23:05.410362856 -0700
+++ mm1-2.6.9-rc1/kernel/fork.c	2004-08-28 02:47:05.712120520 -0700
@@ -261,37 +261,40 @@
 }
 EXPORT_SYMBOL(__wake_up_bit);
 
+/*
+ * To allow interruptible waiting and asynchronous (i.e. nonblocking)
+ * waiting, the actions of __wait_on_bit() and __wait_on_bit_lock() are
+ * permitted return codes. Nonzero return codes halt waiting and return.
+ */
 int __sched __wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
 			void *word,
-			int bit, int (*wait)(void *), unsigned mode)
+			int bit, int (*action)(void *), unsigned mode)
 {
-	int ret;
+	int ret = 0;
 
 	prepare_to_wait(wq, &q->wait, mode);
-	if (test_bit(bit, word)) {
-		if ((ret = (*wait)(word)))
-			return ret;
-	}
+	if (test_bit(bit, word))
+		ret = (*action)(word);
 	finish_wait(wq, &q->wait);
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(__wait_on_bit);
 
 int __sched __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
 			void *word, int bit,
-			int (*wait)(void *), unsigned mode)
+			int (*action)(void *), unsigned mode)
 {
-	int ret;
+	int ret = 0;
 
 	while (test_and_set_bit(bit, word)) {
 		prepare_to_wait_exclusive(wq, &q->wait, mode);
 		if (test_bit(bit, word)) {
-			if ((ret = (*wait)(word)))
-				return ret;
+			if ((ret = (*action)(word)))
+				break;
 		}
 	}
 	finish_wait(wq, &q->wait);
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(__wait_on_bit_lock);
 
Index: mm1-2.6.9-rc1/include/linux/wait.h
===================================================================
--- mm1-2.6.9-rc1.orig/include/linux/wait.h	2004-08-28 01:44:13.448591744 -0700
+++ mm1-2.6.9-rc1/include/linux/wait.h	2004-08-28 02:45:22.458817408 -0700
@@ -343,7 +343,7 @@
  * wait_on_bit - wait for a bit to be cleared
  * @word: the word being waited on, a kernel virtual address
  * @bit: the bit of the word being waited on
- * @wait: the function used to sleep, which may take special actions
+ * @action: the function used to sleep, which may take special actions
  * @mode: the task state to sleep in
  *
  * There is a standard hashed waitqueue table for generic use. This
@@ -354,19 +354,19 @@
  * but has no intention of setting it.
  */
 static inline int wait_on_bit(void *word, int bit,
-				int (*wait)(void *), unsigned mode)
+				int (*action)(void *), unsigned mode)
 {
 	DEFINE_WAIT_BIT(q, word, bit);
 	wait_queue_head_t *wqh = bit_waitqueue(word, bit);
 
-	return __wait_on_bit(wqh, &q, word, bit, wait, mode);
+	return __wait_on_bit(wqh, &q, word, bit, action, mode);
 }
 
 /**
  * wait_on_bit_lock - wait for a bit to be cleared, when wanting to set it
  * @word: the word being waited on, a kernel virtual address
  * @bit: the bit of the word being waited on
- * @wait: the function used to sleep, which may take special actions
+ * @action: the function used to sleep, which may take special actions
  * @mode: the task state to sleep in
  *
  * There is a standard hashed waitqueue table for generic use. This
@@ -379,12 +379,12 @@
  * clear with the intention of setting it, and when done, clearing it.
  */
 static inline int wait_on_bit_lock(void *word, int bit,
-				int (*wait)(void *), unsigned mode)
+				int (*action)(void *), unsigned mode)
 {
 	DEFINE_WAIT_BIT(q, word, bit);
 	wait_queue_head_t *wqh = bit_waitqueue(word, bit);
 
-	return __wait_on_bit_lock(wqh, &q, word, bit, wait, mode);
+	return __wait_on_bit_lock(wqh, &q, word, bit, action, mode);
 }
 	
 #endif /* __KERNEL__ */