2.6.9-rc1-mm2 : compilation error in kernel/wait.c

2.6.9-rc1-mm2 : compilation error in kernel/wait.c

[Eric Valette]

kernel/wait.c:156: error: conflicting types for '__wait_on_bit'
include/linux/wait.h:143: error: previous declaration of '__wait_on_bit' 
was here
kernel/wait.c:156: error: conflicting types for '__wait_on_bit'
include/linux/wait.h:143: error: previous declaration of '__wait_on_bit' 
was here
kernel/wait.c:170: error: conflicting types for '__wait_on_bit_lock'
include/linux/wait.h:144: error: previous declaration of 
'__wait_on_bit_lock' was here
kernel/wait.c:170: error: conflicting types for '__wait_on_bit_lock'
include/linux/wait.h:144: error: previous declaration of 
'__wait_on_bit_lock' was here
make[2]: *** [kernel/wait.o] Error 1
make[1]: *** [kernel] Error 2

-- 
    __
   /  `                   	Eric Valette
  /--   __  o _.          	6 rue Paul Le Flem
(___, / (_(_(__         	35740 Pace

Tel: +33 (0)2 99 85 26 76	Fax: +33 (0)2 99 85 26 76
E-mail: eric.valette@free.fr

Re: 2.6.9-rc1-mm2 : compilation error in kernel/wait.c

[William Lee Irwin III]

On Tue, Aug 31, 2004 at 10:05:10AM +0200, Eric Valette wrote:
> kernel/wait.c:156: error: conflicting types for '__wait_on_bit'
> include/linux/wait.h:143: error: previous declaration of '__wait_on_bit' 
> was here
> kernel/wait.c:156: error: conflicting types for '__wait_on_bit'
> include/linux/wait.h:143: error: previous declaration of '__wait_on_bit' 
> was here
> kernel/wait.c:170: error: conflicting types for '__wait_on_bit_lock'
> include/linux/wait.h:144: error: previous declaration of 
> '__wait_on_bit_lock' was here
> kernel/wait.c:170: error: conflicting types for '__wait_on_bit_lock'
> include/linux/wait.h:144: error: previous declaration of 
> '__wait_on_bit_lock' was here
> make[2]: *** [kernel/wait.o] Error 1
> make[1]: *** [kernel] Error 2

I very distinctly recall compiling and booting these...


-- wli

Re: 2.6.9-rc1-mm2 : compilation error in kernel/wait.c

[Andrew Morton]

Eric Valette <eric.valette@free.fr> wrote:
>
> kernel/wait.c:156: error: conflicting types for '__wait_on_bit'
>  include/linux/wait.h:143: error: previous declaration of '__wait_on_bit' 
>  was here
>  kernel/wait.c:156: error: conflicting types for '__wait_on_bit'
>  include/linux/wait.h:143: error: previous declaration of '__wait_on_bit' 
>  was here
>  kernel/wait.c:170: error: conflicting types for '__wait_on_bit_lock'
>  include/linux/wait.h:144: error: previous declaration of 
>  '__wait_on_bit_lock' was here
>  kernel/wait.c:170: error: conflicting types for '__wait_on_bit_lock'
>  include/linux/wait.h:144: error: previous declaration of 
>  '__wait_on_bit_lock' was here

Try this:



diff -puN kernel/wait.c~__wait_on_bit-fix kernel/wait.c
--- linux-bix/kernel/wait.c~__wait_on_bit-fix	2004-08-31 01:11:02.279382800 -0700
+++ linux-bix-akpm/kernel/wait.c	2004-08-31 01:12:33.856460968 -0700
@@ -150,9 +150,9 @@ EXPORT_SYMBOL(wake_bit_function);
  * 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 (*action)(void *), unsigned mode)
+int __sched fastcall
+__wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
+		void *word, int bit, int (*action)(void *), unsigned mode)
 {
 	int ret = 0;
 
@@ -164,9 +164,9 @@ int __sched __wait_on_bit(wait_queue_hea
 }
 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 (*action)(void *), unsigned mode)
+int __sched fastcall
+__wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
+		void *word, int bit, int (*action)(void *), unsigned mode)
 {
 	int ret = 0;
 
_

Re: 2.6.9-rc1-mm2 : compilation error in kernel/wait.c

[William Lee Irwin III]

On Tue, Aug 31, 2004 at 01:09:16AM -0700, William Lee Irwin III wrote:
> I very distinctly recall compiling and booting these...

Index: mm2-2.6.9-rc1/kernel/wait.c
===================================================================
--- mm2-2.6.9-rc1.orig/kernel/wait.c	2004-08-31 01:06:56.826462592 -0700
+++ mm2-2.6.9-rc1/kernel/wait.c	2004-08-31 01:10:49.805044464 -0700
@@ -150,8 +150,8 @@
  * 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 __sched fastcall __wait_on_bit(wait_queue_head_t *wq,
+			struct wait_bit_queue *q, void *word,
 			int bit, int (*action)(void *), unsigned mode)
 {
 	int ret = 0;
@@ -164,8 +164,8 @@
 }
 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 __sched fastcall __wait_on_bit_lock(wait_queue_head_t *wq,
+			struct wait_bit_queue *q, void *word, int bit,
 			int (*action)(void *), unsigned mode)
 {
 	int ret = 0;

wait_on_bit_lock() must test_and_set_bit(), not test_bit()

[William Lee Irwin III]

On Tue, Aug 31, 2004 at 01:09:16AM -0700, William Lee Irwin III wrote:
>> I very distinctly recall compiling and booting these...

On Tue, Aug 31, 2004 at 01:14:56AM -0700, William Lee Irwin III wrote:
> Index: mm2-2.6.9-rc1/kernel/wait.c

Incremental atop the fastcall fix:

wait_on_bit_lock() needs to test_and_set_bit() in the fastpath, not
test_bit().


-- wli

Index: mm2-2.6.9-rc1/include/linux/wait.h
===================================================================
--- mm2-2.6.9-rc1.orig/include/linux/wait.h	2004-08-31 01:37:55.507900064 -0700
+++ mm2-2.6.9-rc1/include/linux/wait.h	2004-08-31 01:38:34.670946376 -0700
@@ -388,7 +388,7 @@
 	DEFINE_WAIT_BIT(q, word, bit);
 	wait_queue_head_t *wqh;
 
-	if (!test_bit(bit, word))
+	if (!test_and_set_bit(bit, word))
 		return 0;
 
 	wqh = bit_waitqueue(word, bit);

[1/2] move wait ops' contention case completely out of line

[William Lee Irwin III]

On Tue, Aug 31, 2004 at 01:44:58AM -0700, William Lee Irwin III wrote:
> Incremental atop the fastcall fix:
> wait_on_bit_lock() needs to test_and_set_bit() in the fastpath, not
> test_bit().

Move the slow paths of wait_on_bit() and wait_on_bit_lock() out of line.
Also uninline wake_up_bit() to reduce the number of callsites
generated, and adjust loop startup in  __wait_on_bit_lock() to properly
reflect its usage in the contention case.

Incremental atop the fastcall and wait_on_bit_lock()/test_and_set_bit() fixes.
Successfully tested on x86-64.


Index: mm2-2.6.9-rc1/include/linux/wait.h
===================================================================
--- mm2-2.6.9-rc1.orig/include/linux/wait.h	2004-08-31 01:38:34.670946376 -0700
+++ mm2-2.6.9-rc1/include/linux/wait.h	2004-08-31 01:50:40.947535552 -0700
@@ -142,23 +142,11 @@
 void FASTCALL(__wake_up_bit(wait_queue_head_t *, void *, int));
 int FASTCALL(__wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, void *, int, int (*)(void *), unsigned));
 int FASTCALL(__wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, void *, int, int (*)(void *), unsigned));
+void FASTCALL(wake_up_bit(void *, int));
+int FASTCALL(out_of_line_wait_on_bit(void *, int, int (*)(void *), unsigned));
+int FASTCALL(out_of_line_wait_on_bit_lock(void *, int, int (*)(void *), unsigned));
 wait_queue_head_t *FASTCALL(bit_waitqueue(void *, int));
 
-/**
- * wake_up_bit - wake up a waiter on a bit
- * @word: the word being waited on, a kernel virtual address
- * @bit: the bit of the word being waited on
- *
- * There is a standard hashed waitqueue table for generic use. This
- * is the part of the hashtable's accessor API that wakes up waiters
- * on a bit. For instance, if one were to have waiters on a bitflag,
- * one would call wake_up_bit() after clearing the bit.
- */
-static inline void wake_up_bit(void *word, int bit)
-{
-	__wake_up_bit(bit_waitqueue(word, bit), word, bit);
-}
-
 #define wake_up(x)			__wake_up(x, TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 1, NULL)
 #define wake_up_nr(x, nr)		__wake_up(x, TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, nr, NULL)
 #define wake_up_all(x)			__wake_up(x, TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 0, NULL)
@@ -356,14 +344,9 @@
 static inline int wait_on_bit(void *word, int bit,
 				int (*action)(void *), unsigned mode)
 {
-	DEFINE_WAIT_BIT(q, word, bit);
-	wait_queue_head_t *wqh;
-
 	if (!test_bit(bit, word))
 		return 0;
-
-	wqh = bit_waitqueue(word, bit);
-	return __wait_on_bit(wqh, &q, word, bit, action, mode);
+	return out_of_line_wait_on_bit(word, bit, action, mode);
 }
 
 /**
@@ -385,14 +368,9 @@
 static inline int wait_on_bit_lock(void *word, int bit,
 				int (*action)(void *), unsigned mode)
 {
-	DEFINE_WAIT_BIT(q, word, bit);
-	wait_queue_head_t *wqh;
-
 	if (!test_and_set_bit(bit, word))
 		return 0;
-
-	wqh = bit_waitqueue(word, bit);
-	return __wait_on_bit_lock(wqh, &q, word, bit, action, mode);
+	return out_of_line_wait_on_bit_lock(word, bit, action, mode);
 }
 	
 #endif /* __KERNEL__ */
Index: mm2-2.6.9-rc1/kernel/wait.c
===================================================================
--- mm2-2.6.9-rc1.orig/kernel/wait.c	2004-08-31 01:10:49.805044464 -0700
+++ mm2-2.6.9-rc1/kernel/wait.c	2004-08-31 01:51:18.966755752 -0700
@@ -164,24 +164,44 @@
 }
 EXPORT_SYMBOL(__wait_on_bit);
 
+int __sched fastcall out_of_line_wait_on_bit(void *word, int bit,
+					int (*action)(void *), unsigned mode)
+{
+	wait_queue_head_t *wq = bit_waitqueue(word, bit);
+	DEFINE_WAIT_BIT(wait, word, bit);
+
+	return __wait_on_bit(wq, &wait, word, bit, action, mode);
+}
+EXPORT_SYMBOL(out_of_line_wait_on_bit);
+
 int __sched fastcall __wait_on_bit_lock(wait_queue_head_t *wq,
 			struct wait_bit_queue *q, void *word, int bit,
 			int (*action)(void *), unsigned mode)
 {
 	int ret = 0;
 
-	while (test_and_set_bit(bit, word)) {
+	do {
 		prepare_to_wait_exclusive(wq, &q->wait, mode);
 		if (test_bit(bit, word)) {
 			if ((ret = (*action)(word)))
 				break;
 		}
-	}
+	} while (test_and_set_bit(bit, word));
 	finish_wait(wq, &q->wait);
 	return ret;
 }
 EXPORT_SYMBOL(__wait_on_bit_lock);
 
+int __sched fastcall out_of_line_wait_on_bit_lock(void *word, int bit,
+					int (*action)(void *), unsigned mode)
+{
+	wait_queue_head_t *wq = bit_waitqueue(word, bit);
+	DEFINE_WAIT_BIT(wait, word, bit);
+
+	return __wait_on_bit_lock(wq, &wait, word, bit, action, mode);
+}
+EXPORT_SYMBOL(out_of_line_wait_on_bit_lock);
+
 void fastcall __wake_up_bit(wait_queue_head_t *wq, void *word, int bit)
 {
 	struct wait_bit_key key = __WAIT_BIT_KEY_INITIALIZER(word, bit);
@@ -190,6 +210,22 @@
 }
 EXPORT_SYMBOL(__wake_up_bit);
 
+/**
+ * wake_up_bit - wake up a waiter on a bit
+ * @word: the word being waited on, a kernel virtual address
+ * @bit: the bit of the word being waited on
+ *
+ * There is a standard hashed waitqueue table for generic use. This
+ * is the part of the hashtable's accessor API that wakes up waiters
+ * on a bit. For instance, if one were to have waiters on a bitflag,
+ * one would call wake_up_bit() after clearing the bit.
+ */
+void fastcall wake_up_bit(void *word, int bit)
+{
+	__wake_up_bit(bit_waitqueue(word, bit), word, bit);
+}
+EXPORT_SYMBOL(wake_up_bit);
+
 wait_queue_head_t * fastcall bit_waitqueue(void *word, int bit)
 {
 	const int shift = BITS_PER_LONG == 32 ? 5 : 6;

[2/2] reduce number of parameters to __wait_on_bit() and __wait_on_bit_lock()

[William Lee Irwin III]

On Tue, Aug 31, 2004 at 02:03:46PM -0700, William Lee Irwin III wrote:
> Move the slow paths of wait_on_bit() and wait_on_bit_lock() out of line.
> Also uninline wake_up_bit() to reduce the number of callsites
> generated, and adjust loop startup in  __wait_on_bit_lock() to properly
> reflect its usage in the contention case.
> Incremental atop the fastcall and wait_on_bit_lock()/test_and_set_bit() fixes.
> Successfully tested on x86-64.

Some of the parameters to __wait_on_bit() and __wait_on_bit_lock() are
redundant, as the wait_bit_queue parameter holds the flags word and the
bit number. This patch updates __wait_on_bit() and __wait_on_bit_lock()
to fetch that information from the wait_bit_queue passed to them and so
reduce the number of parameters so that -mregparm may be more effective.

Incremental atop the complete out-of-lining of the contention cases and
the fastcall and wait_on_bit_lock()/test_and_set_bit() fixes.

Successfully tested on x86-64.


Index: mm2-2.6.9-rc1/fs/fs-writeback.c
===================================================================
--- mm2-2.6.9-rc1.orig/fs/fs-writeback.c	2004-08-31 01:06:56.824462896 -0700
+++ mm2-2.6.9-rc1/fs/fs-writeback.c	2004-08-31 01:57:17.099311328 -0700
@@ -257,8 +257,8 @@
 		do {
 			__iget(inode);
 			spin_unlock(&inode_lock);
-			__wait_on_bit(wqh, &wq, &inode->i_state, __I_LOCK,
-					inode_wait, TASK_UNINTERRUPTIBLE);
+			__wait_on_bit(wqh, &wq, inode_wait,
+							TASK_UNINTERRUPTIBLE);
 			iput(inode);
 			spin_lock(&inode_lock);
 		} while (inode->i_state & I_LOCK);
Index: mm2-2.6.9-rc1/include/linux/wait.h
===================================================================
--- mm2-2.6.9-rc1.orig/include/linux/wait.h	2004-08-31 01:50:40.947535552 -0700
+++ mm2-2.6.9-rc1/include/linux/wait.h	2004-08-31 01:54:33.867126392 -0700
@@ -140,8 +140,8 @@
 extern void FASTCALL(__wake_up_locked(wait_queue_head_t *q, unsigned int mode));
 extern void FASTCALL(__wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr));
 void FASTCALL(__wake_up_bit(wait_queue_head_t *, void *, int));
-int FASTCALL(__wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, void *, int, int (*)(void *), unsigned));
-int FASTCALL(__wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, void *, int, int (*)(void *), unsigned));
+int FASTCALL(__wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, int (*)(void *), unsigned));
+int FASTCALL(__wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, int (*)(void *), unsigned));
 void FASTCALL(wake_up_bit(void *, int));
 int FASTCALL(out_of_line_wait_on_bit(void *, int, int (*)(void *), unsigned));
 int FASTCALL(out_of_line_wait_on_bit_lock(void *, int, int (*)(void *), unsigned));
Index: mm2-2.6.9-rc1/kernel/wait.c
===================================================================
--- mm2-2.6.9-rc1.orig/kernel/wait.c	2004-08-31 01:51:18.966755752 -0700
+++ mm2-2.6.9-rc1/kernel/wait.c	2004-08-31 02:00:10.430960912 -0700
@@ -151,14 +151,14 @@
  * permitted return codes. Nonzero return codes halt waiting and return.
  */
 int __sched fastcall __wait_on_bit(wait_queue_head_t *wq,
-			struct wait_bit_queue *q, void *word,
-			int bit, int (*action)(void *), unsigned mode)
+			struct wait_bit_queue *q,
+			int (*action)(void *), unsigned mode)
 {
 	int ret = 0;
 
 	prepare_to_wait(wq, &q->wait, mode);
-	if (test_bit(bit, word))
-		ret = (*action)(word);
+	if (test_bit(q->key.bit_nr, q->key.flags))
+		ret = (*action)(q->key.flags);
 	finish_wait(wq, &q->wait);
 	return ret;
 }
@@ -170,23 +170,23 @@
 	wait_queue_head_t *wq = bit_waitqueue(word, bit);
 	DEFINE_WAIT_BIT(wait, word, bit);
 
-	return __wait_on_bit(wq, &wait, word, bit, action, mode);
+	return __wait_on_bit(wq, &wait, action, mode);
 }
 EXPORT_SYMBOL(out_of_line_wait_on_bit);
 
 int __sched fastcall __wait_on_bit_lock(wait_queue_head_t *wq,
-			struct wait_bit_queue *q, void *word, int bit,
+			struct wait_bit_queue *q,
 			int (*action)(void *), unsigned mode)
 {
 	int ret = 0;
 
 	do {
 		prepare_to_wait_exclusive(wq, &q->wait, mode);
-		if (test_bit(bit, word)) {
-			if ((ret = (*action)(word)))
+		if (test_bit(q->key.bit_nr, q->key.flags)) {
+			if ((ret = (*action)(q->key.flags)))
 				break;
 		}
-	} while (test_and_set_bit(bit, word));
+	} while (test_and_set_bit(q->key.bit_nr, q->key.flags));
 	finish_wait(wq, &q->wait);
 	return ret;
 }
@@ -198,7 +198,7 @@
 	wait_queue_head_t *wq = bit_waitqueue(word, bit);
 	DEFINE_WAIT_BIT(wait, word, bit);
 
-	return __wait_on_bit_lock(wq, &wait, word, bit, action, mode);
+	return __wait_on_bit_lock(wq, &wait, action, mode);
 }
 EXPORT_SYMBOL(out_of_line_wait_on_bit_lock);
 
Index: mm2-2.6.9-rc1/mm/filemap.c
===================================================================
--- mm2-2.6.9-rc1.orig/mm/filemap.c	2004-08-31 01:06:56.779469736 -0700
+++ mm2-2.6.9-rc1/mm/filemap.c	2004-08-31 01:59:36.716086352 -0700
@@ -377,8 +377,8 @@
 	DEFINE_WAIT_BIT(wait, &page->flags, bit_nr);
 
 	if (test_bit(bit_nr, &page->flags))
-		__wait_on_bit(page_waitqueue(page), &wait, wait.key.flags,
-				bit_nr, sync_page, TASK_UNINTERRUPTIBLE);
+		__wait_on_bit(page_waitqueue(page), &wait, sync_page,
+							TASK_UNINTERRUPTIBLE);
 }
 EXPORT_SYMBOL(wait_on_page_bit);
 
@@ -436,8 +436,8 @@
 {
 	DEFINE_WAIT_BIT(wait, &page->flags, PG_locked);
 
-	__wait_on_bit_lock(page_waitqueue(page), &wait, wait.key.flags,
-				PG_locked, sync_page, TASK_UNINTERRUPTIBLE);
+	__wait_on_bit_lock(page_waitqueue(page), &wait, sync_page,
+							TASK_UNINTERRUPTIBLE);
 }
 EXPORT_SYMBOL(__lock_page);
 

[3/2] document wake_up_bit()'s requirement for preceding memory barriers

[William Lee Irwin III]

On Tue, Aug 31, 2004 at 02:05:42PM -0700, William Lee Irwin III wrote:
> Some of the parameters to __wait_on_bit() and __wait_on_bit_lock() are
> redundant, as the wait_bit_queue parameter holds the flags word and the
> bit number. This patch updates __wait_on_bit() and __wait_on_bit_lock()
> to fetch that information from the wait_bit_queue passed to them and so
> reduce the number of parameters so that -mregparm may be more effective.
> Incremental atop the complete out-of-lining of the contention cases and
> the fastcall and wait_on_bit_lock()/test_and_set_bit() fixes.
> Successfully tested on x86-64.

Document the requirement to use a memory barrier prior to wake_up_bit().


Index: mm2-2.6.9-rc1/kernel/wait.c
===================================================================
--- mm2-2.6.9-rc1.orig/kernel/wait.c	2004-08-31 02:00:10.000000000 -0700
+++ mm2-2.6.9-rc1/kernel/wait.c	2004-08-31 14:07:13.688481360 -0700
@@ -219,6 +219,13 @@
  * is the part of the hashtable's accessor API that wakes up waiters
  * on a bit. For instance, if one were to have waiters on a bitflag,
  * one would call wake_up_bit() after clearing the bit.
+ *
+ * In order for this to function properly, as it uses waitqueue_active()
+ * internally, some kind of memory barrier must be done prior to calling
+ * this. Typically, this will be smp_mb__after_clear_bit(), but in some
+ * cases where bitflags are manipulated non-atomically under a lock, one
+ * may need to use a less regular barrier, such fs/inode.c's smp_mb(),
+ * because spin_unlock() does not guarantee a memory barrier.
  */
 void fastcall wake_up_bit(void *word, int bit)
 {