Re: [PATCH v3 19/24] sched/deadline: Cancel the replenishment timer with the parameters

[sashiko-bot@kernel.org]

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

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

sched/deadline: Cancel the replenishment timer with the parameters

This commit cancels the deadline replenishment timer when clearing
a task's deadline parameters at its 0-lag time in task_non_contending()
and inactive_task_timer(), and when releasing bandwidth and clearing
period state for a sleeping task in switched_from_dl().

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

[ ... ]

> @@ -3920,8 +3922,10 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
>  		 * boosted owner would never leave its loop over a dl_period
>  		 * and a dl_runtime of 0.
>  		 */
> -		if (released)
> +		if (released) {
> +			cancel_replenish_timer(&p->dl);
>  			__dl_clear_state(&p->dl);
> +		}
>  	}

[Severity: Medium]
Can this cancel p->dl.dl_timer, clear the period state, and release
total_bw before the task's 0-lag time has actually passed if the task
switches back to and from dl_sched_class while asleep?

1. Suppose a SCHED_DEADLINE task p has an armed replenishment timer
(p->dl.dl_timer) and a future 0-lag time, and either blocks and then
switches to SCHED_OTHER, or switches to SCHED_OTHER while throttled and
then blocks. In switched_from_dl(), task_non_contending() has armed
p->dl.inactive_timer, and switched_from_dl() clears
p->dl.dl_non_contending and calls dl_task_depart(p):

kernel/sched/deadline.c:switched_from_dl() {
	...
	if (p->dl.dl_non_contending) {
		p->dl.dl_non_contending = 0;
		dl_task_depart(p);
	}
}

2. While p is still asleep (!task_on_rq_queued(p)) and before its 0-lag
time passes, p re-enters dl_sched_class (via sched_setattr() or a
priority inheritance boost in rt_mutex_setprio()). Because p is not
queued on the rq, enqueue_task_dl() is not called, and switched_to_dl()
cancels p->dl.inactive_timer without re-arming it:

kernel/sched/deadline.c:switched_to_dl() {
	cancel_inactive_timer(&p->dl);
	dl_task_undepart(p);
	inc_dl_tasks_cs(p);

	/* If p is not queued we will update its parameters at next wakeup. */
	if (!task_on_rq_queued(p)) {
		add_rq_bw(&p->dl, &rq->dl);

		return;
	}
	...
}

At this point, p is asleep in dl_sched_class before its 0-lag time with
p->dl.dl_non_contending == 0 and no inactive_timer armed.

3. If p then leaves dl_sched_class again while still asleep (via
sched_setscheduler() or PI deboost in rt_mutex_setprio()) before its
0-lag time, switched_from_dl() skips task_non_contending() because
task_on_rq_queued(p) is false, and enters the !task_on_rq_queued(p)
branch where p->dl.dl_non_contending is 0:

kernel/sched/deadline.c:switched_from_dl() {
	if (task_on_rq_queued(p) && p->dl.dl_runtime)
		task_non_contending(&p->dl, false);
	...
	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);
			released = true;
		}
		sub_rq_bw(&p->dl, &rq->dl);
		if (released) {
			cancel_replenish_timer(&p->dl);
			__dl_clear_state(&p->dl);
		}
	}
}

Wouldn't switched_from_dl() treat p->dl.dl_non_contending == 0 as meaning
the 0-lag time has already elapsed, and prematurely release total_bw via
dl_task_release_bw(p), cancel p->dl.dl_timer, and clear dl_throttled,
runtime, and deadline via __dl_clear_state(&p->dl) while the task is
still before its 0-lag time?

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