在 2026/9/8 09:40, Ridong Chen 写道:
>
>
> On 9/7/2026 5:49 PM, Guopeng Zhang wrote:
>>
>>
>> 在 2026/9/4 09:25, Ridong Chen 写道:
>>>
>>>
>>> On 9/4/2026 2:33 AM, Waiman Long wrote:
>>>> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>>>>> From: Guopeng Zhang <[email protected]>
>>>>>
>>>>> compute_partition_effective_cpumask() checks whether each valid child
>>>>> partition remains covered by the parent exclusive CPU mask and whether it
>>>>> would consume all remaining CPUs of a populated parent.
>>>>>
>>>>> Factor these two checks into child_partition_error() so the same rules can
>>>>> be reused when evaluating a proposed parent configuration. This is a
>>>>> preparatory refactoring with no intended functional change.
>>>>>
>>>>> Signed-off-by: Guopeng Zhang <[email protected]>
>>>>> ---
>>>>> kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>>>>> 1 file changed, 29 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>>> index 8f24171b6055..6994dc75d940 100644
>>>>> --- a/kernel/cgroup/cpuset.c
>>>>> +++ b/kernel/cgroup/cpuset.c
>>>>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct
>>>>> cpuset *cs, int cmd,
>>>>> return 0;
>>>>> }
>>>>> +/*
>>>>> + * Return the error that will invalidate a child partition under a
>>>>> proposed
>>>>> + * parent partition configuration.
>>>>> + */
>>>>> +static enum prs_errcode
>>>>> +child_partition_error(struct cpuset *child,
>>>>> + const struct cpumask *partition_cpus,
>>>>> + const struct cpumask *remaining_cpus,
>>>>> + bool parent_populated)
>>>> I think you should add some functional comments on what "partition_cpus"
>>>> and "remaining_cpus" are supposed to be so that caller knows what to pass
>>>> into this helper.
>>>
>>> Would it help to rename them to excpus and local_excpus?
>>> local already implies "excluding children" (just like cgroup.stat.local),
>>> so I think that makes the intent clearer.
>>>
>>
>> Ridong, thanks for your review.
>>
>> Renaming the masks would help, but I think `local_excpus` may be slightly
>> misleading here. When the helper is called, the mask may still contain
>> active CPUs assigned to the current child and to children that have not yet
>> been visited. It excludes only the CPUs of previously visited children that
>> remain valid under the configuration being evaluated, and the mask itself is
>> restricted to active CPUs.
>>
>> In addition, "local" already has the local-versus-remote partition meaning
>> in cpuset.
>>
>> Would `parent_xcpus` for the complete effective exclusive mask, which may
>> include offline CPUs, and `remaining_ecpus` for the remaining active CPUs
>> before evaluating the current child be clearer? I will also document both
>> parameters explicitly.
>>
>
> Passing struct cpuset *parent into the function simplifies naming, as the
> needed values become local variables derived from it.
>
Thanks, Ridong.
I agree that passing `parent` would make the interface simpler, but I don't
think `parent` alone provides all the state needed by this helper.
In `compute_partition_effective_cpumask()`, the remaining active CPU mask is
temporary state maintained during the child walk:
compute_excpus(cs, new_ecpus);
cpumask_and(new_ecpus, new_ecpus, cpu_active_mask);
cpuset_for_each_child(child, css, cs) {
...
if (child_err)
continue;
cpumask_andnot(new_ecpus, new_ecpus,
child->effective_xcpus);
}
So `new_ecpus` changes as valid children are processed and cannot be replaced
by `parent->effective_cpus`. In this update path, `parent->effective_cpus`
still contains the previously published effective mask, while `new_ecpus`
already represents the active CPUs remaining before the current child is
evaluated.
For example, assume CPUs 1-3 are active, the parent is populated, and child A
is visited before child B. Initially:
parent:
effective_xcpus = 1-3
effective_cpus = 3
child A:
effective_xcpus = 1
child B:
effective_xcpus = 2
If the parent's effective exclusive mask is changed to `1-2`, this update path
publishes the new `effective_xcpus` before recomputing `effective_cpus`.
Therefore, when entering `compute_partition_effective_cpumask()`:
parent->effective_xcpus = 1-2
parent->effective_cpus = 3
new_ecpus = 1-2
After child A remains valid, its CPU is removed from `new_ecpus`:
new_ecpus = 2
So when child B is evaluated, the `PERR_NOCPUS` check uses:
cpumask_subset(new_ecpus, child_B->effective_xcpus)
= cpumask_subset(2, 2)
= true
Since the parent is populated, the `PERR_NOCPUS` condition is met for child B.
At the same point:
remaining mask needed by the check = 2
parent->effective_cpus = 3
Using `parent->effective_cpus` instead would give:
cpumask_subset(parent->effective_cpus,
child_B->effective_xcpus)
= cpumask_subset(3, 2)
= false
and would miss the condition.
So unless `cs_partition_error()` also takes over or reconstructs the relevant
child-walk state, passing only `parent` cannot provide the remaining active CPU
mask needed to evaluate the current child.
The next patch also reuses this helper for a trial parent configuration. In
that case, the complete mask being evaluated comes from
`trialcs->effective_xcpus`, rather than from the currently published state in
the real parent. The follow-up child-invalidation fix similarly needs to
distinguish the complete effective exclusive mask from the remaining active CPU
mask.
So for now, I will use the more general `cs_partition_error()` name, but keep
the two masks explicit in the interface.
In the next version, I plan to include the remaining patches related to this
helper and the associated invalidation paths so that all the call sites and use
cases can be reviewed together.
I may still be thinking within the constraints of the current code structure,
so if you see a cleaner interface once the full picture is visible, I would
very much appreciate your suggestion.
Thanks,
Guopeng
