Re: [PATCH v2] cpufreq: conservative: Restrict deferred downscaling to low load

[Rafael J. Wysocki (Intel)]

On Wed, Sep 16, 2026 at 8:09 AM <hu.shengming@xxxxxxxxxx> wrote:
>
> From: Shengming Hu <hu.shengming@xxxxxxxxxx>
>
> For a shared cpufreq policy, dbs_update() derives the load used for the
> frequency decision from the maximum load among its CPUs, but it may
> also record deferred idle periods accumulated by an idle CPU in the
> policy.
>
> Consequently, a single update can contain both a high decision load
> from one CPU and multiple deferred idle periods from another CPU. The
> conservative governor applies the deferred down steps before evaluating
> the decision load. If that load subsequently triggers an up step, the
> deferred down steps can outweigh the single up step and produce a net
> frequency reduction.
>
> This was observed on a policy shared by CPUs 2 and 3. A CPU-bound
> SCHED_EXT task kept CPU 2 fully utilized while CPU 3 remained idle.
> On this system, SCHED_EXT generated update-util callbacks less
> frequently than CFS, resulting in sparse DBS updates such as:
>
>  load=100 idle_periods=7 interval=59 ms
>  load=100 idle_periods=4 interval=39 ms
>  load=100 idle_periods=2 interval=19 ms
>  load=100 idle_periods=7 interval=59 ms
>
> With the default 5% frequency step and a 2.6 GHz policy maximum, seven
> deferred periods reduce the requested frequency by seven 130 MHz steps
> before the high decision load adds only one step. Repeated updates
> therefore keep the policy near 530 MHz even though CPU 2 is fully
> utilized.
>
> There are two load values relevant to this behavior:
>
>   * sample_load is calculated from the busy and elapsed time of the
>     current sample.
>
>   * decision_load is returned by dbs_update() for the governor's
>     frequency decision. After a long idle interval, it may reuse
>     prev_load to improve the response to a waking task and can therefore
>     differ from sample_load.
>
> The problem is that idle_periods is derived from accumulated idle time
> and is applied independently of whether the current sample load is in
> the conservative governor's downscaling region. A high sample load and
> multiple deferred idle periods can therefore coexist, particularly
> when updates are sparse.
>
> As suggested by Zhongqiu Han, calculate the sample load separately from
> the decision load. If no time has elapsed, use prev_load for the sample
> load as the existing load calculation does, since no load can be
> calculated for that interval.
>
> Record the maximum sample load for the policy and apply deferred down
> steps only when it is below down_threshold. This makes deferred
> downscaling follow the same threshold that the conservative governor
> normally uses to determine whether the policy load is low enough to
> reduce the frequency.
>
> The existing code applies deferred down steps whenever idle_periods is
> valid, regardless of the sample load. When the decision load is in the
> hold region, the locally adjusted frequency is not submitted unless
> another frequency-update branch is taken. Thus, when the sample and
> decision loads are the same and both are in the hold region, this
> change does not alter the requested frequency.
>
> The difference becomes visible when the decision load causes a
> frequency update. In particular, if it exceeds up_threshold, the
> governor applies a single up step after the deferred down steps and
> submits the resulting request. Multiple deferred down steps can then
> outweigh that up step and cause a frequency reduction even when the
> sample load is above down_threshold.
>
> With this change, deferred down steps are skipped whenever the maximum
> sample load is at or above down_threshold, keeping deferred downscaling
> consistent with the conservative governor's threshold semantics. The
> requested frequency may therefore be higher than with the existing
> behavior when deferred idle periods are present and the decision load
> causes a frequency update. This is intentional, because the lower
> request previously resulted from applying deferred downscaling while
> the sampled policy load was outside the governor's downscaling region.
> Accelerated deferred downscaling remains available when the policy
> sample load is below down_threshold, including after a workload has
> completed and the policy has become idle.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 00bfe05889e9 ("cpufreq: conservative: Decrease frequency faster for deferred updates")
> Reviewed-by: Luo Haiyang <luo.haiyang@xxxxxxxxxx>
> Reviewed-by: Run Zhang <zhang.run@xxxxxxxxxx>
> Suggested-by: Zhongqiu Han <zhongqiu.han@xxxxxxxxxxxxxxxx>
> Signed-off-by: Shengming Hu <hu.shengming@xxxxxxxxxx>

Applied as 7.4 material, thanks!

