[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2dc224c9-bd96-46a7-b71a-39abac847176@huaweicloud.com>
Date: Mon, 20 Oct 2025 16:28:29 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Waiman Long <llong@...hat.com>, tj@...nel.org, hannes@...xchg.org,
mkoutny@...e.com
Cc: cgups@...r.kernel.org, linux-kernel@...r.kernel.org,
lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next RFC 08/16] cpuset: introduce
local_partition_invalidate()
On 2025/10/20 10:48, Waiman Long wrote:
> On 9/28/25 3:12 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, 42 insertions(+), 24 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 87ba43e93540..e460d03286ba 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1911,6 +1911,39 @@ static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
>> }
>> }
>> +/**
>> + * 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_invalid(cs))
>> + return;
> You should change the check to if (!is_partition_valid(cs)). You can avoid the case that
> partition_disable() is called with a member.
>
Thank you for the suggestion.
I kept the current check to align with the logic from update_parent_effective_cpumask() and to keep
the changes easier to review.
In patch 12, I've unified local_partition_invalidate and the original local_partition_disable into a
single local_partition_disable function, which now uses the if (!is_partition_valid(cs)) check. This
also brings it in line with the existing remote_partition_disable().
Since the local_partition_invalidate is removed in the subsequent patches, I believe it's a minor issue.
>> + /*
>> + * Make the current partition invalid.
>> + */
>> + if (is_partition_valid(parent))
>> + cpumask_updated = cpumask_and(tmp->addmask,
>> + xcpus, parent->effective_xcpus);
>> + if (cs->partition_root_state > 0)
>> + new_prs = -cs->partition_root_state;
>> +
>> + partition_disable(cs, parent, new_prs, cs->prs_err);
>> + if (cpumask_updated) {
>> + 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
>> @@ -1972,23 +2005,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;
>> - subparts_delta--;
>> - }
>> - goto write_error;
>> - }
>> -
>> /*
>> * The parent must be a partition root.
>> * The new cpumask, if present, or the current cpus_allowed must
>> @@ -2552,7 +2568,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();
>> }
>> }
>> @@ -2592,8 +2608,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);
>> @@ -4037,18 +4052,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))) {
>
> I believe the effective_xcpus is not cleared currently when a partition is invalidated. Anyway, this
> change is also OK especially if effective_xcpus will be cleared in a later patch.
>
> Cheers,
> Longman
>
>> 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