[PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[Waiman Long]

The current cpuset code and test_cpuset_prs.sh test have not fully
account for the possibility of pre-isolated CPUs added by the "isolcpus"
boot command line parameter. This patch series modifies them to do the
right thing whether or not "isolcpus" is present or not.

The updated test_cpuset_prs.sh was run successfully with or without the
"isolcpus" option.

Waiman Long (2):
  cgroup/cpuset: Account for boot time isolated CPUs
  selftest/cgroup: Make test_cpuset_prs.sh deal with pre-isolated CPUs

 kernel/cgroup/cpuset.c                        | 23 +++++++---
 .../selftests/cgroup/test_cpuset_prs.sh       | 44 ++++++++++++++-----
 2 files changed, 51 insertions(+), 16 deletions(-)

-- 
2.43.5

[PATCH-cgroup 1/2] cgroup/cpuset: Account for boot time isolated CPUs

[Waiman Long]

With the "isolcpus" boot command line parameter, we are able to
create isolated CPUs at boot time. These isolated CPUs aren't fully
accounted for in the cpuset code. For instance, the root cgroup's
"cpuset.cpus.isolated" control file does not include the boot time
isolated CPUs. Fix that by looking for pre-isolated CPUs at init time.

The prstate_housekeeping_conflict() function does check the
HK_TYPE_DOMAIN housekeeping cpumask to make sure that CPUs outside of it
can only be used in isolated partition. Given the fact that we are going
to make housekeeping cpumasks dynamic, the current check may not be right
anymore. Save the boot time HK_TYPE_DOMAIN cpumask and check against
it instead of the upcoming dynamic HK_TYPE_DOMAIN housekeeping cpumask.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 7db55eed63cf..8b40df89c3c1 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -224,6 +224,12 @@ static cpumask_var_t	subpartitions_cpus;
  */
 static cpumask_var_t	isolated_cpus;
 
+/*
+ * Housekeeping (HK_TYPE_DOMAIN) CPUs at boot
+ */
+static cpumask_var_t	boot_hk_cpus;
+static bool		have_boot_isolcpus;
+
 /* List of remote partition root children */
 static struct list_head remote_children;
 
@@ -1823,15 +1829,15 @@ static void remote_partition_check(struct cpuset *cs, struct cpumask *newmask,
  * @new_cpus: cpu mask
  * Return: true if there is conflict, false otherwise
  *
- * CPUs outside of housekeeping_cpumask(HK_TYPE_DOMAIN) can only be used in
- * an isolated partition.
+ * CPUs outside of boot_hk_cpus, if defined, can only be used in an
+ * isolated partition.
  */
 static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
 {
-	const struct cpumask *hk_domain = housekeeping_cpumask(HK_TYPE_DOMAIN);
-	bool all_in_hk = cpumask_subset(new_cpus, hk_domain);
+	if (!have_boot_isolcpus)
+		return false;
 
-	if (!all_in_hk && (prstate != PRS_ISOLATED))
+	if ((prstate != PRS_ISOLATED) && !cpumask_subset(new_cpus, boot_hk_cpus))
 		return true;
 
 	return false;
@@ -4345,6 +4351,13 @@ int __init cpuset_init(void)
 
 	BUG_ON(!alloc_cpumask_var(&cpus_attach, GFP_KERNEL));
 
+	have_boot_isolcpus = housekeeping_enabled(HK_TYPE_DOMAIN);
+	if (have_boot_isolcpus) {
+		BUG_ON(!alloc_cpumask_var(&boot_hk_cpus, GFP_KERNEL));
+		cpumask_copy(boot_hk_cpus, housekeeping_cpumask(HK_TYPE_DOMAIN));
+		cpumask_andnot(isolated_cpus, cpu_possible_mask, boot_hk_cpus);
+	}
+
 	return 0;
 }
 
-- 
2.43.5

Re: [PATCH-cgroup 1/2] cgroup/cpuset: Account for boot time isolated CPUs

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Hi.

On Tue, Aug 20, 2024 at 03:55:35PM GMT, Waiman Long <longman@redhat.com> wrote:
> The prstate_housekeeping_conflict() function does check the
> HK_TYPE_DOMAIN housekeeping cpumask to make sure that CPUs outside of it
> can only be used in isolated partition.
> Given the fact that we are going to make housekeeping cpumasks
> dynamic, the current check may not be right anymore. Save the boot
> time HK_TYPE_DOMAIN cpumask and check against it instead of the
> upcoming dynamic HK_TYPE_DOMAIN housekeeping cpumask.

Why is (will be) checking against the stored HK_TYPE_DOMAIN mask correct
when this mask becomes dynamic?

Thanks,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH-cgroup 1/2] cgroup/cpuset: Account for boot time isolated CPUs

[Waiman Long]

On 8/27/24 04:01, Michal Koutný wrote:
> Hi.
>
> On Tue, Aug 20, 2024 at 03:55:35PM GMT, Waiman Long <longman@redhat.com> wrote:
>> The prstate_housekeeping_conflict() function does check the
>> HK_TYPE_DOMAIN housekeeping cpumask to make sure that CPUs outside of it
>> can only be used in isolated partition.
>> Given the fact that we are going to make housekeeping cpumasks
>> dynamic, the current check may not be right anymore. Save the boot
>> time HK_TYPE_DOMAIN cpumask and check against it instead of the
>> upcoming dynamic HK_TYPE_DOMAIN housekeeping cpumask.
> Why is (will be) checking against the stored HK_TYPE_DOMAIN mask correct
> when this mask becomes dynamic?

In term of isolated CPUs, there are 2 categories - static and dynamic. 
Statically isolated CPUs are those that are designed as isolated at boot 
time by "isolcpus". Other isolated CPUs created by the cpuset isolated 
partitions are considered dynamic in the sense that its state can change 
at run time. The degree of CPU isolation of dynamically isolated CPUs 
isn't as good as that of the statically isolated ones. So I want to 
handle them separately which is what the prstate_housekeeping_conflict() 
is intended to do.

As it is my intention to make the housekeeping cpumasks dynamic, I need 
to keep a copy of the statically isolated CPUs and check against them. 
There is no point to check dynamically isolated CPUs as the test may 
produce a false positive result.

In the future when dynamic CPU isolation is as almost as good as the 
static ones, we can get rid of this distinction and treat all of them as 
dynamic.

Cheers,
Longman

[PATCH-cgroup 2/2] selftest/cgroup: Make test_cpuset_prs.sh deal with pre-isolated CPUs

[Waiman Long]

Since isolated CPUs can be reserved at boot time via the "isolcpus"
boot command line option, these pre-isolated CPUs may interfere with
testing done by test_cpuset_prs.sh.

With the previous commit that incorporates those boot time isolated CPUs
into "cpuset.cpus.isolated", we can check for those before testing is
started to make sure that there will be no interference.  Otherwise,
this test will be skipped if incorrect test failure can happen.

As "cpuset.cpus.isolated" is now available in a non cgroup_debug kernel,
we don't need to check for its existence anymore.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 .../selftests/cgroup/test_cpuset_prs.sh       | 44 ++++++++++++++-----
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index 7295424502b9..03c1bdaed2c3 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -84,6 +84,20 @@ echo member > test/cpuset.cpus.partition
 echo "" > test/cpuset.cpus
 [[ $RESULT -eq 0 ]] && skip_test "Child cgroups are using cpuset!"
 
+#
+# If isolated CPUs have been reserved at boot time (as shown in
+# cpuset.cpus.isolated), these isolated CPUs should be outside of CPUs 0-7
+# that will be used by this script for testing purpose. If not, some of
+# the tests may fail incorrectly. These isolated CPUs will also be removed
+# before being compared with the expected results.
+#
+BOOT_ISOLCPUS=$(cat $CGROUP2/cpuset.cpus.isolated)
+if [[ -n "$BOOT_ISOLCPUS" ]]
+then
+	[[ $(echo $BOOT_ISOLCPUS | sed -e "s/[,-].*//") -le 7 ]] &&
+		skip_test "Pre-isolated CPUs ($BOOT_ISOLCPUS) overlap CPUs to be tested"
+	echo "Pre-isolated CPUs: $BOOT_ISOLCPUS"
+fi
 cleanup()
 {
 	online_cpus
@@ -642,7 +656,8 @@ check_cgroup_states()
 # Note that isolated CPUs from the sched/domains context include offline
 # CPUs as well as CPUs in non-isolated 1-CPU partition. Those CPUs may
 # not be included in the cpuset.cpus.isolated control file which contains
-# only CPUs in isolated partitions.
+# only CPUs in isolated partitions as well as those that are isolated at
+# boot time.
 #
 # $1 - expected isolated cpu list(s) <isolcpus1>{,<isolcpus2>}
 # <isolcpus1> - expected sched/domains value
@@ -669,18 +684,21 @@ check_isolcpus()
 	fi
 
 	#
-	# Check the debug isolated cpumask, if present
+	# Check cpuset.cpus.isolated cpumask
 	#
-	[[ -f $ISCPUS ]] && {
+	if [[ -z "$BOOT_ISOLCPUS" ]]
+	then
+		ISOLCPUS=$(cat $ISCPUS)
+	else
+		ISOLCPUS=$(cat $ISCPUS | sed -e "s/,*$BOOT_ISOLCPUS//")
+	fi
+	[[ "$EXPECT_VAL2" != "$ISOLCPUS" ]] && {
+		# Take a 50ms pause and try again
+		pause 0.05
 		ISOLCPUS=$(cat $ISCPUS)
-		[[ "$EXPECT_VAL2" != "$ISOLCPUS" ]] && {
-			# Take a 50ms pause and try again
-			pause 0.05
-			ISOLCPUS=$(cat $ISCPUS)
-		}
-		[[ "$EXPECT_VAL2" != "$ISOLCPUS" ]] && return 1
-		ISOLCPUS=
 	}
+	[[ "$EXPECT_VAL2" != "$ISOLCPUS" ]] && return 1
+	ISOLCPUS=
 
 	#
 	# Use the sched domain in debugfs to check isolated CPUs, if available
@@ -713,6 +731,9 @@ check_isolcpus()
 		fi
 	done
 	[[ "$ISOLCPUS" = *- ]] && ISOLCPUS=${ISOLCPUS}$LASTISOLCPU
+	[[ -n "BOOT_ISOLCPUS" ]] &&
+		ISOLCPUS=$(echo $ISOLCPUS | sed -e "s/,*$BOOT_ISOLCPUS//")
+
 	[[ "$EXPECT_VAL" = "$ISOLCPUS" ]]
 }
 
@@ -730,7 +751,8 @@ test_fail()
 }
 
 #
-# Check to see if there are unexpected isolated CPUs left
+# Check to see if there are unexpected isolated CPUs left beyond the boot
+# time isolated ones.
 #
 null_isolcpus_check()
 {
-- 
2.43.5

Re: [PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[Waiman Long]

On 8/20/24 15:55, Waiman Long wrote:
> The current cpuset code and test_cpuset_prs.sh test have not fully
> account for the possibility of pre-isolated CPUs added by the "isolcpus"
> boot command line parameter. This patch series modifies them to do the
> right thing whether or not "isolcpus" is present or not.
>
> The updated test_cpuset_prs.sh was run successfully with or without the
> "isolcpus" option.
>
> Waiman Long (2):
>    cgroup/cpuset: Account for boot time isolated CPUs
>    selftest/cgroup: Make test_cpuset_prs.sh deal with pre-isolated CPUs
>
>   kernel/cgroup/cpuset.c                        | 23 +++++++---
>   .../selftests/cgroup/test_cpuset_prs.sh       | 44 ++++++++++++++-----
>   2 files changed, 51 insertions(+), 16 deletions(-)
>
Ping! Any comment on these patches?

Thanks,
Longman

Re: [PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[Tejun Heo]

Hello, Waiman.

On Mon, Aug 26, 2024 at 03:01:21PM -0400, Waiman Long wrote:
> Ping! Any comment on these patches?

cgroup side looks fine to me. Ping isol people?

Thanks.

-- 
tejun

Re: [PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[Waiman Long]

On 8/26/24 15:05, Tejun Heo wrote:
> Hello, Waiman.
>
> On Mon, Aug 26, 2024 at 03:01:21PM -0400, Waiman Long wrote:
>> Ping! Any comment on these patches?
> cgroup side looks fine to me. Ping isol people?

This patchset is specific to cpuset code and doesn't touch any isolation 
code at all. My other isolation related patch series [1] does have a 
dependency on this one to work correctly.

[1] https://lore.kernel.org/lkml/20240821142312.236970-1-longman@redhat.com/

Cheers,
Longman

Re: [PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[Tejun Heo]

On Mon, Aug 26, 2024 at 03:41:23PM -0400, Waiman Long wrote:
> 
> On 8/26/24 15:05, Tejun Heo wrote:
> > Hello, Waiman.
> > 
> > On Mon, Aug 26, 2024 at 03:01:21PM -0400, Waiman Long wrote:
> > > Ping! Any comment on these patches?
> > cgroup side looks fine to me. Ping isol people?
> 
> This patchset is specific to cpuset code and doesn't touch any isolation
> code at all. My other isolation related patch series [1] does have a
> dependency on this one to work correctly.
> 
> [1] https://lore.kernel.org/lkml/20240821142312.236970-1-longman@redhat.com/

Oh, I was confused of the two patchsets. I thought the second one was the
replacement for this one. Will take another look.

Thanks.

-- 
tejun

Re: [PATCH-cgroup 0/2] cgroup/cpuset: Account for boot time isolated CPUs

[Tejun Heo]

On Tue, Aug 20, 2024 at 03:55:34PM -0400, Waiman Long wrote:
> The current cpuset code and test_cpuset_prs.sh test have not fully
> account for the possibility of pre-isolated CPUs added by the "isolcpus"
> boot command line parameter. This patch series modifies them to do the
> right thing whether or not "isolcpus" is present or not.
> 
> The updated test_cpuset_prs.sh was run successfully with or without the
> "isolcpus" option.
> 
> Waiman Long (2):
>   cgroup/cpuset: Account for boot time isolated CPUs
>   selftest/cgroup: Make test_cpuset_prs.sh deal with pre-isolated CPUs

Applied 1-2 to cgroup/for-6.12.

Thanks.

-- 
tejun