Re: [PATCH v4 14/26] sched/deadline: Rebuild the root domains' accounting from the task list

[sashiko-bot@kernel.org]

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

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

sched/deadline: Rebuild the root domains' accounting from the task list

This commit moves dl_rebuild_rd_accounting() into the deadline scheduler
to walk the task list under tasklist_lock instead of iterating over
cpusets, and adds dl_accounting_mutex to serialize bandwidth admission
against rebuilds when CONFIG_CPUSETS is disabled.

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 6b0ec20038573..45a896a4f4615 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c

[ ... ]

> @@ -3705,13 +3710,71 @@ void dl_clear_root_domain(struct root_domain *rd)
>  		dl_server_add_bw(rd, i);
>  }

[ ... ]

> +void dl_rebuild_rd_accounting(void)
> +{
> +	struct task_struct *g, *p;
> +
> +	lockdep_assert_cpus_held();
> +	lockdep_assert_held(&sched_domains_mutex);
> +	lockdep_assert_cpuset_lock_held();
> +	lockdep_assert_dl_accounting_held();
> +
> +	dl_clear_root_domains();
> +
> +	read_lock(&tasklist_lock);
> +	for_each_process_thread(g, p) {
> +		/*
> +		 * Tested without the task's lock first, to spare the others
> +		 * the locks: a task enters the class under the locks this
> +		 * rebuild holds, or by a boost, which brings it no bandwidth
> +		 * of its own to count.
> +		 */
> +		if (dl_task(p))
> +			dl_add_task_root_domain(p);
> +	}
> +	read_unlock(&tasklist_lock);
> +}

[Severity: Medium]
Can dl_add_task_root_domain() resurrect departed bandwidth that an earlier
rebuild already dropped?

Unlike css_task_iter, for_each_process_thread() visits un-reaped
EXIT_ZOMBIE tasks.  When a SCHED_DEADLINE task on CPU X exits before its
0-lag time and stays a zombie, it remains dl_task(p) with p->dl.dl_bw > 0
and p->dl.dl_departed_cpu == X on dl_departed_list until its
inactive_timer fires.

If CPU X goes offline, the first rebuild drops the zombie's bandwidth
because !cpu_active(X), leaving p->dl.dl_bw_gen behind at generation G.
If CPU X comes back online before inactive_timer expires, the second
rebuild (generation G + 2) skips the task in dl_clear_root_domains()
because dl_se->dl_bw_gen != dl_bw_gen - 1, but then visits it in the
tasklist walk and calls dl_add_task_root_domain():

kernel/sched/deadline.c:dl_add_task_root_domain() {
	...
	if (p->dl.dl_bw_gen == dl_bw_gen)
		goto unlock;
	if (p->dl.dl_departed_cpu >= 0) {
		cpu = p->dl.dl_departed_cpu;
		if (!cpu_active(cpu))
			goto unlock;
	}
	rq = cpu_rq(cpu);
	dl_b = &rq->rd->dl_bw;

	raw_spin_lock(&dl_b->lock);
	__dl_add(dl_b, p->dl.dl_bw, cpumask_weight(rq->rd->span));
	raw_spin_unlock(&dl_b->lock);
	p->dl.dl_bw_gen = dl_bw_gen;
	...
}

Because p->dl.dl_bw_gen == dl_bw_gen is false (G != G + 2) and cpu_active(X)
is now true, dl_add_task_root_domain() adds p->dl.dl_bw back into
dl_b->total_bw, over-accounting total_bw until inactive_timer expires and
causing new admissions to fail with -EBUSY or pushing total_bw past the
root domain capacity if tasks were admitted while CPU X was offline.
Checking p->dl.dl_bw_gen != dl_bw_gen - 1 when p->dl.dl_departed_cpu >= 0
(matching dl_clear_root_domains()) would avoid re-adding dropped
bandwidth.

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