lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fe8046d7-1f21-4b23-92f2-6be24ef9f58b@huaweicloud.com>
Date: Mon, 17 Nov 2025 15:45:47 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Sun Shaojie <sunshaojie@...inos.cn>, llong@...hat.com
Cc: mkoutny@...e.com, cgroups@...r.kernel.org, hannes@...xchg.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 shuah@...nel.org, tj@...nel.org
Subject: Re: [PATCH v4 1/1] cpuset: relax the overlap check for cgroup-v2



On 2025/11/17 9:57, Sun Shaojie wrote:
> In cgroup v2, a mutual overlap check is required when at least one of two
> cpusets is exclusive. However, this check should be relaxed and limited to
> cases where both cpusets are exclusive.
> 
> This patch ensures that for sibling cpusets A1 (exclusive) and B1
> (non-exclusive), change B1 cannot affect A1's exclusivity.
> 
> for example. Assume a machine has 4 CPUs (0-3).
> 
>    root cgroup
>       /    \
>     A1      B1
> 
> Case 1:
>  Table 1.1: Before applying the patch
>  Step                                       | A1's prstate | B1'sprstate |
>  #1> echo "0-1" > A1/cpuset.cpus            | member       | member      |
>  #2> echo "root" > A1/cpuset.cpus.partition | root         | member      |
>  #3> echo "0" > B1/cpuset.cpus              | root invalid | member      |
> 
> After step #3, A1 changes from "root" to "root invalid" because its CPUs
> (0-1) overlap with those requested by B1 (0-3). However, B1 can actually

B1 (0-3) --> B1(0) ?

> use CPUs 2-3(from B1's parent), so it would be more reasonable for A1 to
> remain as "root."
> 
>  Table 1.2: After applying the patch
>  Step                                       | A1's prstate | B1'sprstate |
>  #1> echo "0-1" > A1/cpuset.cpus            | member       | member      |
>  #2> echo "root" > A1/cpuset.cpus.partition | root         | member      |
>  #3> echo "0" > B1/cpuset.cpus              | root         | member      |
> 
> All other cases remain unaffected. For example, cgroup-v1, both A1 and B1
> are exclusive or non-exlusive.
> 
> Signed-off-by: Sun Shaojie <sunshaojie@...inos.cn>
> ---
>  kernel/cgroup/cpuset-internal.h               |  3 ++
>  kernel/cgroup/cpuset-v1.c                     | 20 +++++++++
>  kernel/cgroup/cpuset.c                        | 43 ++++++++++++++-----
>  .../selftests/cgroup/test_cpuset_prs.sh       |  5 ++-
>  4 files changed, 58 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> index 337608f408ce..c53111998432 100644
> --- a/kernel/cgroup/cpuset-internal.h
> +++ b/kernel/cgroup/cpuset-internal.h
> @@ -292,6 +292,7 @@ void cpuset1_hotplug_update_tasks(struct cpuset *cs,
>  			    struct cpumask *new_cpus, nodemask_t *new_mems,
>  			    bool cpus_updated, bool mems_updated);
>  int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial);
> +bool cpuset1_cpus_excl_conflict(struct cpuset *cs1, struct cpuset *cs2);
>  #else
>  static inline void fmeter_init(struct fmeter *fmp) {}
>  static inline void cpuset1_update_task_spread_flags(struct cpuset *cs,
> @@ -302,6 +303,8 @@ static inline void cpuset1_hotplug_update_tasks(struct cpuset *cs,
>  			    bool cpus_updated, bool mems_updated) {}
>  static inline int cpuset1_validate_change(struct cpuset *cur,
>  				struct cpuset *trial) { return 0; }
> +static inline bool cpuset1_cpus_excl_conflict(struct cpuset *cs1,
> +				struct cpuset *cs2) {return false; }
>  #endif /* CONFIG_CPUSETS_V1 */
>  
>  #endif /* __CPUSET_INTERNAL_H */
> diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
> index 12e76774c75b..5c1296bf6a34 100644
> --- a/kernel/cgroup/cpuset-v1.c
> +++ b/kernel/cgroup/cpuset-v1.c
> @@ -373,6 +373,26 @@ int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial)
>  	return ret;
>  }
>  
> +/*
> + * cpuset1_cpus_excl_conflict() - Check if two cpusets have exclusive CPU conflicts
> + *                                to legacy (v1)
> + * @cs1: first cpuset to check
> + * @cs2: second cpuset to check
> + *
> + * Returns: true if CPU exclusivity conflict exists, false otherwise
> + *
> + * If either cpuset is CPU exclusive, their allowed CPUs cannot intersect.
> + */
> +bool cpuset1_cpus_excl_conflict(struct cpuset *cs1, struct cpuset *cs2)
> +{
> +	if (is_cpu_exclusive(cs1) || is_cpu_exclusive(cs2))
> +		if (cpumask_intersects(cs1->cpus_allowed,
> +				       cs2->cpus_allowed))
> +			return true;
> +
> +	return false;
> +}
> +
>  #ifdef CONFIG_PROC_PID_CPUSET
>  /*
>   * proc_cpuset_show()
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 52468d2c178a..0fd803612513 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -580,35 +580,56 @@ static inline bool cpusets_are_exclusive(struct cpuset *cs1, struct cpuset *cs2)
>  
>  /**
>   * cpus_excl_conflict - Check if two cpusets have exclusive CPU conflicts
> - * @cs1: first cpuset to check
> - * @cs2: second cpuset to check
> + * @cs1: current cpuset to check
> + * @cs2: cpuset involved in the check
>   *
>   * Returns: true if CPU exclusivity conflict exists, false otherwise
>   *
>   * Conflict detection rules:
> - * 1. If either cpuset is CPU exclusive, they must be mutually exclusive
> + * For cgroup-v1:
> + *     see cpuset1_cpus_excl_conflict()
> + * For cgroup-v2:
> + * 1. If cs1 is exclusive, cs1 and cs2 must be mutually exclusive
>   * 2. exclusive_cpus masks cannot intersect between cpusets
> - * 3. The allowed CPUs of one cpuset cannot be a subset of another's exclusive CPUs
> + * 3. If cs2 is exclusive, cs2's allowed CPUs cannot be a subset of cs1's exclusive CPUs
> + * 4. if cs1 and cs2 are not exclusive, the allowed CPUs of one cpuset cannot be a subset
> + *    of another's exclusive CPUs
>   */

