Re: [PATCH v2 18/21] sched/deadline: Credit a rebuild to a CPU that can hold the bandwidth

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] sched/deadline: offline hk_msk CPUs break dl_get_task_effective_cpus()
- [High] sched/deadline: stale HK_TYPE_DOMAIN mask in dl_get_task_effective_cpus()
- [High] sched/deadline: dl_add_task_root_domain() ignores dl_bw_cpu on empty msk
--

commit 119330e07c6138cad461765e2b7413e00f7f5e31
Author: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

sched/deadline: Credit a rebuild to a CPU that can hold the bandwidth

This commit filters out isolated CPUs from a deadline task's effective CPU
mask when housekeeping CPUs are present, and skips tasks that have no active
CPU during a root domain bandwidth accounting rebuild.

> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 029ec2ba5a663..c0af06d0f3688 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -3617,6 +3617,16 @@ static void dl_get_task_effective_cpus(struct task_struct *p, struct cpumask *cp
>  	 * hierarchy up to the partition root.
>  	 */
>  	cpuset_cpus_allowed_locked(p, cpus);
> +	/*
> +	 * Which the CPUs isolated by isolcpu="domain" are not part of: a
> +	 * cpuset can include them beside housekeeping CPUs, and without
> +	 * cpusets the mask is every CPU the task can run on. The first
> +	 * active CPU of it would then be an isolated one, of the default
> +	 * root domain, rather than one of the root domain the task's
> +	 * bandwidth belongs to.
> +	 */
> +	if (housekeeping_enabled(HK_TYPE_DOMAIN))
> +		cpumask_and(cpus, cpus, hk_msk);
>  }

[Severity: High]
Can offline or unpopulated housekeeping CPUs in hk_msk cause
dl_get_task_effective_cpus() to drop or misattribute the bandwidth of a
deadline task running in def_root_domain?

In housekeeping_setup() and housekeeping_update(),
housekeeping_cpumask(HK_TYPE_DOMAIN) is built from cpu_possible_mask minus
the isolated CPUs, so hk_msk continues to include housekeeping CPUs when
they are offline or unpopulated:

kernel/sched/deadline.c:dl_get_task_effective_cpus() {
	hk_msk = housekeeping_cpumask(HK_TYPE_DOMAIN);
	if (housekeeping_enabled(HK_TYPE_DOMAIN)) {
		if (!cpumask_intersects(p->cpus_ptr, hk_msk)) {
			cpumask_andnot(cpus, cpu_active_mask, hk_msk);
			return;
		}
	}
	cpuset_cpus_allowed_locked(p, cpus);
	if (housekeeping_enabled(HK_TYPE_DOMAIN))
		cpumask_and(cpus, cpus, hk_msk);
}

For a deadline task running on an active isolated CPU in def_root_domain,
p->cpus_ptr can still contain offline or unpopulated housekeeping CPUs
(for example, __sched_setscheduler() requires cpumask_subset(rq->rd->span,
p->cpus_ptr) where def_root_domain.span contains offline possible CPUs,
and top_cpuset keeps offline non-subpartition CPUs in p->cpus_ptr).

Because p->cpus_ptr and hk_msk both contain those offline housekeeping CPUs,
cpumask_intersects(p->cpus_ptr, hk_msk) evaluates to true even when p has no
active housekeeping CPU in p->cpus_ptr. Execution then falls through and
cpumask_and(cpus, cpus, hk_msk) strips all active isolated CPUs from cpus:

- If cpus contains no active housekeeping CPUs (for example, all
  housekeeping CPUs in top_cpuset are offline, or p is in a child cpuset cs
  where guarantee_active_cpus() stops at cs because cs->effective_cpus
  intersects cpu_active_mask on the active isolated CPU),
  cpumask_first_and(cpu_active_mask, msk) in dl_add_task_root_domain()
  returns >= nr_cpu_ids and drops the active task's bandwidth from
  def_root_domain.

- If another housekeeping CPU outside p->cpus_ptr is active in top_cpuset
  (or with !CONFIG_CPUSETS), stripping the isolated CPUs causes
  dl_add_task_root_domain() to credit p's bandwidth to that housekeeping
  root domain instead of def_root_domain. Similarly, a task admitted on an
  isolated CPU in def_root_domain with an unnarrowed affinity mask has its
  bandwidth moved to a housekeeping root domain on rebuild while continuing
  to run in def_root_domain.

- Conversely, if a housekeeping deadline task is preempted on an isolated
  CPU while in a migrate_disable() section, migrate_disable_switch() narrows
  p->cpus_ptr to cpumask_of(rq->cpu), so !cpumask_intersects(p->cpus_ptr,
  hk_msk) evaluates to true and credits its bandwidth to def_root_domain
  instead of its housekeeping root domain.

