Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH] cpuhp: make target_store() a nop when target == state | Date | Wed, 25 May 2022 10:48:31 +0100 |
| |
On 24/05/22 12:39, Phil Auld wrote: > On Tue, May 24, 2022 at 04:11:51PM +0100 Valentin Schneider wrote: > >> >> _cpu_up() has: >> >> /* >> * The caller of cpu_up() might have raced with another >> * caller. Nothing to do. >> */ >> if (st->state >= target) >> goto out; >> >> Looks like we want an equivalent in _cpu_down(), what do you think? > > Maybe. I still think that > >> > if (st->state < target) >> > ret = cpu_up(dev->id, target); >> > else >> > ret = cpu_down(dev->id, target); > > is not correct. If we catch the == case earlier then this makes > sense as is. > > I suppose "if (st->state <= target)" would work too since __cpu_up() > already checks. Catching this sooner seems better to me though. >
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.
>> >> > ret = lock_device_hotplug_sysfs(); >> > if (ret) >> > return ret; >> > -- >> > 2.18.0 >> > > > Cheers, > Phil > > --
| |