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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170114195417.GW5238@linux.vnet.ibm.com>
Date:   Sat, 14 Jan 2017 11:54:17 -0800
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     linux-kernel@...r.kernel.org, jiangshanlai@...il.com,
        dipankar@...ibm.com, akpm@...ux-foundation.org,
        mathieu.desnoyers@...icios.com, josh@...htriplett.org,
        tglx@...utronix.de, peterz@...radead.org, rostedt@...dmis.org,
        dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
        fweisbec@...il.com, oleg@...hat.com, bobby.prani@...il.com
Subject: Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

On Sat, Jan 14, 2017 at 10:35:50AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@...ux.vnet.ibm.com> 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@...il.com>
> > Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > ---
> >  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