[PATCH] More barriers in module code.

[Rusty Russell]

Paul McKenney convinced me that there's no *guarantee* that all archs
will refuse the speculate the atomic ops above the state test, eg:

CPU0 (stopref_set_state)		CPU1 (stopref)

 atomic_set(&ack, 0);			if (state == XXX)
 wmb();						atomic_inc(&ack);
 state = XXX;

Certainly Alpha needs a rmb() inside stopref to guarantee it sees
the same ordering.
---
Linus, please apply.

Name: More Barriers for Module Stopref Threads
Author: Rusty Russell
Status: Trivial

D: Paul McKenney convinced me that there's no *guarantee* that all archs
D: will refuse the speculate the atomic ops above the state test, eg:
D:
D:  CPU0 (stopref_set_state)			CPU1 (stopref)
D: 
D:  atomic_set(&ack, 0);			if (state == XXX)
D:  wmb();						atomic_inc(&ack);
D:  state = XXX;
D:
D: Certainly Alpha needs a rmb() inside stopref to guarantee it sees
D: the same ordering.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.69-bk6/kernel/module.c working-2.5.69-bk6-stopref/kernel/module.c
--- linux-2.5.69-bk6/kernel/module.c	2003-05-05 12:37:13.000000000 +1000
+++ working-2.5.69-bk6-stopref/kernel/module.c	2003-05-12 14:26:58.000000000 +1000
@@ -311,6 +311,7 @@ static int stopref(void *cpu)
 	set_cpus_allowed(current, 1UL << (unsigned long)cpu);
 
 	/* Ack: we are alive */
+	mb(); /* Theoretically the ack = 0 might not be on this CPU yet. */
 	atomic_inc(&stopref_thread_ack);
 
 	/* Simple state machine */
@@ -319,11 +320,13 @@ static int stopref(void *cpu)
 			local_irq_disable();
 			irqs_disabled = 1;
 			/* Ack: irqs disabled. */
+			mb(); /* Must read state first. */
 			atomic_inc(&stopref_thread_ack);
 		} else if (stopref_state == STOPREF_PREPARE && !prepared) {
 			/* Everyone is in place, hold CPU. */
 			preempt_disable();
 			prepared = 1;
+			mb(); /* Must read state first. */
 			atomic_inc(&stopref_thread_ack);
 		}
 		if (irqs_disabled || prepared)
@@ -333,6 +336,7 @@ static int stopref(void *cpu)
 	}
 
 	/* Ack: we are exiting. */
+	mb(); /* Must read state first. */
 	atomic_inc(&stopref_thread_ack);
 
 	if (irqs_disabled)

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/