[Severity: High]
Also in dl_get_task_effective_cpus(), does reading HK_TYPE_DOMAIN observe a
stale mask during cgroup v2 isolated partition transitions?

When a cgroup v2 PRS_ISOLATED partition transitions to PRS_ROOT or
PRS_MEMBER, or is invalidated during CPU hotplug,
cpuset_update_sd_hk_unlock() and cpuset_handle_hotplug() invoke
rebuild_sched_domains_locked() -> partition_sched_domains_locked() ->
dl_rebuild_rd_accounting() -> dl_add_task_root_domain() ->
dl_get_task_effective_cpus() before housekeeping_update() updates
HK_TYPE_DOMAIN:

kernel/cgroup/cpuset.c:cpuset_update_sd_hk_unlock() {
	if (force_sd_rebuild)
		rebuild_sched_domains_locked();

	if (update_housekeeping) {
		update_housekeeping = false;
		cpumask_copy(isolated_hk_cpus, isolated_cpus);

		mutex_unlock(&cpuset_mutex);
		cpus_read_unlock();
		WARN_ON_ONCE(housekeeping_update(isolated_hk_cpus));
	}
}

While generate_sched_domains() uses housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT),
dl_get_task_effective_cpus() queries HK_TYPE_DOMAIN, which still excludes the
CPUs transitioning out of PRS_ISOLATED while dl_rebuild_rd_accounting() runs.

If a deadline task in that partition has p->cpus_ptr intersecting the stale
hk_msk (for example, on an offline CPU), cpumask_and(cpus, cpus, hk_msk)
clears the newly un-isolated active CPUs from cpus and drops the task's
bandwidth from the rebuild. If p->cpus_ptr does not intersect the stale
hk_msk, dl_get_task_effective_cpus() returns cpu_active_mask & ~hk_msk,
which can pick a lower-numbered isolated CPU in def_root_domain instead of
the task's new root domain.

>  
>  /* The caller should hold cpuset_mutex */
> @@ -3637,7 +3647,18 @@ void dl_add_task_root_domain(struct task_struct *p)
>  	msk = this_cpu_cpumask_var_ptr(local_cpu_mask_dl);
>  	dl_get_task_effective_cpus(p, msk);
>  	cpu = cpumask_first_and(cpu_active_mask, msk);
> -	BUG_ON(cpu >= nr_cpu_ids);
> +	if (cpu >= nr_cpu_ids) {
> +		/*
> +		 * No active CPU to count this task's bandwidth: the ones it
> +		 * may run on all went offline, which the CPUs isolated by
> +		 * isolcpu="domain" allow, no deadline task of theirs keeping
> +		 * the last of them online. Leave it out of this rebuild,
> +		 * where dl_bw_gen keeps its release out as well, until one
> +		 * has a CPU for it.
> +		 */
> +		raw_spin_unlock_irqrestore(&p->pi_lock, rf.flags);
> +		return;
> +	}

[Severity: High]
Does returning early when cpu >= nr_cpu_ids skip tasks whose bandwidth was
already moved to an active fallback CPU by dl_task_offline_migration(), or
recorded on an active CPU in p->dl.dl_bw_cpu when departing SCHED_DEADLINE?

When a deadline task confined to isolated CPUs is throttled while all of its
isolated CPUs go offline, dl_task_timer() -> dl_task_offline_migration()
picks an active fallback CPU via cpumask_any(cpu_active_mask), migrates p to
later_rq->cpu, and records p->dl.dl_bw_cpu = later_rq->cpu without modifying
p->cpus_ptr:

kernel/sched/deadline.c:dl_task_offline_migration() {
	scoped_guard (raw_spinlock, &dl_departed_lock) {
		if (p->dl.dl_bw_gen == dl_bw_gen) {
			...
		}
		p->dl.dl_bw_cpu = later_rq->cpu;
	}

	set_task_cpu(p, later_rq->cpu);
}

Because p->cpus_ptr is still restricted to the offline isolated CPUs,
dl_get_task_effective_cpus(p, msk) returns an empty mask on rebuild. In
dl_add_task_root_domain(), cpu = cpumask_first_and(cpu_active_mask, msk) is
then >= nr_cpu_ids, and this early return exits before consulting
p->dl.dl_bw_cpu or crediting later_rq->rd->dl_bw.

Similarly, because the cpu >= nr_cpu_ids return happens before acquiring
dl_departed_lock and checking !list_empty(&p->dl.dl_departed_node), a task
that departed SCHED_DEADLINE with an active CPU recorded in p->dl.dl_bw_cpu
and then had its affinity restricted to offline isolated CPUs (before dying
during the rebuild walk or being PI-boosted back into dl_sched_class prior to
its 0-lag time) will also return early and have its bandwidth omitted from
the recorded active CPU's root domain.

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