All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guopeng Zhang <guopeng.zhang@linux.dev>
To: Waiman Long <longman@redhat.com>, cgroups@vger.kernel.org
Cc: ridong.chen@linux.dev, tj@kernel.org, hannes@cmpxchg.org,
	mkoutny@suse.com, shuah@kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Guopeng Zhang <zhangguopeng@kylinos.cn>
Subject: Re: [PATCH 12/17] cgroup/cpuset: Invalidate children outside the new CPU mask
Date: Thu, 10 Sep 2026 11:41:26 +0800	[thread overview]
Message-ID: <3830e505-70eb-4c0b-8d1c-74074f066142@linux.dev> (raw)
In-Reply-To: <3bf50da2-f13a-4cef-bfb8-cd592f862164@redhat.com>



在 2026/8/25 23:11, Waiman Long 写道:
> On 8/20/26 8:41 AM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> When cpuset.cpus changes, compute_partition_effective_cpumask() builds a
>> new exclusive mask but checks child partitions against
>> cs->effective_xcpus. That field still contains the old mask, so a child
>> that no longer fits can remain valid.
>>
>> Use new_xcpus for the check. A later partcmd_update() may revisit the
>> newly invalid child. Report PERR_INVCPUS if the child CPUs are outside
>> the parent effective exclusive mask so that this visit does not make the
>> child valid again.
>>
>> Fixes: 0c7f293efc87 ("cgroup/cpuset: Add cpuset.cpus.exclusive.effective for v2")
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>>   kernel/cgroup/cpuset.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index b9faadf4af6d..b3e749ede7d1 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1987,12 +1987,16 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>                   adding = cpumask_and(tmp->addmask,
>>                                cs->effective_xcpus,
>>                                parent->effective_xcpus);
>> -        } else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
>> -               cpumask_subset(xcpus, parent->effective_xcpus)) {
>> +        } else if (is_partition_invalid(cs) && !cpumask_empty(xcpus)) {
>>               struct cgroup_subsys_state *css;
>>               struct cpuset *child;
>>               bool exclusive = true;
>>   +            if (!cpumask_subset(xcpus, parent->effective_xcpus)) {
>> +                part_error = PERR_INVCPUS;
>> +                goto write_error;
>> +            }
>> +
> A invalid partition means part_error should be set to some error code. This change only makes sure the error code is PERR_INVCPUS. Other than that, I don't see any other tangible change here.

Thanks, Longman.

I think there is a functional difference here. `part_error` is a local variable initialized to `PERR_NONE` on every call to `update_parent_effective_cpumask()`; it is not initialized from `cs->prs_err`.

With the current condition:

} else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
	   cpumask_subset(xcpus, parent->effective_xcpus)) {

an invalid partition whose CPUs are no longer a subset of the parent's effective exclusive mask skips this branch entirely, leaving `part_error` as `PERR_NONE`.

The later state transition then does:

case PRS_INVALID_ROOT:
case PRS_INVALID_ISOLATED:
	if (!part_error)
		new_prs = -old_prs;
	break;

so the partition can be changed back to a valid state.

Therefore, this change does more than set the error code to `PERR_INVCPUS`; it also prevents an out-of-mask invalid partition from being made valid again.

>>               /*
>>                * Convert invalid partition to valid has to
>>                * pass the cpu exclusivity test.
>> @@ -2144,7 +2148,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>           WARN_ON_ONCE(is_remote_partition(child));
>>           WRITE_ONCE(child->prs_err, 0);
>>           if (!cpumask_subset(child->effective_xcpus,
>> -                    cs->effective_xcpus))
>> +                    new_xcpus))
>>               WRITE_ONCE(child->prs_err, PERR_INVCPUS);
>>           else if (populated &&
>>                cpumask_subset(new_ecpus, child->effective_xcpus))
> 
> This hunk should probably be grouped into the same patch making change to compute_partition_effective_cpumask().

I have included these changes in v4 and moved both changes from this patch into the earlier patch that modifies compute_partition_effective_cpumask().

I have also temporarily dropped all selftest patches, including those posted in v3, as the current test design needs some rework.

Thanks,
Guopeng


  reply	other threads:[~2026-09-10  3:41 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20 12:41 [PATCH 00/17] cgroup/cpuset: Fix partition CPU ownership and isolation accounting Guopeng Zhang
2026-08-20 12:41 ` [PATCH 01/17] selftests/cgroup: Drop invalid boot isolation comparison Guopeng Zhang
2026-08-20 12:41 ` [PATCH 02/17] cgroup/cpuset: Preserve boot-isolated CPUs on partition release Guopeng Zhang
2026-08-20 12:41 ` [PATCH 03/17] selftests/cgroup: Test boot-isolated CPU " Guopeng Zhang
2026-08-20 12:41 ` [PATCH 04/17] cgroup/cpuset: Limit type-change accounting to owned CPUs Guopeng Zhang
2026-08-24 14:24   ` Waiman Long
2026-08-26  5:44     ` Guopeng Zhang
2026-08-20 12:41 ` [PATCH 05/17] selftests/cgroup: Test isolated CPU accounting on type changes Guopeng Zhang
2026-08-20 12:41 ` [PATCH 06/17] cgroup/cpuset: Validate type changes against owned CPUs Guopeng Zhang
2026-08-24 14:25   ` Waiman Long
2026-08-26  5:48     ` Guopeng Zhang
2026-08-20 12:41 ` [PATCH 07/17] selftests/cgroup: Test type-change validation with child-owned CPUs Guopeng Zhang
2026-08-24 14:31   ` Waiman Long
2026-08-26  5:51     ` Guopeng Zhang
2026-08-20 12:41 ` [PATCH 08/17] cgroup/cpuset: Release CPUs when a type change is rejected Guopeng Zhang
2026-08-24 14:37   ` Waiman Long
2026-08-26  5:53     ` Guopeng Zhang
2026-08-20 12:41 ` [PATCH 09/17] selftests/cgroup: Test rejected partition type changes Guopeng Zhang
2026-08-20 12:41 ` [PATCH 10/17] cgroup/cpuset: Fix isolated accounting on direct child invalidation Guopeng Zhang
2026-08-20 12:41 ` [PATCH 11/17] selftests/cgroup: Test isolation " Guopeng Zhang
2026-08-20 12:41 ` [PATCH 12/17] cgroup/cpuset: Invalidate children outside the new CPU mask Guopeng Zhang
2026-08-25 15:11   ` Waiman Long
2026-09-10  3:41     ` Guopeng Zhang [this message]
2026-08-20 12:41 ` [PATCH 13/17] selftests/cgroup: Test child invalidation after shrinking cpuset.cpus Guopeng Zhang
2026-08-20 12:41 ` [PATCH 14/17] cgroup/cpuset: Publish cpus_allowed before partition updates Guopeng Zhang
2026-08-20 12:42 ` [PATCH 15/17] selftests/cgroup: Test shrinking cpuset.cpus in a remote partition Guopeng Zhang
2026-08-20 12:42 ` [PATCH 16/17] cgroup/cpuset: Fix isolated accounting on propagated invalidation Guopeng Zhang
2026-08-20 12:42 ` [PATCH 17/17] selftests/cgroup: Test isolation " Guopeng Zhang
2026-08-21  2:48 ` [PATCH 00/17] cgroup/cpuset: Fix partition CPU ownership and isolation accounting Ridong Chen
2026-08-21  3:17   ` Guopeng Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3830e505-70eb-4c0b-8d1c-74074f066142@linux.dev \
    --to=guopeng.zhang@linux.dev \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mkoutny@suse.com \
    --cc=ridong.chen@linux.dev \
    --cc=shuah@kernel.org \
    --cc=tj@kernel.org \
    --cc=zhangguopeng@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.