Re: Workqueues splat due to ending up on wrong CPU

[Peter Zijlstra]

On Thu, Dec 05, 2019 at 11:29:28AM +0100, Peter Zijlstra wrote:
> On Wed, Dec 04, 2019 at 12:11:50PM -0800, Paul E. McKenney wrote:
> 
> > And the good news is that I didn't see the workqueue splat, though my
> > best guess is that I had about a 13% chance of not seeing it due to
> > random chance (and I am currently trying an idea that I hope will make
> > it more probable).  But I did get a couple of new complaints about RCU
> > being used illegally from an offline CPU.  Splats below.
> 
> Shiny!
> 
> > Your patch did rearrange the CPU-online sequence, so let's see if I
> > can piece things together...
> > 
> > RCU considers a CPU to be online at rcu_cpu_starting() time.  This is
> > called from notify_cpu_starting(), which is called from the arch-specific
> > CPU-bringup code.  Any RCU readers before rcu_cpu_starting() will trigger
> > the warning I am seeing.
> 
> Right.
> 
> > The original location of the stop_machine_unpark() was in
> > bringup_wait_for_ap(), which is called from bringup_cpu(), which is in
> > the CPUHP_BRINGUP_CPU entry of cpuhp_hp_states[].  Which, if I am not
> > too confused, is invoked by some CPU other than the to-be-incoming CPU.
> 
> Correct.
> 
> > The new location of the stop_machine_unpark() is in cpuhp_online_idle(),
> > which is called from cpu_startup_entry(), which is invoked from
> > the arch-specific bringup code that runs on the incoming CPU.
> 
> The new place is the final piece of bringup, it is right before where
> the freshly woken CPU will drop into the idle loop and start scheduling
> (for the first time).
> 
> > Which
> > is the same code that invokes notify_cpu_starting(), so we need
> > notify_cpu_starting() to be invoked before cpu_startup_entry().
> 
> Right, that is right before we run what used to be the CPU_STARTING
> notifiers. This is in fact (on x86) before the CPU is marked
> cpu_online(). It has to be before cpu_startup_entry(), before this is
> ran with IRQs disabled, while cpu_startup_entry() demands IRQs are
> enabled.
> 
> > The order is not immediately obvious on IA64.  But it looks like
> > everything else does it in the required order, so I am a bit confused
> > about this.
> 
> That makes two of us, afaict we have RCU up and running when we get to
> the idle loop.

Or did we need rcutree_online_cpu() to have ran? Because that is ran
much later than this...