Re: [PATCH v3 16/24] cgroup/cpuset: Wait for an attach before rebuilding the sched domains

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cgroup/cpuset: v1 file writes rebuild sched domains during attach
--

commit 316ee12eeff2f481c38bdeab56717a9467c1a2eb
Author: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

cgroup/cpuset: Wait for an attach before rebuilding the sched domains

This commit updates rebuild_sched_domains_cpuslocked() to call
wait_attach_done_lock() instead of mutex_lock(&cpuset_mutex), preventing
scheduler domain rebuilds from clearing root domain deadline bandwidth
reservations while a task attach is in progress.

> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index ae0bbe23a6332..e2b6f7a198eed 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1067,7 +1067,13 @@ void rebuild_sched_domains_locked(void)
>  
>  static void rebuild_sched_domains_cpuslocked(void)
>  {
> -	mutex_lock(&cpuset_mutex);
> +	/*
> +	 * An attach in progress has set aside, in the root domain its tasks
> +	 * move to, the deadline bandwidth it moves once it takes
> +	 * cpuset_mutex again: the rebuild of the accounting would drop that
> +	 * reservation, and the move would go through on its strength.
> +	 */
> +	wait_attach_done_lock();
>  	rebuild_sched_domains_locked();

[Severity: High]
Does this leave cgroup v1 file writes exposed to the same race when
rebuild_sched_domains_locked() is called directly without going through
rebuild_sched_domains_cpuslocked()?

The commit message notes that writes to a cpuset's files wait for an
in-progress attach via wait_attach_done_lock() since commit 75f77a25ec6bb
("cgroup/cpuset: Prevent race between task attach and cpuset state change").
However, that commit only added the wait to cpuset_write_resmask() and
cpuset_partition_write(), leaving cpuset_full_lock() calling
mutex_lock(&cpuset_mutex) directly:

kernel/cgroup/cpuset.c:cpuset_full_lock() {
	mutex_lock(&cpuset_top_mutex);
	cpus_read_lock();
	mutex_lock(&cpuset_mutex);
}

When CONFIG_CPUSETS_V1 is enabled, writes to cpuset.sched_load_balance and
cpuset.sched_relax_domain_level go through cgroup_file_write() (which does
not hold cgroup_mutex) into cpuset_write_u64() and cpuset_write_s64(). Both
acquire cpuset_mutex via cpuset_full_lock() and invoke
rebuild_sched_domains_locked() directly via cpuset_update_flag() and
update_relax_domain_level():

kernel/cgroup/cpuset-v1.c:cpuset_update_flag() {
	...
	if (!cpumask_empty(trialcs->cpus_allowed) && balance_flag_changed)
		rebuild_sched_domains_locked();
	...
}

kernel/cgroup/cpuset-v1.c:update_relax_domain_level() {
	...
	if (val != cs->relax_domain_level) {
		cs->relax_domain_level = val;
		if (!cpumask_empty(cs->cpus_allowed) &&
		    is_sched_load_balance(cs))
			rebuild_sched_domains_locked();
	}
	...
}

If either file is written between cpuset_can_attach() and cpuset_attach(),
rebuild_sched_domains_locked() -> partition_sched_domains() ->
dl_rebuild_rd_accounting() will still run during the attach and wipe out the
destination root domain's deadline bandwidth reservation, causing
dl_bw->total_bw to underflow and over-admit deadline tasks.

Should cpuset_full_lock() call wait_attach_done_lock() instead of
mutex_lock(&cpuset_mutex) so that cpuset_write_u64(), cpuset_write_s64(),
and hk_sd_workfn() also wait for an in-progress attach to complete?

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