| 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
| ||
|
Message-ID: <f32d2f31-630f-450b-911f-b512bbeb380a@huaweicloud.com>
Date: Mon, 17 Nov 2025 19:37:31 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Sun Shaojie <sunshaojie@...inos.cn>
Cc: cgroups@...r.kernel.org, hannes@...xchg.org,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
llong@...hat.com, mkoutny@...e.com, shuah@...nel.org, tj@...nel.org
Subject: Re: [PATCH v4 1/1] cpuset: relax the overlap check for cgroup-v2
On 2025/11/17 18:00, Sun Shaojie wrote:
> On 2025/11/17 15:45, Chen Ridong Wrote:
>> 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) ?
>
> Sorry, that was a typo. It should indeed be 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"?
>>
>
>
> Certainly, this rule applies regardless of whether cs1 or cs2 is exclusive,
> and the current implementation already handles it this way.
> The following two cases cover this rule.
> "1. If cs1 is exclusive, cs1 and cs2 must be mutually exclusive"
> "3. If cs2 is exclusive, cs2's allowed CPUs cannot be a subset of cs1's exclusive CPUs"
>
I believe this function should return the same result regardless of whether it is called as
cpus_excl_conflict(A1, B1) or cpus_excl_conflict(B1, A1), which means cs1 and cs2 should be treated
symmetrically. However, since cs1 and cs2 are handled differently, it is difficult to convince me
that this implementation is correct.
>
>>> 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.
>>
>
> The current modification specifically addresses the scenario where one cpuset
> A1 is exclusive and its sibling cpuset B1 is non-exclusive.
> The goal is to ensure that when the non-exclusive cpuset B1 modifies its own
> "cpuset.cpus" or "cpuset.cpus.exclusive", it does not cause A1 to change from
> exclusive to non-exclusive.
>
> The following three scenarios are not affected by this patch:
> 1.both A1 and B1 are exclusive.
> 2.both A1 and B1 are non-exclusive.
> 3.A1 is exclusive, B1 is non-exclusive, change "cpuset.cpus" or "cpuset.cpus.exclusive" of A1.
>
>
>>> 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