Re: [PATCH 1/4] sched/idle: Fix missing need_resched() check after rcu_idle_enter()

[Frederic Weisbecker]

On Tue, Jan 05, 2021 at 10:55:03AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 04, 2021 at 04:20:55PM +0100, Frederic Weisbecker wrote:
> > Entering RCU idle mode may cause a deferred wake up of an RCU NOCB_GP
> > kthread (rcuog) to be serviced.
> > 
> > Usually a wake up happening while running the idle task is spotted in
> > one of the need_resched() checks carefully placed within the idle loop
> > that can break to the scheduler.
> 
> Urgh, this is horrific and fragile :/ You having had to audit and fix a
> number of rcu_idle_enter() callers should've made you realize that
> making rcu_idle_enter() return something would've been saner.
> 
> Also, I might hope that when RCU does do that wakeup, it will not have
> put RCU in idle mode? So it is a natural 'fail' state for
> rcu_idle_enter(), *sigh* it continues to put RCU to sleep, so that needs
> fixing too.

Heh, yes you're right, that looks saner.

> 
> I'm thinking that rcu_user_enter() will have the exact same problem? Did
> you audit that?

Yes and I wanted to fix it seperately since it's a bit harder to fix because
we are past the last need_resched() check, all syscall exit works, lockdep
hardirqs on entry prep, tracing hardirqs on, etc... I need to manage to
rollback safely and cleanly.

Unless I can decouple the wakeup from rcu_user_enter() and put it around the
exit_to_user_mode_loop(). But then I must make sure that call_rcu() isn't called
afterward.

> 
> Something like the below, combined with a fixup for all callers (which
> the compiler will help us find thanks to __must_check).

Right, I just need to make sure that the wake up is local as the kthread
awaken can be queued anywhere. But a simple need_resched() check after the
wake up should be fine to get that.

Thanks.