| 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: <fd98e16d-0602-4ecd-9f8b-9ee494ddaa1d@huaweicloud.com>
Date: Thu, 13 Nov 2025 15:03:16 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Waiman Long <llong@...hat.com>, tj@...nel.org, hannes@...xchg.org,
mkoutny@...e.com
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH RFC v2 12/22] cpuset: introduce
local_partition_invalidate()
On 2025/11/13 6:54, Waiman Long wrote:
> On 10/25/25 2:48 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@...wei.com>
>>
>> Build on the partition_disable() infrastructure introduced in the previous
>> patch to handle local partition invalidation.
>>
>> The local_partition_invalidate() function factors out the local partition
>> invalidation logic from update_parent_effective_cpumask(), which delegates
>> to partition_disable() to complete the invalidation process.
>>
>> Additionally, correct the transition logic in cpuset_hotplug_update_tasks()
>> when determining whether to transition an invalid partition root, the check
>> should be based on non-empty user_cpus rather than non-empty
>> effective_xcpus. This correction addresses the scenario where
>> exclusive_cpus is not set but cpus_allowed is configured - in this case,
>> effective_xcpus may be empty even though the partition should be considered
>> for re-enablement. The user_cpus-based check ensures proper partition state
>> transitions under these conditions.
>>
>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>> ---
>> kernel/cgroup/cpuset.c | 66 +++++++++++++++++++++++++++---------------
>> 1 file changed, 43 insertions(+), 23 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index f36d17a4d8cd..73a43ab58f72 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1914,6 +1914,40 @@ static void local_partition_disable(struct cpuset *cs, enum prs_errcode
>> part_err
>> }
>> }
>> +/**
>> + * local_partition_invalidate - Invalidate a local partition
>> + * @cs: Target cpuset (local partition root) to invalidate
>> + * @tmp: Temporary masks
>> + */
>> +static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
>> +{
>> + struct cpumask *xcpus = user_xcpus(cs);
>> + struct cpuset *parent = parent_cs(cs);
>> + int new_prs = cs->partition_root_state;
>> + bool cpumask_updated = false;
>> +
>> + lockdep_assert_held(&cpuset_mutex);
>> + WARN_ON_ONCE(is_remote_partition(cs)); /* For local partition only */
>> +
>> + if (!is_partition_valid(cs))
>> + return;
>> +
>> + /*
>> + * Make the current partition invalid.
>> + */
>> + if (is_partition_valid(parent))
>> + cpumask_updated = cpumask_and(tmp->addmask,
>> + xcpus, parent->effective_xcpus);
> Invalidation is different from disable. It can be called when parent is no longer a valid partition
> root. So the check here is appropriate.
In patch 17, I’ve unified local_partition_disable() and local_partition_invalidate() into a single
local_partition_disable() function—this simplifies the logic significantly. For remote partitions,
only remote_partition_disable() is used, making the interface symmetrical.
I split this into a separate patch solely to make the review clearer and easier.
Maybe I should directly replace the relevant logic in update_parent_effective_cpumask() with
local_partition_disable()?
>> + if (cs->partition_root_state > 0)
>> + new_prs = -cs->partition_root_state;
>> +
>> + partition_disable(cs, parent, new_prs, cs->prs_err);
>> + if (cpumask_updated) {
>
> The cpumask_and() operation above is no longer relevant as it should be done inside
> partition_disable(). Instead of cpumask_updated, we can just do a "is_partition_valid(parent))"
> check here to decide if the following two helpers should be called.
>
> Cheers,
> Longman
>
Similar to local_partition_disable, cpumask_updated indicates whether any CPUs need to be returned
to the parent. partition_disable will return the CPUs to the parent if tmp->addmask is empty.
However, since tmp->addmask may indeed be empty, I believe cpumask_updated is necessary.
In the next version, I’ll try directly replacing the relevant logic in
update_parent_effective_cpumask() with local_partition_disable()—this should make the code much clearer.
>
>> + cpuset_update_tasks_cpumask(parent, tmp->addmask);
>> + update_sibling_cpumasks(parent, cs, tmp);
>> + }
>> +}
>> +
>> /**
>> * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
>> * @cs: The cpuset that requests change in partition root state
>> @@ -1974,22 +2008,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>> adding = deleting = false;
>> old_prs = new_prs = cs->partition_root_state;
>> - if (cmd == partcmd_invalidate) {
>> - if (is_partition_invalid(cs))
>> - return 0;
>> -
>> - /*
>> - * Make the current partition invalid.
>> - */
>> - if (is_partition_valid(parent))
>> - adding = cpumask_and(tmp->addmask,
>> - xcpus, parent->effective_xcpus);
>> - if (old_prs > 0)
>> - new_prs = -old_prs;
>> -
>> - goto write_error;
>> - }
>> -
>> /*
>> * The parent must be a partition root.
>> * The new cpumask, if present, or the current cpus_allowed must
>> @@ -2553,7 +2571,7 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset
>> *trialc
>> if (is_partition_valid(cp) &&
>> cpumask_intersects(xcpus, cp->effective_xcpus)) {
>> rcu_read_unlock();
>> - update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, tmp);
>> + local_partition_invalidate(cp, tmp);
>> rcu_read_lock();
>> }
>> }
>> @@ -2593,8 +2611,7 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>> trialcs->effective_xcpus, tmp);
>> } else {
>> if (trialcs->prs_err)
>> - update_parent_effective_cpumask(cs, partcmd_invalidate,
>> - NULL, tmp);
>> + local_partition_invalidate(cs, tmp);
>> else
>> update_parent_effective_cpumask(cs, partcmd_update,
>> trialcs->effective_xcpus, tmp);
>> @@ -4040,18 +4057,21 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks
>> *tmp)
>> * partitions.
>> */
>> if (is_local_partition(cs) && (!is_partition_valid(parent) ||
>> - tasks_nocpu_error(parent, cs, &new_cpus)))
>> + tasks_nocpu_error(parent, cs, &new_cpus))) {
>> partcmd = partcmd_invalidate;
>> + local_partition_invalidate(cs, tmp);
>> + }
>> /*
>> * On the other hand, an invalid partition root may be transitioned
>> - * back to a regular one with a non-empty effective xcpus.
>> + * back to a regular one with a non-empty user xcpus.
>> */
>> else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
>> - !cpumask_empty(cs->effective_xcpus))
>> + !cpumask_empty(user_xcpus(cs))) {
>> partcmd = partcmd_update;
>> + update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>> + }
>> if (partcmd >= 0) {
>> - update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>> if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
>> compute_partition_effective_cpumask(cs, &new_cpus);
>> cpuset_force_rebuild();
>
--
Best regards,
Ridong
Powered by blists - more mailing lists