lkml.org 
[lkml]   [2012]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@linux.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@linaro.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > Acked-by: Deepthi Dharwar <deepthi@linux.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 :)


\
 
 \ /
  Last update: 2012-02-09 16:37    [W:0.063 / U:0.668 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site