lkml.org 
[lkml]   [2020]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT pull] locking/urgent for v5.10-rc6
On Sun, Nov 29, 2020 at 11:31:41AM -0800, Linus Torvalds wrote:
> On Sun, Nov 29, 2020 at 5:38 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Yet two more places which invoke tracing from RCU disabled regions in the
> > idle path. Similar to the entry path the low level idle functions have to
> > be non-instrumentable.
>
> This really seems less than optimal.
>
> In particular, lookie here:
>
> > @@ -94,9 +94,35 @@ void __cpuidle default_idle_call(void)
> >
> > trace_cpu_idle(1, smp_processor_id());
> > stop_critical_timings();
> > +
> > + /*
> > + * arch_cpu_idle() is supposed to enable IRQs, however
> > + * we can't do that because of RCU and tracing.
> > + *
> > + * Trace IRQs enable here, then switch off RCU, and have
> > + * arch_cpu_idle() use raw_local_irq_enable(). Note that
> > + * rcu_idle_enter() relies on lockdep IRQ state, so switch that
> > + * last -- this is very similar to the entry code.
> > + */
> > + trace_hardirqs_on_prepare();
> > + lockdep_hardirqs_on_prepare(_THIS_IP_);
> > rcu_idle_enter();
> > + lockdep_hardirqs_on(_THIS_IP_);
> > +
> > arch_cpu_idle();
> > +
> > + /*
> > + * OK, so IRQs are enabled here, but RCU needs them disabled to
> > + * turn itself back on.. funny thing is that disabling IRQs
> > + * will cause tracing, which needs RCU. Jump through hoops to
> > + * make it 'work'.
> > + */
> > + raw_local_irq_disable();
> > + lockdep_hardirqs_off(_THIS_IP_);
> > rcu_idle_exit();
> > + lockdep_hardirqs_on(_THIS_IP_);
> > + raw_local_irq_enable();
> > +
> > start_critical_timings();
> > trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
> > }
>
> And look at what the code generation for the idle exit path is when
> lockdep isn't even on.

Agreed.

The idea was to flip all of arch_cpu_idle() to not enable interrupts.

This is suboptimal for things like x86 where arch_cpu_idle() is
basically STI;HLT, but x86 isn't likely to actually use this code path
anyway, given all the various cpuidle drivers it has.

Many of the other archs are now doing things like arm's:
wfi();raw_local_irq_enable().

Doing that tree-wide interrupt-state flip was something I didn't want to
do at this late a stage, the chanse of messing that up is just too high.

After that I need to go look at flipping cpuidle, which is even more
'interesting'. cpuidle_enter() has the exact same semantics, and this is
the code path that x86 actually uses, and here it's inconsitent at best.

\
 
 \ /
  Last update: 2020-11-30 09:00    [W:0.124 / U:0.284 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site