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

Powered by Openwall GNU/*/Linux Powered by OpenVZ