Re: [PATCH] fix misplaced mb() in rcu_enter/exit_nohz()

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Mon, Mar 17, 2008 at 09:30:47PM +0300, Oleg Nesterov wrote:
> On 03/16, Paul E. McKenney wrote:
> > 
> > In the process of writing up the mechanical proof of correctness for the
> > dynticks/preemptable-RCU interface, I noticed misplaced memory barriers
> > in rcu_enter_nohz() and rcu_exit_nohz().
> 
> Can't comment this patch, there is no rcu_enter_nohz() in my rcupreempt.h ;)

It is in 2.6.25-rc4 and later.  ;-)

> I'm not sure the code below is up to date, but what I have in
> arch/s390/kernel/time.c is:
> 
> 	stop_hz_timer:
> 
> 		cpu_set(cpu, nohz_cpu_mask);
> 		
> 		if (rcu_needs_cpu(cpu) || local_softirq_pending()) {
> 			cpu_clear(cpu, nohz_cpu_mask);
> 			return;
> 		}
> 
> Don't we need smp_mb() after cpu_set() ?

S390's memory model is quite strong, so it might not be needed.  In any
case, if needed, it goes -before- the cpu_set(), because the problems
would arise if prior RCU read-side critical sections were to be reordered
to follow this cpu_set(), right?

Let's see...  In S390, a store cannot be reordered to precede any prior
load or store, so any preceding RCU read-side critical section would be
seen by all CPUs as preceding the shift to nohz mode.  Might be trouble
for the opposite transition...

But last I heard, the s390 guys were thinking in terms of moving to the
generic dynticks model.  If they really are doing so, then the above
code goes away in any case.

							Thanx, Paul