linux-kernel - Re: [PATCH tip/core/rcu 45/47] rcu: Allow nesting of rcu_idle_enter() and rcu_idle

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] [day] [month] [year] [list]
Date:	Thu, 9 Feb 2012 16:33:49 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, laijs@...fujitsu.com,
	dipankar@...ibm.com, akpm@...ux-foundation.org,
	mathieu.desnoyers@...ymtl.ca, josh@...htriplett.org,
	niv@...ibm.com, tglx@...utronix.de, peterz@...radead.org,
	rostedt@...dmis.org, Valdis.Kletnieks@...edu, dhowells@...hat.com,
	eric.dumazet@...il.com, darren@...art.com, patches@...aro.org,
	"Paul E. McKenney" <paul.mckenney@...aro.org>
Subject: Re: [PATCH tip/core/rcu 45/47] rcu: Allow nesting of
 rcu_idle_enter() and rcu_idle_exit()

On Thu, Feb 09, 2012 at 07:26:20AM -0800, Paul E. McKenney wrote:
> On Thu, Feb 09, 2012 at 05:07:04AM +0100, Frederic Weisbecker wrote:
> > On Fri, Feb 03, 2012 at 05:45:20PM -0800, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> > > 
> > > Use of RCU in the idle loop is incorrect, quite a few instances of
> > > just that have made their way into mainline, primarily event tracing.
> > > The problem with RCU read-side critical sections on CPUs that RCU believes
> > > to be idle is that RCU is completely ignoring the CPU, along with any
> > > attempts and RCU read-side critical sections.
> > > 
> > > The approaches of eliminating the offending uses and of pushing the
> > > definition of idle down beyond the offending uses have both proved
> > > impractical.  The new approach is to encapsulate offending uses of RCU
> > > with rcu_idle_exit() and rcu_idle_enter(), but this requires nesting
> > > for code that is invoked both during idle and and during normal execution.
> > > Therefore, this commit modifies rcu_idle_enter() and rcu_idle_exit() to
> > > permit nesting.
> > > 
> > > Signed-off-by: Paul E. McKenney <paul.mckenney@...aro.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > > Reviewed-by: Josh Triplett <josh@...htriplett.org>
> > > Acked-by: Deepthi Dharwar <deepthi@...ux.vnet.ibm.com>
> > > ---
> > >  kernel/rcu.h     |   21 ++++++++++++++++++++-
> > >  kernel/rcutiny.c |   16 ++++++++++++----
> > >  kernel/rcutree.c |   21 ++++++++++++++-------
> > >  3 files changed, 46 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/kernel/rcu.h b/kernel/rcu.h
> > > index 30876f4..8ba99cd 100644
> > > --- a/kernel/rcu.h
> > > +++ b/kernel/rcu.h
> > > @@ -33,8 +33,27 @@
> > >   * Process-level increment to ->dynticks_nesting field.  This allows for
> > >   * architectures that use half-interrupts and half-exceptions from
> > >   * process context.
> > > + *
> > > + * DYNTICK_TASK_NEST_MASK defines a field of width DYNTICK_TASK_NEST_WIDTH
> > > + * that counts the number of process-based reasons why RCU cannot
> > > + * consider the corresponding CPU to be idle, and DYNTICK_TASK_NEST_VALUE
> > > + * is the value used to increment or decrement this field.
> > > + *
> > > + * The rest of the bits could in principle be used to count interrupts,
> > > + * but this would mean that a negative-one value in the interrupt
> > > + * field could incorrectly zero out the DYNTICK_TASK_NEST_MASK field.
> > > + * We therefore provide a two-bit guard field defined by DYNTICK_TASK_MASK
> > > + * that is set to DYNTICK_TASK_FLAG upon initial exit from idle.
> > > + * The DYNTICK_TASK_EXIT_IDLE value is thus the combined value used upon
> > > + * initial exit from idle.
> > >   */
> > > -#define DYNTICK_TASK_NESTING (LLONG_MAX / 2 - 1)
> > > +#define DYNTICK_TASK_NEST_WIDTH 7
> > > +#define DYNTICK_TASK_NEST_VALUE ((LLONG_MAX >> DYNTICK_TASK_NEST_WIDTH) + 1)
> > > +#define DYNTICK_TASK_NEST_MASK  (LLONG_MAX - DYNTICK_TASK_NEST_VALUE + 1)
> > > +#define DYNTICK_TASK_FLAG	   ((DYNTICK_TASK_NEST_VALUE / 8) * 2)
> > > +#define DYNTICK_TASK_MASK	   ((DYNTICK_TASK_NEST_VALUE / 8) * 3)
> > 
> > There is one unused bit between DYNTICK_TASK_NEST_MASK and DYNTICK_TASK_MASK, is
> > that intentional?
> 
> Yep, it makes it easier for me to read hex dumps of the variables.

I see.
 
> > Also do you want to allow nesting of that kind?
> > 
> > 	rcu_idle_enter();
> > 		rcu_idle_enter();
> > 		rcu_idle_exit();
> > 	rcu_idle_exit()
> 
> No -- only the inverse where you exit idle multiple times.
> 
> > in which case I guess that rcu_irq_enter()/rcu_irq_exit() also need to
> > be updated.
> > 
> > If we have this:
> > 
> > 	rcu_idle_enter()
> > 	rcu_idle_enter()
> > 
> > 	rcu_irq_enter()
> > 	rcu_irq_exit()
> > 
> > 	rcu_idle_exit()
> > 	rcu_idle_exit()
> > 
> > On rcu_irq_enter(), oldval will never be 0 and we'll miss rcu_idle_exit_common().
> > rcu_irq_exit() has a similar problem as it won't enter rcu_idle_enter_common().
> > 
> > Its check on WARN_ON_ONCE(rdtp->dynticks_nesting < 0) is also wrong because after
> > two calls of rcu_idle_enter(), the value of dynticks_nesting is negative : it's
> > -DYNTICK_TASK_NEST_VALUE.
> > 
> > Perhaps this change would allow that. But again that's just in case you need to
> > support that kind of nesting.
> 
> Interesting.  I don't know of a use case for this -- do you have any?
> 
> 							Thanx, Paul

Not really. I was just not sure what you were targeting exactly :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/