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 <[email protected]>
> ---
> 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
