Re: [patch 20/20] rcu: Make CPU_DYING_IDLE an explicit call

From: Paul E. McKenney
Date: Sat Feb 27 2016 - 06:05:58 EST


On Sat, Feb 27, 2016 at 08:47:41AM +0100, Thomas Gleixner wrote:
> On Fri, 26 Feb 2016, Paul E. McKenney wrote:
> > > > --- a/kernel/cpu.c
> > > > +++ b/kernel/cpu.c
> > > > @@ -762,6 +762,7 @@ void cpuhp_report_idle_dead(void)
> > > > BUG_ON(st->state != CPUHP_AP_OFFLINE);
> > > > st->state = CPUHP_AP_IDLE_DEAD;
> > > > complete(&st->done);
> > >
> > > What prevents the other CPU from killing this CPU at this point, so
> > > that this CPU does not tell RCU that it is dead?
> > >
> > > I agree that the odds should be low, but there are all manner of things
> > > that might delay a CPU for just a little bit too long...
> > >
> > > Or am I missing something subtle here?
>
> No. The reason why I moved the rcu call past the complete is, that otherwise
> complete() complains about rcu being dead already. Hmm, but you are right. In
> theory the other side could allow physical removal before it actually told rcu
> that it's gone.

There is one case where this is OK, and that is where the outgoing CPU
puts itself to sleep (or whatever) without help from the other CPU.

> > Just in case I am not missing anything...
> >
> > One approach is to go back to the spinning, but to do rcu_report_dead()
> > just before kicking the other CPU. This would also fix some issues with
> > use of RCU of the offline path, so would definitely be better than my
> > earlier approach of notifying RCU from within the idle loop.
> >
> > This assumes that all the offline paths have been consolidated into
> > this path. (Yes, I was too lazy and cowardly to consolidate them all
> > last I touched this code, but perhaps that has happened elsewise?)
>
> The question is whether the rcu dead notification has to happen
> instantaniously and needs to be done on the dead cpu. If we can avoid both,
> then there is a very simple solution.

Hmmm...

The rcu_cleanup_dying_idle_cpu() can be invoked from the surviving CPU,
-as- -long- -as- nothing in the intervening code path waits for a grace
period. The wakeup path itself had better not wait for a grace period,
of course. The concern would be that the task running on the surviving
CPU might be waiting for a grace period before sleeping -- which used to
be possible due to the CPU-hotplug notifiers that it might be executing
before getting to RCU's CPU-hotplug notifiers.

The rcu_report_exp_rdp() is considerably more scary. At first glance,
it looks OK, but I will need to stare at it for a bit.

Of course, if the task running on the surviving CPU can be waiting for a
grace period, one of two problems can happen:

o RCU times out the outgoing CPU before it has really left.
This is the current state, and needs to change. The dying idle
stuff was half of the needed change, the other half being on
the incoming side.

o Deadlock -- the outgoing CPU won't respond to RCU, so the
task running on the surviving CPU never wakes up from its
wait on a grace period.

So if rcu_report_exp_rdp() turns out to be OK -and- if the outgoing
task never waits on a grace period during the CPU-shutdown process,
this might work.

Of course, my ability to test this sufficiently viciously is currently
blocked by the lost-wakeup problem I am currently chasing. (Hey, at
least I finally get ftrace output! Completely baffling output, but so
it goes...) :-/

Thanx, Paul