On Fri, Jan 22, 2021 at 09:31:37AM +0100, Peter Zijlstra wrote:
> On Thu, Jan 21, 2021 at 04:20:12PM -0800, Paul E. McKenney wrote:
>
> > > ---
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 368749008ae8..2c8d4c3e341e 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -445,7 +445,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
> > > /*
> > > * Usually called from the tick; but also used from smp_function_call()
> > > * for expedited grace periods. This latter can result in running from
> > > - * the idle task, instead of an actual IPI.
> > > + * a (usually the idle) task, instead of an actual IPI.
> >
> > The story is growing enough hair that we should tell it only once.
> > So here just where it is called from:
> >
> > /*
> > * Usually called from the tick; but also used from smp_function_call()
> > * for expedited grace periods.
> > */
> >
> > > lockdep_assert_irqs_disabled();
> > >
> > > @@ -461,9 +461,14 @@ static int rcu_is_cpu_rrupt_from_idle(void)
> > > return false;
> > >
> > > /*
> > > - * If we're not in an interrupt, we must be in the idle task!
> > > + * If we're not in an interrupt, we must be in task context.
> > > + *
> > > + * This will typically be the idle task through:
> > > + * flush_smp_call_function_from_idle(),
> > > + *
> > > + * but can also be in CPU HotPlug through smpcfd_dying().
> > > */
> >
> > Good, but how about like this?
> >
> > /*
> > * If we are not in an interrupt handler, we must be in
> > * smp_call_function() handler.
> > *
> > * Normally, smp_call_function() handlers are invoked from
> > * the idle task via flush_smp_call_function_from_idle().
> > * However, they can also be invoked from CPU hotplug
> > * operations via smpcfd_dying().
> > */
> >
> > > - WARN_ON_ONCE(!nesting && !is_idle_task(current));
> > > + WARN_ON_ONCE(!nesting && !in_task(current));
> >
> > This is used in time-critical contexts, so why not RCU_LOCKDEP_WARN()?
> > That should also allow checking more closely. Would something like the
> > following work?
> >
> > RCU_LOCKDEP_WARN(!nesting && !is_idle_task(current) &&
> > (!in_task(current) || !lockdep_cpus_write_held()));
> >
> > Where lockdep_cpus_write_held is defined in kernel/cpu.c:
>
> Works for me, except s/in_task(current)/in_task()/ compiles a lot
> better.
These compilers sure constrain my creativity! ;-)
Might be a good thing, though...
Thanx, Paul
