Re: [PATCH tip/core/rcu 04/26] rcu: Settle config for userspaceextended quiescent state

[Josh Triplett]

On Tue, Sep 04, 2012 at 05:34:59PM -0700, Paul E. McKenney wrote:
> On Fri, Aug 31, 2012 at 04:44:01PM -0700, Josh Triplett wrote:
> > On Thu, Aug 30, 2012 at 02:05:21PM -0700, Paul E. McKenney wrote:
> > > From: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> > > 
> > > Create a new config option under the RCU menu that put
> > > CPUs under RCU extended quiescent state (as in dynticks
> > > idle mode) when they run in userspace. This require
> > > some contribution from architectures to hook into kernel
> > > and userspace boundaries.
> > > 
> > > Signed-off-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> > > Cc: Alessio Igor Bogani <abogani@xxxxxxxxxx>
> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > > Cc: Avi Kivity <avi@xxxxxxxxxx>
> > > Cc: Chris Metcalf <cmetcalf@xxxxxxxxxx>
> > > Cc: Christoph Lameter <cl@xxxxxxxxx>
> > > Cc: Geoff Levand <geoff@xxxxxxxxxxxxx>
> > > Cc: Gilad Ben Yossef <gilad@xxxxxxxxxxxxx>
> > > Cc: Hakan Akkan <hakanakkan@xxxxxxxxx>
> > > Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> > > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > > Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> > > Cc: Kevin Hilman <khilman@xxxxxx>
> > > Cc: Max Krasnyansky <maxk@xxxxxxxxxxxx>
> > > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > > Cc: Stephen Hemminger <shemminger@xxxxxxxxxx>
> > > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > > Cc: Sven-Thorsten Dietrich <thebigcorporation@xxxxxxxxx>
> > > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > 
> > One question below, but nonethelesss:
> > 
> > Reviewed-by: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> > 
> > >  arch/Kconfig             |   10 ++++++++++
> > >  include/linux/rcupdate.h |    8 ++++++++
> > >  init/Kconfig             |   10 ++++++++++
> > >  kernel/rcutree.c         |    5 ++++-
> > >  4 files changed, 32 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > index 72f2fa1..1401a75 100644
> > > --- a/arch/Kconfig
> > > +++ b/arch/Kconfig
> > > @@ -281,4 +281,14 @@ config SECCOMP_FILTER
> > >  
> > >  	  See Documentation/prctl/seccomp_filter.txt for details.
> > >  
> > > +config HAVE_RCU_USER_QS
> > > +	bool
> > > +	help
> > > +	  Provide kernel entry/exit hooks necessary for userspace
> > > +	  RCU extended quiescent state. Syscalls need to be wrapped inside
> > > +	  rcu_user_exit()-rcu_user_enter() through the slow path using
> > > +	  TIF_NOHZ flag. Exceptions handlers must be wrapped as well. Irqs
> > > +	  are already protected inside rcu_irq_enter/rcu_irq_exit() but
> > > +	  preemption or signal handling on irq exit still need to be protected.
> > > +
> > >  source "kernel/gcov/Kconfig"
> > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > index 81d3d5c..e411117 100644
> > > --- a/include/linux/rcupdate.h
> > > +++ b/include/linux/rcupdate.h
> > > @@ -191,10 +191,18 @@ extern void rcu_idle_enter(void);
> > >  extern void rcu_idle_exit(void);
> > >  extern void rcu_irq_enter(void);
> > >  extern void rcu_irq_exit(void);
> > > +
> > > +#ifdef CONFIG_RCU_USER_QS
> > >  extern void rcu_user_enter(void);
> > >  extern void rcu_user_exit(void);
> > >  extern void rcu_user_enter_irq(void);
> > >  extern void rcu_user_exit_irq(void);
> > > +#else
> > > +static inline void rcu_user_enter(void) { }
> > > +static inline void rcu_user_exit(void) { }
> > > +#endif /* CONFIG_RCU_USER_QS */
> > > +
> > > +
> > >  extern void exit_rcu(void);
> > >  
> > >  /**
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index af6c7f8..f6a1830 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -441,6 +441,16 @@ config PREEMPT_RCU
> > >  	  This option enables preemptible-RCU code that is common between
> > >  	  the TREE_PREEMPT_RCU and TINY_PREEMPT_RCU implementations.
> > >  
> > > +config RCU_USER_QS
> > > +	bool "Consider userspace as in RCU extended quiescent state"
> > > +	depends on HAVE_RCU_USER_QS && SMP
> > 
> > Does this actually depend on SMP, or does it depend on the non-TINY RCU
> > implementation?  If the latter, it should depend on that rather than
> > SMP.
> > 
> > (I assume that the tiny RCU implementation simply doesn't need all this
> > machinery because it doesn't need coordinated quiescence at all?  Or
> > does tiny RCU still cause a periodic wakeup on UP?)
> 
> It actually does depend on SMP.  There has to be at least one CPU taking
> scheduling-clock interrupts in order to keep time computation accurate,
> so a de-facto UP system cannot adaptive-dynticks its sole CPU.

Ah.  That seems like a removable limitation, albeit a difficult one.
Nonetheless, it makes sense to avoid providing the option when it won't
help.

However, once a config symbol for adaptive dynticks exists, perhaps that
symbol should depend on SMP and RCU_USER_QS should depend on that
instead, documenting the limitation in the right place and making it
easier to find and change eventually.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/