More barriers in module code.

PATCH New Rusty Russell 1 participant 1 message Oct 9, 2003

Share & tools

Rusty Russell

author

2003-10-09 08:51 UTC #0 lore raw

PATCH 1/1 More barriers in module code.

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.

kernel/module.c +4 -0

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
@@ -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.