[patch] possible SMP races all over the place in wait_event interface

Andrea Arcangeli (andrea@suse.de)
Mon, 16 Aug 1999 14:50:55 +0200 (CEST)

Maybe I am dreaming, but it seems to me that the wait_even interface has
SMP races. To make it SMP safe we should enforce read and write ordering
in the wait_event code.

Take for example the wait_on_buffer/unlock_buffer code.

The wakeup interface should write to the bus the cleared bit _before_
reading the state of the current task in wake_up_process.

The sleep interface should write to the bus the UNINTERRUPTIBLE state of
the process _before_ reading if the locked bit is set or not.

The ordering is the basic rule of the wait_even interface and the way we
avoid deadlocking after missing an event.

So if I am not dreaming patches like the one below should be applyed all
over the place: (against 2.3.13)

diff -urN 2.3.13/fs/buffer.c 2.3.13-tmp/fs/buffer.c
--- 2.3.13/fs/buffer.c Thu Aug 12 02:53:23 1999
+++ 2.3.13-tmp/fs/buffer.c Mon Aug 16 14:46:11 1999
@@ -146,8 +146,9 @@
atomic_inc(&bh->b_count);
add_wait_queue(&bh->b_wait, &wait);
repeat:
- tsk->state = TASK_UNINTERRUPTIBLE;
run_task_queue(&tq_disk);
+ tsk->state = TASK_UNINTERRUPTIBLE;
+ mb();
if (buffer_locked(bh)) {
schedule();
goto repeat;
diff -urN 2.3.13/include/linux/locks.h 2.3.13-tmp/include/linux/locks.h
--- 2.3.13/include/linux/locks.h Fri Aug 13 00:17:17 1999
+++ 2.3.13-tmp/include/linux/locks.h Mon Aug 16 14:06:54 1999
@@ -29,6 +29,7 @@
extern inline void unlock_buffer(struct buffer_head *bh)
{
clear_bit(BH_Lock, &bh->b_state);
+ mb();
wake_up(&bh->b_wait);
}

diff -urN 2.3.13/include/linux/sched.h 2.3.13-tmp/include/linux/sched.h
--- 2.3.13/include/linux/sched.h Fri Aug 13 00:17:17 1999
+++ 2.3.13-tmp/include/linux/sched.h Mon Aug 16 14:47:43 1999
@@ -716,6 +716,7 @@
add_wait_queue(&wq, &__wait); \
for (;;) { \
current->state = TASK_UNINTERRUPTIBLE; \
+ mb(); \
if (condition) \
break; \
schedule(); \
@@ -739,6 +740,7 @@
add_wait_queue(&wq, &__wait); \
for (;;) { \
current->state = TASK_INTERRUPTIBLE; \
+ mb(); \
if (condition) \
break; \
if (!signal_pending(current)) { \

Comments? If I am right I can start to fix such SMP race in all the
kernel.

Andrea

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/