Re: [PATCH v3 20/24] sched/deadline: Count the bandwidth a boosted task is given

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sched/deadline: uncounted bandwidth skipped in sched_dl_overflow()
--

commit 58e85ecec73a900cbca47b57215c8b485328d349
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

sched/deadline: Count the bandwidth a boosted task is given

This commit moves the runqueue bandwidth accounting update from
sched_dl_overflow() to __setparam_dl() so that tasks boosted into
SCHED_DEADLINE via priority inheritance have their new bandwidth added to
rq->dl.this_bw when their parameters are updated. It also charges new_bw when
dl_se->dl_bw_oncpu is not set and skips special SCHED_FLAG_SUGOV requests.

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index f772a944a3e53..746bffb03b7cb 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c

[ ... ]

> @@ -4283,6 +4292,20 @@ int sched_dl_overflow(struct task_struct *p, int policy,
>  void __setparam_dl(struct task_struct *p, const struct sched_attr *attr)
>  {
>  	struct sched_dl_entity *dl_se = &p->dl;
> +	u64 period = attr->sched_period ?: attr->sched_deadline;
> +
> +	/*
> +	 * The bandwidth of a special entity is counted on no runqueue, and
> +	 * the request tells one, the flags of the entity being those of the
> +	 * parameters it had. The rest have theirs changed here, where their
> +	 * parameters are:
> +	 * switched_to_dl() hands it to the runqueue of a task that enters
> +	 * the class, and a task that priority inheritance boosted into the
> +	 * class is in it already, so nothing dequeues it and enqueues it
> +	 * back for that.
> +	 */
> +	if (!(attr->sched_flags & SCHED_FLAG_SUGOV))
> +		dl_change_utilization(p, to_ratio(period, attr->sched_runtime));

[Severity: High]
When a sugov kthread is given a reservation of its own via sched_setattr()
without SCHED_FLAG_SUGOV, or when a deadline task whose bandwidth was dropped
by a root domain rebuild updates its parameters, what happens if the new
runtime-to-period ratio matches the existing p->dl.dl_bw?

For example, sugov_kthread_create() initializes a sugov kthread with
SCHED_DEADLINE, SCHED_FLAG_SUGOV, sched_runtime = 1ms, and sched_period =
10ms. In sched_dl_overflow(), the SCHED_FLAG_SUGOV check returns 0 early,
leaving p->dl.dl_bw_cpu at -1 and not adding to dl_b->total_bw, while
__setparam_dl() still sets p->dl.dl_bw to to_ratio(10ms, 1ms).

If sched_setattr() is later called on that kthread without SCHED_FLAG_SUGOV
and with the same 1:10 ratio, dl_param_changed() returns true because the
flags changed, and __sched_setscheduler() calls sched_dl_overflow():

kernel/sched/deadline.c:sched_dl_overflow() {
	...
	/* !deadline task may carry old deadline bandwidth */
	if (new_bw == p->dl.dl_bw && task_has_dl_policy(p))
		return 0;

	counted = task_has_dl_policy(p) && p->dl.dl_bw_cpu >= 0 &&
		  p->dl.dl_bw_gen == dl_bw_gen;
	...
}

Because new_bw == p->dl.dl_bw && task_has_dl_policy(p) is checked before
counted, sched_dl_overflow() returns 0 early without running admission
control, adding new_bw to dl_b->total_bw, or setting p->dl.dl_bw_cpu.

__setparam_dl() then adds the bandwidth to rq->dl.this_bw via
dl_change_utilization() (or on enqueue if queued) and clears SCHED_FLAG_SUGOV
from p->dl.flags while p->dl.dl_bw_cpu remains -1. If the task later exits or
leaves SCHED_DEADLINE, dl_task_release_bw() hits:

kernel/sched/deadline.c:dl_task_release_bw() {
	...
	cpu = dl_se->dl_bw_cpu;
	WARN_ON_ONCE(dl_se->dl_bw && cpu < 0);
	if (cpu < 0 || dl_se->dl_bw_gen != dl_bw_gen)
		return;
	...
}

The same early return in sched_dl_overflow() also skips restoring root domain
bandwidth when a task whose bandwidth was dropped by a root domain rebuild
(p->dl.dl_bw_gen != dl_bw_gen) changes its deadline or flags while keeping
new_bw == p->dl.dl_bw.

Should the early return in sched_dl_overflow() require counted to be true
before skipping the root domain update when new_bw == p->dl.dl_bw?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105193359.1450487-1-mathieu.desnoyers@efficios.com?part=20