[PATCH] cgroup/cpuset: Drop redundant no-cpu check in partcmd_update with newmask

[PATCH] cgroup/cpuset: Drop redundant no-cpu check in partcmd_update with newmask

[Shaojie Sun]

update_parent_effective_cpumask() with a non-NULL newmask is only ever
reached from partition_cpus_change(), and only after validate_partition()
has checked the very same condition with the very same arguments:

  tasks_nocpu_error(parent_cs(cs), cs, trialcs->effective_xcpus)

since newmask is exactly trialcs->effective_xcpus and both functions use
parent_cs(cs) as the parent. cs, parent and cpu_active_mask cannot change
in between either, as cpuset_mutex and cpu_read_lock are held for the
whole operation. Hence

  nocpu = tasks_nocpu_error(parent, cs, newmask);

is always false here and the block that turns a "nocpu" condition into
PERR_NOCPUS is dead code. A new CPU list that would drain the parent is
fully handled by validate_partition(), which routes the write to
partcmd_invalidate and hands the partition's CPUs back to the parent.

The "!adding || !cpumask_intersects(addmask, cpu_active_mask)" test that
guards the block does not change this: the block can only be entered when
nocpu is true, and that never happens here.

Drop the redundant check and the dead PERR_NOCPUS block, and leave a
comment pointing at where the "parent could be drained" case is handled.
The empty-newmask check in the same branch is left alone: it validates
the newmask argument of this function, while what is removed here is a
decision that belongs to validate_partition().

No functional change.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 kernel/cgroup/cpuset.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 3219f927c35e..c7b3a734cda8 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2016,9 +2016,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 			goto write_error;
 		}
 
-		/* Check newmask whether cpus are available for parent/cs */
-		nocpu = tasks_nocpu_error(parent, cs, newmask);
-
 		/*
 		 * partcmd_update with newmask:
 		 *
@@ -2033,6 +2030,11 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		 * For invalid partition:
 		 *   delmask = newmask & parent->effective_xcpus
 		 *   The partition may become valid soon.
+		 *
+		 * Note that the no-cpu check is not repeated here:
+		 * validate_partition() has already evaluated
+		 * tasks_nocpu_error() with this very mask, and a failing
+		 * write is routed to partcmd_invalidate instead.
 		 */
 		if (is_partition_invalid(cs)) {
 			adding = false;
@@ -2056,18 +2058,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 			cpumask_and(tmp->new_cpus, tmp->delmask, cpu_active_mask);
 			WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
 		}
-
-		/*
-		 * Make partition invalid if parent's effective_cpus could
-		 * become empty and there are tasks in the parent.
-		 */
-		if (nocpu && (!adding ||
-		    !cpumask_intersects(tmp->addmask, cpu_active_mask))) {
-			part_error = PERR_NOCPUS;
-			deleting = false;
-			adding = cpumask_and(tmp->addmask,
-					     cs->effective_xcpus, parent->effective_xcpus);
-		}
 	} else {
 		/*
 		 * partcmd_update w/o newmask
-- 
2.50.1

Re: [PATCH] cgroup/cpuset: Drop redundant no-cpu check in partcmd_update with newmask

[Waiman Long]

On 10/30/26 9:14 AM, Shaojie Sun wrote:
> update_parent_effective_cpumask() with a non-NULL newmask is only ever
> reached from partition_cpus_change(), and only after validate_partition()
> has checked the very same condition with the very same arguments:
>
>    tasks_nocpu_error(parent_cs(cs), cs, trialcs->effective_xcpus)
>
> since newmask is exactly trialcs->effective_xcpus and both functions use
> parent_cs(cs) as the parent. cs, parent and cpu_active_mask cannot change
> in between either, as cpuset_mutex and cpu_read_lock are held for the
> whole operation. Hence
>
>    nocpu = tasks_nocpu_error(parent, cs, newmask);
>
> is always false here and the block that turns a "nocpu" condition into
> PERR_NOCPUS is dead code. A new CPU list that would drain the parent is
> fully handled by validate_partition(), which routes the write to
> partcmd_invalidate and hands the partition's CPUs back to the parent.
>
> The "!adding || !cpumask_intersects(addmask, cpu_active_mask)" test that
> guards the block does not change this: the block can only be entered when
> nocpu is true, and that never happens here.
>
> Drop the redundant check and the dead PERR_NOCPUS block, and leave a
> comment pointing at where the "parent could be drained" case is handled.
> The empty-newmask check in the same branch is left alone: it validates
> the newmask argument of this function, while what is removed here is a
> decision that belongs to validate_partition().
>
> No functional change.
>
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 20 +++++---------------
>   1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 3219f927c35e..c7b3a734cda8 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2016,9 +2016,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   			goto write_error;
>   		}
>   
> -		/* Check newmask whether cpus are available for parent/cs */
> -		nocpu = tasks_nocpu_error(parent, cs, newmask);
> -
>   		/*
>   		 * partcmd_update with newmask:
>   		 *
> @@ -2033,6 +2030,11 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   		 * For invalid partition:
>   		 *   delmask = newmask & parent->effective_xcpus
>   		 *   The partition may become valid soon.
> +		 *
> +		 * Note that the no-cpu check is not repeated here:
> +		 * validate_partition() has already evaluated
> +		 * tasks_nocpu_error() with this very mask, and a failing
> +		 * write is routed to partcmd_invalidate instead.
>   		 */
>   		if (is_partition_invalid(cs)) {
>   			adding = false;
> @@ -2056,18 +2058,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   			cpumask_and(tmp->new_cpus, tmp->delmask, cpu_active_mask);
>   			WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
>   		}
> -
> -		/*
> -		 * Make partition invalid if parent's effective_cpus could
> -		 * become empty and there are tasks in the parent.
> -		 */
> -		if (nocpu && (!adding ||
> -		    !cpumask_intersects(tmp->addmask, cpu_active_mask))) {
> -			part_error = PERR_NOCPUS;
> -			deleting = false;
> -			adding = cpumask_and(tmp->addmask,
> -					     cs->effective_xcpus, parent->effective_xcpus);
> -		}
>   	} else {
>   		/*
>   		 * partcmd_update w/o newmask

LKTM

Reviewed-by: Waiman Long <longman@redhat.com>