The revised conflict detection rules are confusing to me. I thought cs1 and cs2 should be treated
symmetrically, but that doesn’t seem to be the case here.

Shouldn’t the following rule apply regardless of whether cs1 or cs2 is exclusive: "The allowed CPUs
of one cpuset cannot be a subset of another's exclusive CPUs"?

>  static inline bool cpus_excl_conflict(struct cpuset *cs1, struct cpuset *cs2)
>  {
> -	/* If either cpuset is exclusive, check if they are mutually exclusive */
> -	if (is_cpu_exclusive(cs1) || is_cpu_exclusive(cs2))
> +	/* For cgroup-v1 */
> +	if (!cpuset_v2())
> +		return cpuset1_cpus_excl_conflict(cs1, cs2);
> +
> +	/* If cs1 are exclusive, check if they are mutually exclusive */
> +	if (is_cpu_exclusive(cs1))
>  		return !cpusets_are_exclusive(cs1, cs2);
>  
> +	/* The following check applies when either
> +	 * both cs1 and cs2 are non-exclusive,or
> +	 * only cs2 is exclusive.
> +	 */
> +
>  	/* Exclusive_cpus cannot intersect */
>  	if (cpumask_intersects(cs1->exclusive_cpus, cs2->exclusive_cpus))
>  		return true;
>  
> -	/* The cpus_allowed of one cpuset cannot be a subset of another cpuset's exclusive_cpus */
> -	if (!cpumask_empty(cs1->cpus_allowed) &&
> -	    cpumask_subset(cs1->cpus_allowed, cs2->exclusive_cpus))
> -		return true;
> -
> +	/* cs2's allowed CPUs cannot be a subset of cs1's exclusive CPUs */
>  	if (!cpumask_empty(cs2->cpus_allowed) &&
>  	    cpumask_subset(cs2->cpus_allowed, cs1->exclusive_cpus))
>  		return true;
>  
> +	/* If cs2 is exclusive, check finished here */
> +	if (is_cpu_exclusive(cs2))
> +		return false;
> +
> +	/* The following check applies only if both cs1 and cs2 are non-exclusive. */
> +
> +	/* cs1's allowed CPUs cannot be a subset of cs1's exclusive CPUs */
> +	if (!cpumask_empty(cs1->cpus_allowed) &&
> +	    cpumask_subset(cs1->cpus_allowed, cs2->exclusive_cpus))
> +		return true;
> +
>  	return false;
>  }
>  

>From your commit message, it appears you intend to modify "if (is_cpu_exclusive(cs1) ||
is_cpu_exclusive(cs2))" to "if (is_cpu_exclusive(cs1) && is_cpu_exclusive(cs2))".

However, I’m having trouble following the change.

> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index a17256d9f88a..b848bc0729cf 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -388,10 +388,11 @@ TEST_MATRIX=(
>  	"  C0-1:S+  C1      .    C2-3     .      P2     .      .     0 A1:0-1|A2:1 A1:P0|A2:P-2"
>  	"  C0-1:S+ C1:P2    .    C2-3     P1     .      .      .     0 A1:0|A2:1 A1:P1|A2:P2 0-1|1"
>  
> -	# A non-exclusive cpuset.cpus change will invalidate partition and its siblings
> +	# A non-exclusive cpuset.cpus change will not invalidate its siblings partition.
> +	# But a exclusive cpuset.cpus change will invalidate itself.
>  	"  C0-1:P1   .      .    C2-3   C0-2     .      .      .     0 A1:0-2|B1:2-3 A1:P-1|B1:P0"
>  	"  C0-1:P1   .      .  P1:C2-3  C0-2     .      .      .     0 A1:0-2|B1:2-3 A1:P-1|B1:P-1"
> -	"   C0-1     .      .  P1:C2-3  C0-2     .      .      .     0 A1:0-2|B1:2-3 A1:P0|B1:P-1"
> +	"   C0-1     .      .  P1:C2-3  C0-2     .      .      .     0 A1:0-1|B1:2-3 A1:P0|B1:P1"
>  
>  	# cpuset.cpus can overlap with sibling cpuset.cpus.exclusive but not subsumed by it
>  	"   C0-3     .      .    C4-5     X5     .      .      .     0 A1:0-3|B1:4-5"

-- 
Best regards,
Ridong


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