Re: [PATCH v3 11/24] sched/deadline: Do not take a waking task's bandwidth off a runqueue it left

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sched/deadline: premature bw release and state reset in switched_from_dl()
--

commit 32169fc2d3492f6357debcf70a3512a72875ebc4
Author: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

sched/deadline: Do not take a waking task's bandwidth off a runqueue it left

This commit adds a dl_bw_oncpu flag to struct sched_dl_entity to track
whether a task's bandwidth is currently accounted in a runqueue's this_bw.
This prevents double-charging or underflowing this_bw when a migrating task
is modified or changes scheduling class while sitting on a target CPU's
wake_list before being enqueued.

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 37b931fc2faf2..d4cd5b5fb27b7 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c

[ ... ]

> @@ -2789,6 +2810,11 @@ static void migrate_task_rq_dl(struct task_struct *p, int new_cpu __maybe_unused
>  		 */
>  		cancel_inactive_timer(&p->dl);
>  	}
> +	/*
> +	 * Until the enqueue on the CPU it wakes on charges it there, this
> +	 * bandwidth is on no runqueue: the rq of task_cpu() is not the one
> +	 * it was taken from, which dl_bw_oncpu now says.
> +	 */
>  	sub_rq_bw(&p->dl, &rq->dl);
>  	rq_unlock(rq, &rf);
>  }

[Severity: Medium]
When a task blocks before its 0-lag time and then wakes up on another CPU,
can switching it out of SCHED_DEADLINE while it is still on the target
CPU's wake_list prematurely release its root-domain bandwidth and reset its
CBS period state?

When the task blocks, schedule() -> dequeue_task_dl() ->
dequeue_dl_entity() calls task_non_contending(&p->dl, true), which sets
p->dl.dl_non_contending = 1 and arms p->dl.inactive_timer for the 0-lag
time.

If the task wakes up on another CPU before that 0-lag time passes,
try_to_wake_up() -> set_task_cpu() -> migrate_task_rq_dl() subtracts
running_bw from the old runqueue, clears p->dl.dl_non_contending = 0,
cancels p->dl.inactive_timer, and clears p->dl.dl_bw_oncpu = 0 via
sub_rq_bw():

kernel/sched/deadline.c:migrate_task_rq_dl() {
	...
	if (p->dl.dl_non_contending) {
		update_rq_clock(rq);
		sub_running_bw(&p->dl, &rq->dl);
		p->dl.dl_non_contending = 0;
		cancel_inactive_timer(&p->dl);
	}
	sub_rq_bw(&p->dl, &rq->dl);
}

If try_to_wake_up() queues the task on the target CPU's wake_list via
ttwu_queue_wakelist(), p->on_rq remains 0 (!task_on_rq_queued(p)) and
p->dl.dl_bw_oncpu is 0 until sched_ttwu_pending() runs on the target CPU.

If sched_setscheduler() or sched_setattr() switches the task out of
SCHED_DEADLINE during that window (such as the switch out and back in
described in the commit message), switched_from_dl() sees
!task_on_rq_queued(p) and p->dl.dl_non_contending == 0:

kernel/sched/deadline.c:switched_from_dl() {
	if (task_on_rq_queued(p) && p->dl.dl_runtime)
		task_non_contending(&p->dl, false);

	dec_dl_tasks_cs(p);

	if (!task_on_rq_queued(p)) {
		bool released = false;

		if (p->dl.dl_non_contending) {
			sub_running_bw(&p->dl, &rq->dl);
		} else if (p->dl.dl_runtime && !dl_entity_is_special(&p->dl)) {
			dl_task_release_bw(p, task_cpu(p));
			released = true;
		}
		sub_rq_bw(&p->dl, &rq->dl);
		if (released)
			__dl_clear_state(&p->dl);
	}
}

Because migrate_task_rq_dl() already cleared p->dl.dl_non_contending and
canceled the inactive timer, switched_from_dl() treats the task as if its
0-lag time has already passed. It immediately releases total_bw via
dl_task_release_bw() and resets runtime, deadline, dl_throttled, and
dl_yielded via __dl_clear_state(&p->dl), allowing another task to be
admitted or the same task to switch back into SCHED_DEADLINE with a fresh
full budget before its 0-lag time.

Should switched_from_dl() also re-evaluate the 0-lag time via
task_non_contending(&p->dl, false) when !p->dl.dl_bw_oncpu?

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