Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Oct 08, 2015 at 08:33:51AM -0700, Paul E. McKenney wrote:

> > > o	CPU B therefore moves up the tree, acquiring the parent
> > > 	rcu_node structures' ->lock.  In so doing, it forces full
> > > 	ordering against all prior RCU read-side critical sections
> > > 	of all CPUs corresponding to all leaf rcu_node structures
> > > 	subordinate to the current (non-leaf) rcu_node structure.
> > 
> > And here we iterate the tree and get another lock var involved, here the
> > barrier upgrade will actually do something.
> 
> Yep.  And I am way too lazy to sort out exactly which acquisitions really
> truly need smp_mb__after_unlock_lock() and which don't.  Besides, if I
> tried to sort it out, I would occasionally get it wrong, and this would be
> a real pain to debug.  Therefore, I simply do smp_mb__after_unlock_lock()
> on all acquisitions of the rcu_node structures' ->lock fields.  I can
> actually validate that!  ;-)

This is a whole different line of reasoning once again.

The point remains, that the sole purpose of the barrier upgrade is for
the tree iteration, having some extra (pointless but harmless) instances
does not detract from that.

> Fair enough, but I will be sticking to the simple coding rule that keeps
> RCU out of trouble!

Note that there are rnp->lock acquires without the extra barrier though,
so you seem somewhat inconsistent with your own rule.

See for example:

	rcu_dump_cpu_stacks()
	print_other_cpu_stall()
	print_cpu_stall()

(did not do an exhaustive scan, there might be more)

and yes, that is 'obvious' debug code and not critical to the correct
behaviour of the code, but it is a deviation from 'the rule'.