diff mbox series

[v1,1/5] cpufreq/sched: Check fast_switch_enabled when setting need_freq_update

Message ID 8533207.T7Z3S40VBb@rjwysocki.net
State New
Headers show
Series cpufreq/sched: Improve synchronization of policy limits updates with schedutil | expand

Commit Message

Rafael J. Wysocki April 14, 2025, 8:41 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit 8e461a1cb43d ("cpufreq: schedutil: Fix superfluous updates caused
by need_freq_update") overlooked the fact that when fast swtching is
enabled, it is the only way to pick up new policy limits and so
need_freq_update needs to be set in that case when limits_changed is
set.

This causes policy limits updates to be missed in some cases, so
make sugov_should_update_freq() also set need_freq_update when the
fast_switch_enabled policy flag is set.

Fixes: 8e461a1cb43d ("cpufreq: schedutil: Fix superfluous updates caused by need_freq_update")
Closes: https://lore.kernel.org/lkml/Z_Tlc6Qs-tYpxWYb@linaro.org/
Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/sched/cpufreq_schedutil.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Rafael J. Wysocki April 15, 2025, 9:12 a.m. UTC | #1
On Mon, Apr 14, 2025 at 10:52 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit 8e461a1cb43d ("cpufreq: schedutil: Fix superfluous updates caused
> by need_freq_update") overlooked the fact that when fast swtching is
> enabled, it is the only way to pick up new policy limits and so
> need_freq_update needs to be set in that case when limits_changed is
> set.
>
> This causes policy limits updates to be missed in some cases, so
> make sugov_should_update_freq() also set need_freq_update when the
> fast_switch_enabled policy flag is set.

Earlier today I realized that this patch would not be sufficient
because if the policy limits change, schedutil needed to invoke the
driver callback for the new limits to take effect regardless of
whether or not fast switching had been enabled.

After making this observation I've realized that there's a better fix
that covers all of the relevant cases, but it requires patch [2/5] to
be rebased and one more can be made between the new fix and patch
[2/5].

So there will be a v2.

> Fixes: 8e461a1cb43d ("cpufreq: schedutil: Fix superfluous updates caused by need_freq_update")
> Closes: https://lore.kernel.org/lkml/Z_Tlc6Qs-tYpxWYb@linaro.org/
> Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  kernel/sched/cpufreq_schedutil.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -83,7 +83,9 @@
>
>         if (unlikely(sg_policy->limits_changed)) {
>                 sg_policy->limits_changed = false;
> -               sg_policy->need_freq_update = cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS);
> +               sg_policy->need_freq_update =
> +                       sg_policy->policy->fast_switch_enabled ||
> +                       cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS);
>                 return true;
>         }
>
>
>
>
>
diff mbox series

Patch

--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -83,7 +83,9 @@ 
 
 	if (unlikely(sg_policy->limits_changed)) {
 		sg_policy->limits_changed = false;
-		sg_policy->need_freq_update = cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS);
+		sg_policy->need_freq_update =
+			sg_policy->policy->fast_switch_enabled ||
+			cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS);
 		return true;
 	}