Re: [PATCH-next v5 3/6] cgroup/cpuset: Expand the scope of cpuset_can_attach_check()

[Ridong Chen]

On 2026/6/2 10:32, Waiman Long wrote:
> Expand the scope of cpuset_can_attach_check() by including the setting
> of setsched flag inside cpuset_can_attach_check() with the new @oldcs
> and @psetsched argument. As cpuset_can_attach_check() is also called
> from cpuset_can_fork(), set the new arguments to NULL from that caller.
> 

Hi Waiman,

The code change itself looks good to me. However, the commit message
has two paragraphs that don't match this patch:

> While at it, expose the source and destination cpuset cpu/memory check
> results in the new attach_cpus_updated and attach_mems_updated static
> flags so that these flags can be used directly from cpuset_attach()
> without the need to do the same computations again.
> 
> Two new global attach related flags are added (attach_cpus_updated &
> attach_mems_updated) which are set to indicate that CPUs or memory nodes
> are updated. These 2 flags are set in cpuset_can_attach() and are used
> in cpuset_attach() for optimization. Since cpuset_mutex will be released
> between the 2 calls, it is possible that an intervening cpuset action
> may change the CPU or node mask of the relevant cpusets, so check is
> added to set these flags if the effective_cpus or effective_mems of
> those cpusets is changed.
> 
> Signed-off-by: Waiman Long <longman@xxxxxxxxxx>

Other than that:

Reviewed-by: Ridong Chen <ridong.chen@xxxxxxxxx>

> ---
>  kernel/cgroup/cpuset.c | 52 ++++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 5c1f3ee48d5d..5c777b1237a8 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2982,12 +2982,39 @@ static struct cpuset *cpuset_attach_old_cs;
>   * For v1, cpus_allowed and mems_allowed can't be empty.
>   * For v2, effective_cpus can't be empty.
>   * Note that in v1, effective_cpus = cpus_allowed.
> + *
> + * Also set the boolean flag passed in by @psetsched depending on if
> + * security_task_setscheduler() call is needed and @oldcs is not NULL.
>   */
> -static int cpuset_can_attach_check(struct cpuset *cs)
> +static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
> +				   bool *psetsched)
>  {
>  	if (cpumask_empty(cs->effective_cpus) ||
>  	   (!is_in_v2_mode() && nodes_empty(cs->mems_allowed)))
>  		return -ENOSPC;
> +
> +	if (!oldcs)
> +		return 0;
> +
> +	/*
> +	 * Skip rights over task setsched check in v2 when nothing changes,
> +	 * migration permission derives from hierarchy ownership in
> +	 * cgroup_procs_write_permission()).
> +	 */
> +	*psetsched = !cpuset_v2() ||
> +		!cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
> +		!nodes_equal(cs->effective_mems, oldcs->effective_mems);
> +
> +	/*
> +	 * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
> +	 * brings the last online CPU offline as users are not allowed to empty
> +	 * cpuset.cpus when there are active tasks inside. When that happens,
> +	 * we should allow tasks to migrate out without security check to make
> +	 * sure they will be able to run after migration.
> +	 */
> +	if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
> +		*psetsched = false;
> +
>  	return 0;
>  }
>  
> @@ -3034,29 +3061,10 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
>  	mutex_lock(&cpuset_mutex);
>  
>  	/* Check to see if task is allowed in the cpuset */
> -	ret = cpuset_can_attach_check(cs);
> +	ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
>  	if (ret)
>  		goto out_unlock;
>  
> -	/*
> -	 * Skip rights over task setsched check in v2 when nothing changes,
> -	 * migration permission derives from hierarchy ownership in
> -	 * cgroup_procs_write_permission()).
> -	 */
> -	setsched_check = !cpuset_v2() ||
> -		!cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
> -		!nodes_equal(cs->effective_mems, oldcs->effective_mems);
> -
> -	/*
> -	 * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
> -	 * brings the last online CPU offline as users are not allowed to empty
> -	 * cpuset.cpus when there are active tasks inside. When that happens,
> -	 * we should allow tasks to migrate out without security check to make
> -	 * sure they will be able to run after migration.
> -	 */
> -	if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
> -		setsched_check = false;
> -
>  	cgroup_taskset_for_each(task, css, tset) {
>  		ret = task_can_attach(task);
>  		if (ret)
> @@ -3601,7 +3609,7 @@ static int cpuset_can_fork(struct task_struct *task, struct css_set *cset)
>  	mutex_lock(&cpuset_mutex);
>  
>  	/* Check to see if task is allowed in the cpuset */
> -	ret = cpuset_can_attach_check(cs);
> +	ret = cpuset_can_attach_check(cs, NULL, NULL);
>  	if (ret)
>  		goto out_unlock;
>  

-- 
Best regards,
Ridong