Re: [PATCH] cpufreq: conservative: Ignore idle periods when a policy CPU is busy
From: Zhongqiu Han
Date: Sun Sep 06 2026 - 11:14:01 EST
Hi Shengming,
Thanks for the patch.
On 9/2/2026 3:47 PM, hu.shengming@xxxxxxxxxx wrote:
From: Shengming Hu <hu.shengming@xxxxxxxxxx>
For a shared cpufreq policy, dbs_update() derives the load from the
highest utilization among its CPUs, but it also records deferred idle
periods from any CPU whose idle time exceeds two sampling intervals.
This lets a single update report both a high load (from a busy CPU)
and several deferred idle periods (from an idle sibling). Since
conservative applies the deferred down steps before the up step
triggered by the high load, the down steps can outweigh the single
up step.
The issue reproduces on a policy shared by CPUs 2 and 3: a CPU-bound
SCHED_EXT task keeps CPU 2 at 100% utilization while CPU 3 stays
idle. On this system SCHED_EXT generates update-util callbacks less
frequently than CFS, so DBS updates are sparse, tracing shows:
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% step and a 2.6 GHz ceiling, conservative first
removes seven 130 MHz steps and then adds only one. Repeating this
sequence keeps the policy near 530 MHz despite CPU 2 being fully busy.
Only retain deferred idle periods when every CPU in the policy meets
the long-idle condition. This keeps the existing behavior for
single-CPU and fully idle shared policies, while preventing an idle
sibling from downscaling a policy that contains a busy CPU.
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>
Signed-off-by: Shengming Hu <hu.shengming@xxxxxxxxxx>
---
drivers/cpufreq/cpufreq_governor.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 710d93ec89b5..64eb6b5f08a4 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -126,6 +126,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
unsigned int ignore_nice = dbs_data->ignore_nice_load;
unsigned int max_load = 0, idle_periods = UINT_MAX;
unsigned int sampling_rate, io_busy, j;
+ bool all_cpus_idle = true;
u64 cur_nice;
/*
@@ -233,13 +234,15 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
if (periods < idle_periods)
idle_periods = periods;
+ } else {
+ all_cpus_idle = false;
The problem is real, but I don't think this condition is the right one.
idle_time > 2 * sampling_rate tells us how many sampling periods were
deferred for that CPU, so its negation means "this CPU was sampled on
time", not "this CPU is busy".
Since all_cpus_idle is per-policy, one such CPU is enough to discard the
deferred periods for the whole policy, and in a shared policy it is
possible. That effectively disables the optimization from 00bfe05889e9
for shared policies, which is the opposite of what we want for power.
What matters is whether the CPU was busy over the sample, that is,
whether the skipped sampling periods would have led to a frequency
reduction at all. It seems more appropriate to key that off the load
measured over the sample (kept separate from the possibly inherited one)
against up_threshold, so an idle-but-punctually-sampled sibling does not
throw the deferred periods away, while the fast downscale on wakeup is
preserved.
May I know could you comment and try this patch on your scenario? Once everyone agrees I can send this formally:
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 710d93ec89b5..f10fc335e70e 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -126,6 +126,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
unsigned int ignore_nice = dbs_data->ignore_nice_load;
unsigned int max_load = 0, idle_periods = UINT_MAX;
unsigned int sampling_rate, io_busy, j;
+ bool busy_cpu_seen = false;
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, measured_load;
cur_idle_time = get_cpu_idle_time(j, &update_time, io_busy);
@@ -186,6 +187,19 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
j_cdbs->prev_cpu_nice = cur_nice;
+
+ /*
+ * Load actually measured over this sample. The value used for
+ * making frequency decisions below may be inherited from the
+ * previous sample, so this one is needed in order to be able to
+ * tell whether or not this CPU has really been busy. Note that
+ * no per-CPU state is updated by this computation.
+ */
+ if (unlikely(!time_elapsed) || idle_time >= time_elapsed)
+ measured_load = 0;
+ else
+ measured_load = 100 * (time_elapsed - idle_time) / time_elapsed;
+
if (unlikely(!time_elapsed)) {
/*
* That can only happen when this function is called
@@ -220,14 +234,16 @@ 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 = measured_load;
j_cdbs->prev_load = load;
}
+
+ if (measured_load > dbs_data->up_threshold)
+ busy_cpu_seen = true;
+
+
if (unlikely(idle_time > 2 * sampling_rate)) {
unsigned int periods = idle_time / sampling_rate;
@@ -239,7 +255,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
max_load = load;
}
- policy_dbs->idle_periods = idle_periods;
+ policy_dbs->idle_periods = busy_cpu_seen ? UINT_MAX : idle_periods;
return max_load;
}
}
if (load > max_load)
max_load = load;
}
- policy_dbs->idle_periods = idle_periods;
+ policy_dbs->idle_periods = all_cpus_idle ? idle_periods : UINT_MAX;
return max_load;
}
--
Thx and BRs,
Zhongqiu Han