Message-ID: <sashiko-outbox-206938@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH v2 2/4] sched/fair: refill bandwidth for the periods the
 timer slept through
Reply-To: sashiko-reviews@lists.linux.dev
To: "Fernand Sieber" <sieberf@amazon.com>
Cc: sched-ext@lists.linux.dev, bpf@vger.kernel.org, tj@kernel.org, hannes@cmpxchg.org, mkoutny@suse.com
In-Reply-To: <681751a52093faada8da25d8dbccc59444c3d251.1794303670.git.sieberf@amazon.com>
References: <cover.1794303670.git.sieberf@amazon.com>
 <681751a52093faada8da25d8dbccc59444c3d251.1794303670.git.sieberf@amazon.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 10 Nov 2026 11:29:06 +0000
Precedence: list

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] sched/fair: period_timer forwarded with old period in
  tg_set_cfs_bandwidth()
--

commit 6dcac4d8cedf5c5fafb47bd8f0ac725a2be649bc
Author: Fernand Sieber <sieberf@amazon.com>

sched/fair: refill bandwidth for the periods the timer slept through

This commit refills a CFS group's accumulated bandwidth for the period
boundaries missed while its period timer was stopped, capped at quota +
burst, and updates bandwidth configuration and cpu.max.runtime reads and
writes to account for those idle periods.

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 76fcd70ff207a..75a5f5d1b8f54 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -9856,6 +9856,12 @@ static int tg_set_cfs_bandwidth(struct task_group =
*tg,
>  		cfs_bandwidth_usage_inc();
> =20
>  	scoped_guard (raw_spinlock_irq, &cfs_b->lock) {
> +		/*
> +		 * The new bandwidth applies from now on. The group keeps what
> +		 * it earned while idle under the old one.
> +		 */
> +		cfs_bandwidth_refill_idle_periods(cfs_b);
> +
>  		cfs_b->period =3D ns_to_ktime(period);
>  		cfs_b->quota =3D quota;
>  		cfs_b->burst =3D burst;

[Severity: Medium]
When cfs_b->period_active is 0, does calling
cfs_bandwidth_refill_idle_periods() before updating cfs_b->period arm
cfs_b->period_timer with the old period's expiry instead of the new one?

cfs_bandwidth_refill_idle_periods() calls hrtimer_forward_now() with the
old cfs_b->period, pushing period_timer's expiry into (now, now +
old_period].  When tg_set_cfs_bandwidth() then calls
start_cfs_bandwidth() after updating cfs_b->period:

kernel/sched/core.c:tg_set_cfs_bandwidth() {
	...
		__refill_cfs_bandwidth_runtime(cfs_b);

		/*
		 * Restart the period timer (if active) to handle new
		 * period expiry:
		 */
		if (runtime_enabled)
			start_cfs_bandwidth(cfs_b);
	}
}

the second cfs_bandwidth_refill_idle_periods() call inside
start_cfs_bandwidth() sees an expiry already in the future, so
hrtimer_forward_now() returns 0 without adjusting it to the new period.
When a newly created cgroup (initialized with a 100ms period) or an idle
cgroup is configured with a shorter period such as 1ms,
hrtimer_start_expires() arms period_timer up to old_period in the future,
starving the group of refills for that interval after its initial quota is
spent.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1794303670.gi=
t.sieberf@amazon.com?part=3D2