> ---
> Changes in v2:
>
> - Calculate the sample load separately from the decision load that may
>   reuse prev_load after a long idle interval, as suggested by Zhongqiu Han.
> - Preserve the existing prev_load fallback when time_elapsed is zero.
> - Gate deferred down steps on down_threshold.
> - Link to v1: https://lore.kernel.org/all/2026090215474182681fN7LLOSpqIc3s3OqJaW@xxxxxxxxxx/
>
> ---
>  drivers/cpufreq/cpufreq_conservative.c |  8 ++++---
>  drivers/cpufreq/cpufreq_governor.c     | 29 +++++++++++++++++++-------
>  drivers/cpufreq/cpufreq_governor.h     |  2 ++
>  3 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 0b32ae28ec85..02bfd46543e9 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -85,10 +85,12 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>         freq_step = get_freq_step(cs_tuners, policy);
>
>         /*
> -        * Decrease requested_freq one freq_step for each idle period that
> -        * we didn't update the frequency.
> +        * Apply deferred down steps only when the policy sample load is
> +        * below down_threshold. Otherwise, multiple deferred down steps may
> +        * cause a net frequency decrease outside the downscaling region.
>          */
> -       if (policy_dbs->idle_periods < UINT_MAX) {
> +       if (policy_dbs->max_sample_load < cs_tuners->down_threshold &&
> +           policy_dbs->idle_periods < UINT_MAX) {
>                 unsigned int freq_steps = policy_dbs->idle_periods * freq_step;
>
>                 if (requested_freq > policy->min + freq_steps)
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index 710d93ec89b5..f3bd3dbe7c41 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -124,7 +124,8 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>         struct policy_dbs_info *policy_dbs = policy->governor_data;
>         struct dbs_data *dbs_data = policy_dbs->dbs_data;
>         unsigned int ignore_nice = dbs_data->ignore_nice_load;
> -       unsigned int max_load = 0, idle_periods = UINT_MAX;
> +       unsigned int max_load = 0, max_sample_load = 0;
> +       unsigned int idle_periods = UINT_MAX;
>         unsigned int sampling_rate, io_busy, j;
>         u64 cur_nice;
>
> @@ -147,7 +148,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>                 struct cpu_dbs_info *j_cdbs = &per_cpu(cpu_dbs, j);
>                 u64 update_time, cur_idle_time;
>                 unsigned int idle_time, time_elapsed;
> -               unsigned int load;
> +               unsigned int load, sample_load;
>
>                 cur_idle_time = get_cpu_idle_time(j, &update_time, io_busy);
>
> @@ -186,6 +187,20 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>
>                 j_cdbs->prev_cpu_nice = cur_nice;
>
> +               /*
> +                * Compute the sample load separately from the prev_load value
> +                * that may be reused after a long idle interval. The conservative
> +                * governor uses it to decide whether to apply deferred down steps.
> +                * If no time has elapsed, retain the existing behavior and use
> +                * prev_load.
> +                */
> +               if (unlikely(!time_elapsed))
> +                       sample_load = j_cdbs->prev_load;
> +               else if (time_elapsed > idle_time)
> +                       sample_load = 100 * (time_elapsed - idle_time) / time_elapsed;
> +               else
> +                       sample_load = 0;
> +
>                 if (unlikely(!time_elapsed)) {
>                         /*
>                          * That can only happen when this function is called
> @@ -220,11 +235,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>                         load = j_cdbs->prev_load;
>                         j_cdbs->prev_load = 0;
>                 } else {
> -                       if (time_elapsed > idle_time)
> -                               load = 100 * (time_elapsed - idle_time) / time_elapsed;
> -                       else
> -                               load = 0;
> -
> +                       load = sample_load;
>                         j_cdbs->prev_load = load;
>                 }
>
> @@ -237,9 +248,13 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
>
>                 if (load > max_load)
>                         max_load = load;
> +
> +               if (sample_load > max_sample_load)
> +                       max_sample_load = sample_load;
>         }
>
>         policy_dbs->idle_periods = idle_periods;
> +       policy_dbs->max_sample_load = max_sample_load;
>
>         return max_load;
>  }
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index 73b8ed7cfaae..806d8fb4dff1 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -95,6 +95,8 @@ struct policy_dbs_info {
>         /* Multiplier for increasing sample delay temporarily. */
>         unsigned int rate_mult;
>         unsigned int idle_periods;      /* For conservative */
> +       /* Maximum load from the current policy sample. */
> +       unsigned int max_sample_load;
>         /* Status indicators */
>         bool is_shared;         /* This object is used by multiple CPUs */
>         bool work_in_progress;  /* Work is being queued up or in progress */
> --
> 2.25.1
>