Re: [PATCH v5 22/29] sched/deadline: Make a rebuild use the CPU recorded with a task's bandwidth

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] sched/deadline: wrong root domain in dl_task_rebuild_cpu() on
  empty mask
--

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

sched/deadline: Make a rebuild use the CPU recorded with a task's bandwidth

This commit updates deadline root domain accounting rebuilds to select a
task's root domain from its current CPU, recorded bandwidth CPU, or active
cpumask, and skips tasks with no active CPU instead of hitting BUG_ON().

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 8d9e55bca9439..bc31045e4686f 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
[ ... ]
> @@ -3720,6 +3726,55 @@ static void dl_get_task_effective_cpus(struct task_struct *p, struct cpumask *cp
[ ... ]
> +static unsigned int dl_task_rebuild_cpu(struct task_struct *p)
> +{
> +	struct cpumask *msk = this_cpu_cpumask_var_ptr(local_cpu_mask_dl);
> +	int recorded = p->dl.dl_bw_cpu;
> +	unsigned int cpu;
> +
> +	/*
> +	 * Look at the mask of the task first. A change of the cpusets sets
> +	 * the mask of each task before its rebuild. The housekeeping mask
> +	 * that dl_get_task_effective_cpus() reads is updated after it. If no
> +	 * CPU of the mask is active, take the CPU recorded if it is active:
> +	 * dl_task_offline_migration() moved the task there. Else ask the
> +	 * cpuset.
> +	 */
> +	cpumask_and(msk, &p->cpus_mask, cpu_active_mask);
> +	cpu = task_cpu(p);
> +	if (cpumask_test_cpu(cpu, msk))
> +		return cpu;
> +	if (cpumask_empty(msk)) {
> +		if (recorded >= 0 && cpu_active(recorded))
> +			return recorded;
> +		dl_get_task_effective_cpus(p, msk);
> +		cpumask_and(msk, msk, cpu_active_mask);
> +	}
> +	if (recorded >= 0 && cpumask_test_cpu(recorded, msk))
> +		return recorded;

[Severity: Medium]
Can returning recorded whenever cpu_active(recorded) is true on an empty
msk charge a sleeping task's bandwidth to a stale root domain instead of
consulting dl_get_task_effective_cpus()?

Suppose a task in top_cpuset with user_cpus_ptr = {1} and cpus_mask = {1}
is asleep when CPU 1 goes offline.  Top_cpuset tasks do not have cpus_mask
reset on hotplug, so the hotplug rebuild sees cpumask_empty(msk), queries
dl_get_task_effective_cpus(), and records active CPU 0 in p->dl.dl_bw_cpu.
If a new cpuset partition root is then created on CPU 0 while the task
stays asleep, cpuset_update_tasks_cpumask() intersects the remaining
possible CPUs with user_cpus_ptr ({1}), leaving cpus_mask as {1}.  The
partition rebuild then sees cpumask_empty(msk) and cpu_active(0), and
returns CPU 0 without calling dl_get_task_effective_cpus(), charging the
task's bandwidth to the new partition on CPU 0 rather than top_cpuset.

Also, because the cpumask_empty(msk) block returns early whenever recorded
is active, if execution reaches dl_get_task_effective_cpus() and
cpumask_and(msk, msk, cpu_active_mask), recorded is guaranteed to be
inactive, so the subsequent cpumask_test_cpu(recorded, msk) check can
never succeed after that fallback.

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