All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guopeng Zhang <guopeng.zhang@linux.dev>
To: "Waiman Long" <longman@redhat.com>,
	"Ridong Chen" <ridong.chen@linux.dev>,
	"Tejun Heo" <tj@kernel.org>,
	"Johannes Weiner" <hannes@cmpxchg.org>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Shuah Khan" <shuah@kernel.org>
Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH-next] cgroup/cpuset: Fix incorrect CPU accounting with parent's effective_xcpus expansion
Date: Thu, 29 Oct 2026 17:42:52 +0800	[thread overview]
Message-ID: <661486ab-4879-47cd-8035-ae0d7a7d7462@linux.dev> (raw)
In-Reply-To: <20261028152608.648427-1-longman@redhat.com>



在 2026/10/28 23:26, Waiman Long 写道:
> Commit ec0254b9eb49 ("cgroup/cpuset: Release invalid isolated CPUs with
> a shrunk root partition parent") fixes incorrect isolated_cpus accounting
> when contraction of a parent's effective_xcpus can cause a child partition
> to lose isolated CPUs. Expansion of parent's effective_cpus can also allow
> a child partition to gain more effective_xcpus. However, the current code
> doesn't handle this case correctly leading to incorrect effective_cpus
> and isolated_cpus accounting. Fix that by looking for this possible
> expansion of effective_xcpus and perform the proper accounting action.
> 
> New test cases are added to test this effective_xcpus expansion scenarios.
> 
> Reported-by: Guopeng Zhang <guopeng.zhang@linux.dev>
> Link: https://lore.kernel.org/lkml/20261027123837.100909-1-guopeng.zhang@linux.dev
> Signed-off-by: Waiman Long <longman@redhat.com>

I have tested this patch. In the scenarios I tested, the double grant
and the isolated_cpus accounting after the expansion are now correct.

> ---
>  kernel/cgroup/cpuset.c                            | 12 ++++++++++++
>  tools/testing/selftests/cgroup/test_cpuset_prs.sh | 15 +++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 3219f927c35e..39c81a0a8ab9 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2097,6 +2097,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  							     parent->effective_xcpus);
>  			if (!cpumask_empty(tmp->addmask))
>  				isolated_cpus_release(cs, tmp->addmask);
> +			/* Expansion of parent's effective_xcpus is handled below */
>  		}
>  		if (nocpu) {
>  			part_error = PERR_NOCPUS;
> @@ -2106,11 +2107,22 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  						     parent->effective_xcpus);
>  		} else if ((parent_prs == PRS_ISOLATED) &&
>  			   ((old_prs == PRS_ROOT) || (old_prs == -PRS_ROOT))) {
> +			/* Parent partition state changed to PRS_ISOLATED */
>  			part_error = PERR_ISOPARENT;
>  			if (old_prs == PRS_ROOT) {
>  				cpumask_copy(tmp->addmask, cs->effective_xcpus);
>  				adding = true;
>  			}
> +		} else if (is_partition_valid(cs) &&
> +			   !cpumask_equal(tmp->delmask, cs->effective_xcpus)) {

The branch is also entered when the freshly computed effective xcpus
shrink, e.g. after a parent contraction. The cpumask_andnot() then
computes an empty delta and it doesn't seem to cause any harm. Do we
need to tighten the condition?

> +			/*
> +			 * If there is an expansion of parent's effective_xcpus,
> +			 * we may take more xcpus from parent. The parent's
> +			 * effective_cpus should still have active CPUs or nocpu
> +			 * should have been set.
> +			 */
> +			deleting = cpumask_andnot(tmp->delmask, tmp->delmask,
> +						  cs->effective_xcpus);
>  		} else if (is_partition_invalid(cs)) {
>  			struct cgroup_subsys_state *css;
>  			struct cpuset *child;
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index 83ef6c781af3..844efdf029cd 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -531,6 +531,21 @@ TEST_MATRIX=(
>  	" C1-4:P1 C3-4:P2   .      .    T:C4-6   .      .      .     0 A1:5-6|A2:4 A1:P1|A2:P2 4"
>  	" C1-4:P1 C1-2:P2   .      .    T:C4-6   .      .      .     0 A1:4-6|A2:4-6 A1:P1|A2:P-2 ."
>  
> +	# The extra CPUs gained by a child partition when its parent expands the
> +	# exclusive CPUs have to be taken away from the parent.
> +	" C2-6:X2-5:P1 CX3-6:P1 \
> +	                    .      .    X2-6     .      .      .     0 A1:2|A2:3-6|XA2:3-6 A1:P1|A2:P1 ."
> +	" C2-6:X2-5:P1 CX3-6:P2 \
> +	                    .      .    X2-6     .      .      .     0 A1:2|A2:3-6|XA2:3-6 A1:P1|A2:P2 3-6"
> +
> +	# Both expansion and contraction of parent's effective_xcpus will be
> +	# correctly handled in child partition and the child can be invalidated
> +	# if a populated parent could lose all its effective_cpus.
> +	" C2-6:X2-5:P1 CX3,5-6:P2 \
> +	                    .      .    X4-6     .      .      .     0 A1:4|A2:5-6|XA2:5-6 A1:P1|A2:P2 5-6"
> +	" C2-6:X2-5:P1 CX3-6:P2 \
> +	                    .      .   T:X4-6    .      .      .     0 A1:4-6|A2:4-6 A1:P1|A2:P-2 ."
> +
>  	#
>  	#  old-A1 old-A2 old-A3 old-B1 new-A1 new-A2 new-A3 new-B1 fail ECPUs Pstate ISOLCPUS
>  	#  ------ ------ ------ ------ ------ ------ ------ ------ ---- ----- ------ --------

BTW, about the housekeeping check issue from my previous email.
I verified it with the session below on a nohz_full=8-31 boot where
CPUs 0-7 are the housekeeping CPUs:

  # cat /sys/devices/system/cpu/nohz_full
  8-31
  # cd /sys/fs/cgroup
  # echo +cpuset > cgroup.subtree_control
  # mkdir -p A/B
  # echo 0-7 > A/cpuset.cpus
  # echo 1-7 > A/cpuset.cpus.exclusive
  # echo root > A/cpuset.cpus.partition
  # echo +cpuset > A/cgroup.subtree_control
  # echo 0-7 > A/B/cpuset.cpus
  # echo 0-7 > A/B/cpuset.cpus.exclusive
  # echo isolated > A/B/cpuset.cpus.partition
  # cat /sys/fs/cgroup/cpuset.cpus.isolated
  1-7
  # cat A/B/cpuset.cpus.exclusive.effective
  1-7
  # echo 0-7 > A/cpuset.cpus.exclusive
  # cat A/B/cpuset.cpus.partition
  isolated
  # cat /sys/fs/cgroup/cpuset.cpus.isolated
  0-7

B stays a valid isolated partition holding all the housekeeping CPUs
and there is no warning at all.

If the housekeeping issue is going to be addressed in this series, is
a test case like the one below worth adding?

+	# An isolated child partition cannot grow to take over the last
+	# housekeeping CPU when its parent expands the exclusive CPUs.
+	" C0-7:X1-7:P1 C0-7:X0-7:P2 .      .      X0-7     .      .      .     0 A1:0-7 A1:P1|A2:P-2 ."

Thanks,
Guopeng

  reply	other threads:[~2026-10-29  9:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-28 15:26 [PATCH-next] cgroup/cpuset: Fix incorrect CPU accounting with parent's effective_xcpus expansion Waiman Long
2026-10-29  9:42 ` Guopeng Zhang [this message]
2026-10-29 18:04   ` Waiman Long

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=661486ab-4879-47cd-8035-ae0d7a7d7462@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 \
    /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.