在 2026/9/8 09:27, Ridong Chen 写道:
>
>
> On 9/7/2026 6:16 PM, Guopeng Zhang wrote:
>>
>>
>> 在 2026/9/4 09:55, Ridong Chen 写道:
>>>
>>>
>>> On 9/4/2026 9:25 AM, Ridong Chen wrote:
>>>>
>>>>
>>>> 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.
>>>>
>>>
>>> Regarding the naming: child_partition_error is a bit odd — it sounds like
>>> it's validating a hierarchical child partition, but it's actually
>>> validating the partition itself (the child argument). The parent is only
>>> needed as context for the validation logic, not because we're checking a
>>> subordinate partition.
>>>
>>> I'd suggest renaming it to something like:
>>>
>>> ```
>>> static enum prs_errcode cs_partition_error(struct cpuset *cs, ...)
>>>
>>> ```
>>>
>>> That would better reflect what the function actually does.
>>>
>>
>> Thanks, Ridong. I see what you mean about the name.
>>
>> My intention with "child" was to describe the role of the partition in these
>> checks. Both callers use the helper while walking a parent's child
>> partitions, and the result depends on the parent's complete exclusive mask,
>> whether the parent is populated, and the active CPUs remaining at that point
>> in the walk. The helper also checks only the two conditions that can
>> invalidate a child during this process, rather than performing general
>> validation of an arbitrary cpuset partition.
>>
>> For that reason, `cs_partition_error()` might suggest a broader scope than
>> the helper actually has. Would it make sense to keep
>> `child_partition_error()` and clarify the comment, for example:
>>
>
> As you can see, we are continuing to add partition checks, so I'd still
> suggest avoiding child_partition_error(), it would become confusing when we
> later refactor the partition validation logic. All of these functions are
> ultimately used to validate a partition, and the validation may depend on
> either the parent or a trialcs, or something else.
>
> So I'd propose keeping it as:
>
> static enum prs_errcode cs_partition_error(struct cpuset *cs, struct cpuset
> *parent);
>
> This is more general and better accommodates future extensions.
>
I see your point now. You are looking at this from a longer-term design
perspective. I will update it in the next version.
Thanks,
Guopeng
