Re: [PATCH] cpuhp: make target_store() a nop when target == state

Valentin Schneider
2022-05-25T09:48:41Z

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
>
> --