Re: [PATCH v2 2/6] cpufreq/sched: Explicitly synchronize limits_changed flag handling
From: Christian Loehle
Date: Wed Apr 16 2025 - 08:02:04 EST
On 4/15/25 10:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> The handling of the limits_changed flag in struct sugov_policy needs to
> be explicitly synchronized to ensure that cpufreq policy limits updates
> will not be missed in some cases.
>
> Without that synchronization it is theoretically possible that
> the limits_changed update in sugov_should_update_freq() will be
> reordered with respect to the reads of the policy limits in
> cpufreq_driver_resolve_freq() and in that case, if the limits_changed
> update in sugov_limits() clobbers the one in sugov_should_update_freq(),
> the new policy limits may not take effect for a long time.
>
> Likewise, the limits_changed update in sugov_limits() may theoretically
> get reordered with respect to the updates of the policy limits in
> cpufreq_set_policy() and if sugov_should_update_freq() runs between
> them, the policy limits change may be missed.
>
> To ensure that the above situations will not take place, add memory
> barriers preventing the reordering in question from taking place and
> add READ_ONCE() and WRITE_ONCE() annotations around all of the
> limits_changed flag updates to prevent the compiler from messing up
> with that code.
>
> Fixes: 600f5badb78c ("cpufreq: schedutil: Don't skip freq update when limits change")
> Cc: 5.3+ <stable@xxxxxxxxxxxxxxx> # 5.3+
typo in the address here.
I don't fully understand why we wouldn't want this in 6.15-rc already,
even if the actual impact may be limited?
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Reviewed-by: Christian Loehle <christian.loehle@xxxxxxx>
>[snip]