Messages in this thread | | | Date | Thu, 5 Dec 2019 11:32:13 +0100 | From | Peter Zijlstra <> | Subject | Re: Workqueues splat due to ending up on wrong CPU |
| |
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...
| |