* [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
@ 2026-10-28 3:44 Qing Wang
2026-10-28 9:01 ` Kayra Cizmeci
2026-10-28 9:05 ` Peter Zijlstra
0 siblings, 2 replies; 7+ messages in thread
From: Qing Wang @ 2026-10-28 3:44 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: Tim Chen, Chen Yu, Davi Chaves Azevedo, linux-kernel, Qing Wang
The current NI_RANDOM implementation uses a random dice roll to allow
newidle_balance attempts according to the success rate. There is a better
way to implement it.
Replace the random dice with a Bresenham accumulator that distributes the
allowed attempts uniformly across a 1024-step window:
Each step does the following:
- Accumulate (1 + newidle_ratio) into newidle_window_pos.
- If the accumulator reaches 1024, allow the balance attempt and
subtract 1024.
This guarantees exactly (1 + newidle_ratio) newidle_balance operations per
1024 steps, uniformly spaced for any ratio in [0, 1023].
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Qing Wang <wangqing7171@gmail.com>
---
v2:
Removed sched_rnd_state and sched_rng() since they are no longer used.
v3:
Corrected the code comments.
Performance testing results from Prateek using tbench
(most sensitive workload for newidle balance):
Clients: tip bresenham_accm
1 321.65 (0.00 pct) 313.97 (-2.38 pct)
2 641.92 (0.00 pct) 638.74 (-0.49 pct)
4 1245.65 (0.00 pct) 1237.26 (-0.67 pct)
8 2435.80 (0.00 pct) 2442.23 ( 0.26 pct)
16 4717.66 (0.00 pct) 4688.19 (-0.62 pct)
32 9303.53 (0.00 pct) 9390.71 ( 0.93 pct)
64 18002.57 (0.00 pct) 17911.56 (-0.50 pct)
128 27729.26 (0.00 pct) 27621.95 (-0.38 pct)
256 47134.77 (0.00 pct) 46137.36 (-2.11 pct)
512 43179.41 (0.00 pct) 43277.53 ( 0.22 pct)
1024 40339.30 (0.00 pct) 40176.49 (-0.40 pct)
The %diff is within the noise range, indicating no performance regression.
include/linux/sched/topology.h | 1 +
kernel/sched/core.c | 3 ---
kernel/sched/fair.c | 19 ++++++++++++++-----
kernel/sched/sched.h | 6 ------
kernel/sched/topology.c | 1 +
5 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index f96812d71c51..e951d6f3fb0d 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -113,6 +113,7 @@ struct sched_domain {
unsigned int newidle_call;
unsigned int newidle_success;
unsigned int newidle_ratio;
+ unsigned int newidle_window_pos;
u64 newidle_stamp;
u64 max_newidle_lb_cost;
unsigned long last_decay_max_lb_cost;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1754d9f95f2c..4d764e68897c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -128,7 +128,6 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_dl_server_start_tp);
EXPORT_TRACEPOINT_SYMBOL_GPL(sched_dl_server_stop_tp);
DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
-DEFINE_PER_CPU(struct rnd_state, sched_rnd_state);
#ifdef CONFIG_SCHED_PROXY_EXEC
DEFINE_STATIC_KEY_TRUE(__sched_proxy_exec);
@@ -8927,8 +8926,6 @@ void __init sched_init_smp(void)
{
sched_init_numa(NUMA_NO_NODE);
- prandom_init_once(&sched_rnd_state);
-
/*
* There's no userspace yet to cause hotplug operations; hence all the
* CPU masks are stable and all blatant races in the below code cannot
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0f2364479f9d..172c4b53c11a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -14198,6 +14198,7 @@ static inline void update_newidle_stats(struct sched_domain *sd, unsigned int su
ratio += sd->newidle_success;
sd->newidle_ratio = min(1024, ratio);
+ sd->newidle_window_pos = 0;
sd->newidle_call /= 2;
sd->newidle_success /= 2;
}
@@ -14928,16 +14929,24 @@ static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
if (sched_feat(NI_RANDOM) && sd->newidle_ratio < 1024) {
/*
- * Throw a 1k sided dice; and only run
- * newidle_balance according to the success
- * rate.
+ * Bresenham's line algorithm based accumulator
+ *
+ * At each invocation, the ratio is added to an
+ * accumulator. A balance is triggered whenever
+ * the sum >= 1024.
+ *
+ * This guarantees exactly 'ratio' balance
+ * operations per 1024 calls, distributing them
+ * as uniformly as possible across the interval.
*/
- u32 d1k = sched_rng() % 1024;
weight = 1 + sd->newidle_ratio;
- if (d1k > weight) {
+
+ sd->newidle_window_pos += weight;
+ if (sd->newidle_window_pos < 1024) {
update_newidle_stats(sd, 0);
continue;
}
+ sd->newidle_window_pos -= 1024;
weight = (1024 + weight/2) / weight;
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 4c25fbe84fb5..210aa35b5720 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1424,12 +1424,6 @@ static inline bool is_migration_disabled(struct task_struct *p)
}
DECLARE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
-DECLARE_PER_CPU(struct rnd_state, sched_rnd_state);
-
-static inline u32 sched_rng(void)
-{
- return prandom_u32_state(this_cpu_ptr(&sched_rnd_state));
-}
static __always_inline struct rq *__this_rq(void)
{
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 3dab0253976f..8cc5315a6945 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1984,6 +1984,7 @@ sd_init(struct sched_domain_topology_level *tl,
.newidle_call = 512,
.newidle_success = 256,
.newidle_ratio = 512,
+ .newidle_window_pos = 0,
.newidle_stamp = now,
.max_newidle_lb_cost = 0,
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
2026-10-28 3:44 [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator Qing Wang
@ 2026-10-28 9:01 ` Kayra Cizmeci
2026-10-29 7:30 ` Qing Wang
2026-10-28 9:05 ` Peter Zijlstra
1 sibling, 1 reply; 7+ messages in thread
From: Kayra Cizmeci @ 2026-10-28 9:01 UTC (permalink / raw)
To: Qing Wang
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Tim Chen, Chen Yu,
Davi Chaves Azevedo, linux-kernel
On Wed, Oct 28, 2026 at 11:44:49AM +0800, Qing Wang wrote:
> The current NI_RANDOM implementation uses a random dice roll to allow
> newidle_balance attempts according to the success rate. There is a better
> way to implement it.
>
> Replace the random dice with a Bresenham accumulator that distributes the
> allowed attempts uniformly across a 1024-step window:
>
> Each step does the following:
> - Accumulate (1 + newidle_ratio) into newidle_window_pos.
> - If the accumulator reaches 1024, allow the balance attempt and
> subtract 1024.
>
> This guarantees exactly (1 + newidle_ratio) newidle_balance operations per
> 1024 steps, uniformly spaced for any ratio in [0, 1023].
I still quite don't understand why we need this. If there's not a gain,
the old code is simpler. I'm not against this but still I'm just asking.
Changelog just says "better way to implement it" but doesn't say why
it is better.
>
> Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Qing Wang <wangqing7171@gmail.com>
> ---
> v2:
> Removed sched_rnd_state and sched_rng() since they are no longer used.
>
> v3:
> Corrected the code comments.
>
> Performance testing results from Prateek using tbench
> (most sensitive workload for newidle balance):
>
> Clients: tip bresenham_accm
> 1 321.65 (0.00 pct) 313.97 (-2.38 pct)
> 2 641.92 (0.00 pct) 638.74 (-0.49 pct)
> 4 1245.65 (0.00 pct) 1237.26 (-0.67 pct)
> 8 2435.80 (0.00 pct) 2442.23 ( 0.26 pct)
> 16 4717.66 (0.00 pct) 4688.19 (-0.62 pct)
> 32 9303.53 (0.00 pct) 9390.71 ( 0.93 pct)
> 64 18002.57 (0.00 pct) 17911.56 (-0.50 pct)
> 128 27729.26 (0.00 pct) 27621.95 (-0.38 pct)
> 256 47134.77 (0.00 pct) 46137.36 (-2.11 pct)
> 512 43179.41 (0.00 pct) 43277.53 ( 0.22 pct)
> 1024 40339.30 (0.00 pct) 40176.49 (-0.40 pct)
>
> The %diff is within the noise range, indicating no performance regression.
>
>
> include/linux/sched/topology.h | 1 +
> kernel/sched/core.c | 3 ---
> kernel/sched/fair.c | 19 ++++++++++++++-----
> kernel/sched/sched.h | 6 ------
> kernel/sched/topology.c | 1 +
> 5 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index f96812d71c51..e951d6f3fb0d 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -113,6 +113,7 @@ struct sched_domain {
> unsigned int newidle_call;
> unsigned int newidle_success;
> unsigned int newidle_ratio;
> + unsigned int newidle_window_pos;
> u64 newidle_stamp;
> u64 max_newidle_lb_cost;
> unsigned long last_decay_max_lb_cost;
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1754d9f95f2c..4d764e68897c 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -128,7 +128,6 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_dl_server_start_tp);
> EXPORT_TRACEPOINT_SYMBOL_GPL(sched_dl_server_stop_tp);
>
> DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
> -DEFINE_PER_CPU(struct rnd_state, sched_rnd_state);
>
> #ifdef CONFIG_SCHED_PROXY_EXEC
> DEFINE_STATIC_KEY_TRUE(__sched_proxy_exec);
> @@ -8927,8 +8926,6 @@ void __init sched_init_smp(void)
> {
> sched_init_numa(NUMA_NO_NODE);
>
> - prandom_init_once(&sched_rnd_state);
> -
> /*
> * There's no userspace yet to cause hotplug operations; hence all the
> * CPU masks are stable and all blatant races in the below code cannot
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 0f2364479f9d..172c4b53c11a 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -14198,6 +14198,7 @@ static inline void update_newidle_stats(struct sched_domain *sd, unsigned int su
> ratio += sd->newidle_success;
>
> sd->newidle_ratio = min(1024, ratio);
> + sd->newidle_window_pos = 0;
Why do we need to make this 0? We need
512 calls for this to happen. But for balance to run we need
newidle_window_pos to be 1024.
So, we reset newidle_window_pos per 512 calls but we need it to be 1024
in order for balance to work When weight is 1, the newidle balance just
doesn't run.
Or am I missing something :-)?
Thanks,
Kayra
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
2026-10-28 3:44 [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator Qing Wang
2026-10-28 9:01 ` Kayra Cizmeci
@ 2026-10-28 9:05 ` Peter Zijlstra
2026-10-29 8:41 ` Qing Wang
1 sibling, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2026-10-28 9:05 UTC (permalink / raw)
To: Qing Wang
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Tim Chen, Chen Yu, Davi Chaves Azevedo,
linux-kernel
On Wed, Oct 28, 2026 at 11:44:49AM +0800, Qing Wang wrote:
> The current NI_RANDOM implementation uses a random dice roll to allow
> newidle_balance attempts according to the success rate. There is a better
> way to implement it.
Well, sorta. So Bresenham is a linear algorithm; it doesn't (and cannot)
properly deal with slope changes, and that is exactly what we do here.
Your solution:
> @@ -14198,6 +14198,7 @@ static inline void update_newidle_stats(struct sched_domain *sd, unsigned int su
> ratio += sd->newidle_success;
>
> sd->newidle_ratio = min(1024, ratio);
> + sd->newidle_window_pos = 0;
> sd->newidle_call /= 2;
> sd->newidle_success /= 2;
> }
is to simply delete the residual on slope change. If we were drawing
actual lines (like Bresenham was), everybody would be yelling at you for
creating jaggies.
Now, we can rescale the residual, but realistically this probably
doesn't matter.
> Replace the random dice with a Bresenham accumulator that distributes the
> allowed attempts uniformly across a 1024-step window:
>
> Each step does the following:
> - Accumulate (1 + newidle_ratio) into newidle_window_pos.
> - If the accumulator reaches 1024, allow the balance attempt and
> subtract 1024.
>
> This guarantees exactly (1 + newidle_ratio) newidle_balance operations per
> 1024 steps, uniformly spaced for any ratio in [0, 1023].
Right, except you get the boundaries wrong. Also; as you mention, the
new pattern is uniform, which might (but probably doesn't) matter.
But the thing is; you didn't call out any of this. Can you please update
the Changelog to be explicit about these things and argue why it doesn't
matter.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
2026-10-28 9:01 ` Kayra Cizmeci
@ 2026-10-29 7:30 ` Qing Wang
2026-10-29 8:41 ` Kayra Cizmeci
0 siblings, 1 reply; 7+ messages in thread
From: Qing Wang @ 2026-10-29 7:30 UTC (permalink / raw)
To: kayracizmeci
Cc: bsegall, davichazbh, dietmar.eggemann, juri.lelli,
kprateek.nayak, linux-kernel, mgorman, mingo, peterz, rostedt,
tim.c.chen, vincent.guittot, vschneid, wangqing7171, yu.c.chen
On Wed, 28 Oct 2026 at 17:01, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
> On Wed, Oct 28, 2026 at 11:44:49AM +0800, Qing Wang wrote:
> > The current NI_RANDOM implementation uses a random dice roll to allow
> > newidle_balance attempts according to the success rate. There is a better
> > way to implement it.
> >
> > Replace the random dice with a Bresenham accumulator that distributes the
> > allowed attempts uniformly across a 1024-step window:
> >
> > Each step does the following:
> > - Accumulate (1 + newidle_ratio) into newidle_window_pos.
> > - If the accumulator reaches 1024, allow the balance attempt and
> > subtract 1024.
> >
> > This guarantees exactly (1 + newidle_ratio) newidle_balance operations per
> > 1024 steps, uniformly spaced for any ratio in [0, 1023].
>
> I still quite don't understand why we need this. If there's not a gain,
> the old code is simpler. I'm not against this but still I'm just asking.
> Changelog just says "better way to implement it" but doesn't say why
> it is better.
Sorry for not explaining this clearly in the changelog. My reasoning is:
1. It is deterministic and reproducible. For a given ratio, the
balance frequency is exact and not subject to random variance,
which makes the behavior predictable when debugging or tracing.
2. The attempts are evenly spaced, without bursts. With Bresenham the
interval between two balance attempts is uniform, whereas with the
random dice the number of skipped attempts follows a geometric
distribution: at low ratios we may go for a long stretch without
balancing, or balance several times in a row.
Also, looking through the scheduler code, I found very few places that
rely on random-number-based probabilistic decisions like this. IMHO
that approach is not a great fit for a kernel mechanism where
predictability matters. Using a window and spreading a fixed number of
allowed attempts evenly within it seems more reasonable to me.
I'll put this rationale into the changelog in the next version.
> Why do we need to make this 0? We need
> 512 calls for this to happen. But for balance to run we need
> newidle_window_pos to be 1024.
>
> So, we reset newidle_window_pos per 512 calls but we need it to be 1024
> in order for balance to work When weight is 1, the newidle balance just
> doesn't run.
>
> Or am I missing something :-)?
You're right, this is a bug. I wrongly assumed that newidle_ratio is
updated once every 1024 calls, so I reset newidle_window_pos in the
update path. Since newidle_call is halved after each update, the update
actually happens every 512 calls, and the reset discards the
accumulated value before it can reach 1024.
The reset isn't needed at all. newidle_window_pos always stays within
[0, 1024), since weight <= 1024 and we subtract 1024 whenever it
reaches the threshold, so the residue can simply carry over across
ratio updates. Dropping the reset fixes the issue. I'll send a v4 with
it removed.
--
Best regards,
Qing
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
2026-10-28 9:05 ` Peter Zijlstra
@ 2026-10-29 8:41 ` Qing Wang
0 siblings, 0 replies; 7+ messages in thread
From: Qing Wang @ 2026-10-29 8:41 UTC (permalink / raw)
To: peterz
Cc: bsegall, davichazbh, dietmar.eggemann, juri.lelli,
kprateek.nayak, linux-kernel, mgorman, mingo, rostedt,
tim.c.chen, vincent.guittot, vschneid, wangqing7171, yu.c.chen
Hi Peter,
Thanks for the review. The newidle balance throttling you added is a
really nice optimization. I only have a slightly different view on
how to implement it.
On Wed, 28 Oct 2026 at 17:05, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Oct 28, 2026 at 11:44:49AM +0800, Qing Wang wrote:
> > The current NI_RANDOM implementation uses a random dice roll to allow
> > newidle_balance attempts according to the success rate. There is a better
> > way to implement it.
>
> Well, sorta. So Bresenham is a linear algorithm; it doesn't (and cannot)
> properly deal with slope changes, and that is exactly what we do here.
>
> Your solution:
>
> > @@ -14198,6 +14198,7 @@ static inline void update_newidle_stats(struct sched_domain *sd, unsigned int su
> > ratio += sd->newidle_success;
> >
> > sd->newidle_ratio = min(1024, ratio);
> > + sd->newidle_window_pos = 0;
> > sd->newidle_call /= 2;
> > sd->newidle_success /= 2;
> > }
>
> is to simply delete the residual on slope change. If we were drawing
> actual lines (like Bresenham was), everybody would be yelling at you for
> creating jaggies.
>
> Now, we can rescale the residual, but realistically this probably
> doesn't matter.
Sorry, the "sd->newidle_window_pos = 0" in update_newidle_stats() is
a bug and I'll send a v4 with it removed. I wrongly assumed that
newidle_ratio is updated once every 1024 calls. Since newidle_call
is halved after each update, the update actually happens every 512
calls, and the reset discards the accumulated value before it can
reach 1024.
Right, Bresenham assumes a constant slope. But the only effect of a
slope change is that the first attempt after it may come a bit earlier
or later.
A simulation where the ratio jumps to a random value in [0, 1023]
every 512 calls (20000 windows, ~10M calls):
accumulator random dice
max per-window error 0.5 attempts 41 attempts (~16%)
> > Replace the random dice with a Bresenham accumulator that distributes the
> > allowed attempts uniformly across a 1024-step window:
> >
> > Each step does the following:
> > - Accumulate (1 + newidle_ratio) into newidle_window_pos.
> > - If the accumulator reaches 1024, allow the balance attempt and
> > subtract 1024.
> >
> > This guarantees exactly (1 + newidle_ratio) newidle_balance operations per
> > 1024 steps, uniformly spaced for any ratio in [0, 1023].
>
> Right, except you get the boundaries wrong. Also; as you mention, the
> new pattern is uniform, which might (but probably doesn't) matter.
I don't understand which boundaries you mean here. Could you spell
it out?
> But the thing is; you didn't call out any of this. Can you please update
> the Changelog to be explicit about these things and argue why it doesn't
> matter.
I'll add more explanation in the v4 changelog.
--
Best regards,
Qing
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
2026-10-29 7:30 ` Qing Wang
@ 2026-10-29 8:41 ` Kayra Cizmeci
2026-10-29 9:31 ` Qing Wang
0 siblings, 1 reply; 7+ messages in thread
From: Kayra Cizmeci @ 2026-10-29 8:41 UTC (permalink / raw)
To: Qing Wang
Cc: bsegall, davichazbh, dietmar.eggemann, juri.lelli,
kprateek.nayak, linux-kernel, mgorman, mingo, peterz, rostedt,
tim.c.chen, vincent.guittot, vschneid, yu.c.chen
Hi Qing,
On Thu, Oct 29, 2026 at 03:30:17PM +0800, Qing Wang wrote:
> On Wed, 28 Oct 2026 at 17:01, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
> > On Wed, Oct 28, 2026 at 11:44:49AM +0800, Qing Wang wrote:
> > > The current NI_RANDOM implementation uses a random dice roll to allow
> > > newidle_balance attempts according to the success rate. There is a better
> > > way to implement it.
> > >
> > > Replace the random dice with a Bresenham accumulator that distributes the
> > > allowed attempts uniformly across a 1024-step window:
> > >
> > > Each step does the following:
> > > - Accumulate (1 + newidle_ratio) into newidle_window_pos.
> > > - If the accumulator reaches 1024, allow the balance attempt and
> > > subtract 1024.
> > >
> > > This guarantees exactly (1 + newidle_ratio) newidle_balance operations per
> > > 1024 steps, uniformly spaced for any ratio in [0, 1023].
> >
> > I still quite don't understand why we need this. If there's not a gain,
> > the old code is simpler. I'm not against this but still I'm just asking.
> > Changelog just says "better way to implement it" but doesn't say why
> > it is better.
>
> Sorry for not explaining this clearly in the changelog. My reasoning is:
>
> 1. It is deterministic and reproducible. For a given ratio, the
> balance frequency is exact and not subject to random variance,
> which makes the behavior predictable when debugging or tracing.
>
> 2. The attempts are evenly spaced, without bursts. With Bresenham the
> interval between two balance attempts is uniform, whereas with the
> random dice the number of skipped attempts follows a geometric
> distribution: at low ratios we may go for a long stretch without
> balancing, or balance several times in a row.
But there are no numbers showing whether the bursts have any effect or
not.
>
> Also, looking through the scheduler code, I found very few places that
> rely on random-number-based probabilistic decisions like this. IMHO
> that approach is not a great fit for a kernel mechanism where
> predictability matters. Using a window and spreading a fixed number of
> allowed attempts evenly within it seems more reasonable to me.
>
> I'll put this rationale into the changelog in the next version.
>
> > Why do we need to make this 0? We need
> > 512 calls for this to happen. But for balance to run we need
> > newidle_window_pos to be 1024.
> >
> > So, we reset newidle_window_pos per 512 calls but we need it to be 1024
> > in order for balance to work When weight is 1, the newidle balance just
> > doesn't run.
> >
> > Or am I missing something :-)?
>
> You're right, this is a bug. I wrongly assumed that newidle_ratio is
> updated once every 1024 calls, so I reset newidle_window_pos in the
> update path. Since newidle_call is halved after each update, the update
> actually happens every 512 calls, and the reset discards the
> accumulated value before it can reach 1024.
>
> The reset isn't needed at all. newidle_window_pos always stays within
> [0, 1024), since weight <= 1024 and we subtract 1024 whenever it
> reaches the threshold, so the residue can simply carry over across
> ratio updates. Dropping the reset fixes the issue. I'll send a v4 with
> it removed.
> --
> Best regards,
> Qing
IMO, I'd like to keep the current code if there are no numbers.
It isn’t worth it.
The test results of Prateek on this version don't reflect v4,
since they were taken with this bug.
BTW, this patch removes sched_rng() which breaks the build on Linus's
tree and tip:
kernel/sched/fair.c:8663:55: error: implicit declaration of function ‘sched_rng’ [-Wimplicit-function-declaration]
Thanks,
Kayra :-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator
2026-10-29 8:41 ` Kayra Cizmeci
@ 2026-10-29 9:31 ` Qing Wang
0 siblings, 0 replies; 7+ messages in thread
From: Qing Wang @ 2026-10-29 9:31 UTC (permalink / raw)
To: kayracizmeci, kprateek.nayak
Cc: bsegall, davichazbh, dietmar.eggemann, juri.lelli, linux-kernel,
mgorman, mingo, peterz, rostedt, tim.c.chen, vincent.guittot,
vschneid, wangqing7171, yu.c.chen
> IMO, I'd like to keep the current code if there are no numbers.
> It isn’t worth it.
>
> The test results of Prateek on this version don't reflect v4,
> since they were taken with this bug.
You're right, there are no on-machine test results yet. All I have is
the output of a Python program I just wrote that compares the accuracy
of the two algorithms.
It re-randomizes the ratio to a value in [0, 1023] every 512 calls
(the actual update interval, since newidle_call is halved after
each update) and runs 20000 windows (~10M calls):
Bresenham random dice
max per-window error 0.5 attempts 41 attempts (~16%)
Bresenham has a much lower error.
the python program code
```python
import random
random.seed(1)
W=512; NW=20000
pos=0; maxerr_win_b=0; maxerr_win_r=0
for _ in range(NW):
w = 1 + random.randrange(1024)
ideal = W*w/1024
fb=0; fr=0
for _ in range(W):
pos += w
if pos >= 1024: pos -= 1024; fb += 1
if random.randrange(1024) <= w-1: fr += 1
maxerr_win_b = max(maxerr_win_b, abs(fb-ideal))
maxerr_win_r = max(maxerr_win_r, abs(fr-ideal))
print(f"bresenham: max per-window err = {maxerr_win_b:.1f}")
print(f"random : max per-window err = {maxerr_win_r:.1f}")
```
Hi Prateek, I'll send a v4 soon that fixes a reset bug — it just
drops the "sd->newidle_window_pos = 0;" line in update_newidle_stats().
If you want to test before v4 lands, that's the only change needed.
Could you please re-run the tbench tests on it? Thanks!
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
> BTW, this patch removes sched_rng() which breaks the build on Linus's
> tree and tip:
>
> kernel/sched/fair.c:8663:55: error: implicit declaration of function ‘sched_rng’ [-Wimplicit-function-declaration]
>
> Thanks,
>
> Kayra :-)
Sorry about that. I'll fix the build in v4.
--
Best regards,
Qing
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-29 9:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 3:44 [PATCH v3] sched/fair: Replace random newidle_balance with Bresenham accumulator Qing Wang
2026-10-28 9:01 ` Kayra Cizmeci
2026-10-29 7:30 ` Qing Wang
2026-10-29 8:41 ` Kayra Cizmeci
2026-10-29 9:31 ` Qing Wang
2026-10-28 9:05 ` Peter Zijlstra
2026-10-29 8:41 ` Qing Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®