Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH] cpuhp: make target_store() a nop when target == state | Date | Wed, 25 May 2022 16:09:29 +0100 |
| |
On 25/05/22 09:31, Phil Auld wrote: > On Wed, May 25, 2022 at 10:48:31AM +0100 Valentin Schneider wrote: >> >> Yeah it would be neater to not even enter cpu_{up, down}(), but my paranoia >> makes me think we need the comparison to happen with at least the >> cpu_add_remove_lock held to make sure st->state isn't moving under our >> feet, otherwise we may still end up with target == state in _cpu_down() and >> hit the bug you're describing. >> > > This is what I was originally doing before I tried to "optimize" it: > > if (st->state < target) > ret = cpu_up(dev->id, target); > else if (st->state > target) > ret = cpu_down(dev->id, target); > > This does the check under the lock and just falls through if state==target. > I think I'll go back to that version. > > I also noticed while testing that the boot cpu does not get its target set. > It's got state 233 but target 0. So reading that out and writing it back > on offlines cpu0. I'll try to find where that is not getting set. >
If I had to guess I'd say it's because the boot CPU doesn't go through the regular hotplug machinery and sets its state straight to CPUHP_ONLINE
/me digs
Maybe around this?
void __init boot_cpu_hotplug_init(void) { this_cpu_write(cpuhp_state.booted_once, true); this_cpu_write(cpuhp_state.state, CPUHP_ONLINE); }
| |