Re: [PATCH v4 15/26] sched/deadline: Leave a dying task out of a rebuild of the accounting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] sched/deadline: premature bandwidth drop in
  dl_add_task_root_domain()
--

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

sched/deadline: Leave a dying task out of a rebuild of the accounting

This commit skips tasks in TASK_DEAD state that are not on the departed list
when rebuilding root domain deadline bandwidth accounting in
dl_add_task_root_domain().

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 45a896a4f4615..3f3de4b74a2dc 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -3643,6 +3643,15 @@ void dl_add_task_root_domain(struct task_struct *p)
>  		cpu = p->dl.dl_departed_cpu;
>  		if (!cpu_active(cpu))
>  			goto unlock;
> +	} else if (READ_ONCE(p->__state) == TASK_DEAD) {
> +		/*
> +		 * Dead and not departed: its bandwidth was released at its
> +		 * last schedule, under its runqueue lock and not this one,
> +		 * and dl_bw may still say otherwise. do_task_dead() sets
> +		 * TASK_DEAD under pi_lock before that, so a task found alive
> +		 * here dies after this walk, and releases what it counted.
> +		 */
> +		goto unlock;
>  	}

[Severity: Medium]
Can this drop the bandwidth of a dying task whose 0-lag time is still in the
future?

do_task_dead() calls set_special_state(TASK_DEAD), which sets p->__state =
TASK_DEAD under p->pi_lock and immediately releases p->pi_lock before
calling __schedule().  The task only sets p->dl.dl_departed_cpu later inside
__schedule() -> ... -> dequeue_task_dl() -> task_non_contending() ->
dl_task_depart():

CPU0                                  CPU1
do_task_dead()                        dl_rebuild_rd_accounting()
  set_special_state(TASK_DEAD)          dl_clear_root_domains()
    lock(&p->pi_lock)                     dl_bw_gen++
    p->__state = TASK_DEAD                rd->dl_bw.total_bw = 0
    unlock(&p->pi_lock)                 ...
                                        dl_add_task_root_domain(p)
                                          p->dl.dl_departed_cpu < 0
                                          p->__state == TASK_DEAD
                                          goto unlock  <-- skips p->dl.dl_bw
  __schedule(SM_NONE)
    ...
    task_non_contending(p)
      /* zerolag_time >= 0 */
      dl_task_depart(p, ...)
      start_inactive_timer(p)

Until inactive_timer fires at the 0-lag time, p's bandwidth is missing from
rd->dl_bw.total_bw, allowing sched_dl_overflow() to over-admit new deadline
tasks beyond the root domain's capacity.  In addition, because p joins
dl_departed_list with a stale p->dl.dl_bw_gen, a second rebuild before
inactive_timer fires will take the p->dl.dl_departed_cpu >= 0 branch in
dl_add_task_root_domain() and add p->dl.dl_bw back on top of any newly
admitted tasks.

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