Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Paul E. McKenney]

On Sat, Jan 14, 2017 at 10:35:50AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > If a process invokes synchronize_srcu(), is delayed just the right amount
> > of time, and thus does not sleep when waiting for the grace period to
> > complete, there is no ordering between the end of the grace period and
> > the code following the synchronize_srcu().  Similarly, there can be a
> > lack of ordering between the end of the SRCU grace period and callback
> > invocation.
> > 
> > This commit adds the necessary ordering.
> > 
> > Reported-by: Lance Roy <ldr709@xxxxxxxxx>
> > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > ---
> >  include/linux/rcupdate.h | 12 ++++++++++++
> >  kernel/rcu/srcu.c        |  5 +++++
> >  kernel/rcu/tree.h        | 12 ------------
> >  3 files changed, 17 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 01f71e1d2e94..608d56f908f2 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -1161,5 +1161,17 @@ do { \
> >  		ftrace_dump(oops_dump_mode); \
> >  } while (0)
> >  
> > +/*
> > + * Place this after a lock-acquisition primitive to guarantee that
> > + * an UNLOCK+LOCK pair act as a full barrier.  This guarantee applies
> > + * if the UNLOCK and LOCK are executed by the same CPU or if the
> > + * UNLOCK and LOCK operate on the same lock variable.
> 
> minor typo:
> 
>   s/an UNLOCK+LOCK pair act as
>     an UNLOCK+LOCK pair acts as

Fixed.

> > + */
> > +#ifdef CONFIG_PPC
> > +#define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> > +#else /* #ifdef CONFIG_PPC */
> > +#define smp_mb__after_unlock_lock()	do { } while (0)
> > +#endif /* #else #ifdef CONFIG_PPC */
> 
> Yeah, so I realize that this was pre-existing code, but putting CONFIG_$ARCH
> #ifdefs into generic headers is generally frowned upon.
> 
> The canonical approach would be either to define a helper Kconfig variable that 
> can be set by PPC (but other architectures don't need to set it), or to expose a 
> suitable macro (function) for architectures to define in their barrier.h arch 
> header file.

Very well, I will add a separate commit for this.  4.11 OK?

							Thanx, Paul