cgroup/cpuset: Preserve boot-isolated CPUs on partition release
Subsystems (3)
v2 cover cgroup/cpuset: Preserve boot-isolated CPUs on partition release
From: Guopeng Zhang <[email protected]>
This series fixes the loss of boot-isolated CPUs when an isolated cpuset partition is released. It also removes a selftest comparison against a boot-only sysfs mask which cannot track dynamic cpuset isolation.
---
Changes since v1:
- split the boot-isolation fixes from the original 17-patch series;
- explain why the boot-only sysfs mask cannot validate dynamic cpuset isolation;
- add reproducers and observed/expected results to patches 1 and 2;
- no code changes.
Guopeng Zhang (3):
selftests/cgroup: Drop invalid boot isolation comparison cgroup/cpuset: Preserve boot-isolated CPUs on partition release selftests/cgroup: Add test for preserving boot-isolated CPUs
kernel/cgroup/cpuset.c | 39 ++++++++--- .../selftests/cgroup/test_cpuset_prs.sh | 64 +++++++++++++++++-- 2 files changed, 87 insertions(+), 16 deletions(-)
base-commit: 6a746cd265aed59107ebdaa9ce039bb832922969
Signature · 1 line
2.43.0
PATCH v2 1/3 selftests/cgroup: Drop invalid boot isolation comparison
From: Guopeng Zhang <[email protected]>
check_isolcpus() clears ISOLCPUS before rebuilding it from sched domain data. Comparing that empty value with /sys/devices/system/cpu/isolated makes the test fail whenever isolcpus=domain is present.
That sysfs file is generated from HK_TYPE_DOMAIN_BOOT and does not change when cpuset updates HK_TYPE_DOMAIN. Re-reading it cannot validate dynamic housekeeping updates. The cpuset.cpus.isolated and sched domain checks already cover the two dynamic interfaces, so remove the invalid comparison.
This can be reproduced on a kernel booted with isolcpus=domain,15:
# tools/testing/selftests/cgroup/test_cpuset_prs.sh
The test fails its first state-matrix isolation check before the change and continues past that check afterward.
--- tools/testing/selftests/cgroup/test_cpuset_prs.sh | 6 ------ 1 file changed, 6 deletions(-)
tools/testing/selftests/cgroup/test_cpuset_prs.sh +0 -6
@@ -797,7 +797,6 @@ check_isolcpus() EXPECTED_ISOLCPUS=$1 ISCPUS=${CGROUP2}/cpuset.cpus.isolated ISOLCPUS=$(cat $ISCPUS) - HKICPUS=$(cat /sys/devices/system/cpu/isolated) LASTISOLCPU= SCHED_DOMAINS=/sys/kernel/debug/sched/domains if [[ $EXPECTED_ISOLCPUS = . ]]
@@ -835,11 +834,6 @@ check_isolcpus() ISOLCPUS= EXPECTED_ISOLCPUS=$EXPECTED_SDOMAIN - # - # The inverse of HK_TYPE_DOMAIN cpumask in $HKICPUS should match $ISOLCPUS - # - [[ "$ISOLCPUS" != "$HKICPUS" ]] && return 1 - # # Use the sched domain in debugfs to check isolated CPUs, if available #
Signature · 1 line
2.43.0
Re: [PATCH v2 1/3] selftests/cgroup: Drop invalid boot isolation comparison
Quoted 39 lines · Guopeng Zhang
From: Guopeng Zhang <[email protected]>
check_isolcpus()clears ISOLCPUS before rebuilding it from sched domain data. Comparing that empty value with/sys/devices/system/cpu/isolatedmakes the test fail whenever isolcpus=domain is present.That sysfs file is generated from
HK_TYPE_DOMAIN_BOOTand does not change when cpuset updatesHK_TYPE_DOMAIN. Re-reading it cannot validate dynamic housekeeping updates. The cpuset.cpus.isolated and sched domain checks already cover the two dynamic interfaces, so remove the invalid comparison.This can be reproduced on a kernel booted with isolcpus=domain,15:
# tools/testing/selftests/cgroup/test_cpuset_prs.shThe test fails its first state-matrix isolation check before the change and continues past that check afterward.
Fixes 6df415aa46ec ("cgroup/cpuset: Defer housekeeping_update() calls from CPU hotplug to workqueue")Signed-off-by Guopeng Zhang <[email protected]>--- tools/testing/selftests/cgroup/test_cpuset_prs.sh | 6 ------ 1 file changed, 6 deletions(-)tools/testing/selftests/cgroup/test_cpuset_prs.sh +0
-1@@ -797,7 +797,6 @@ check_isolcpus() EXPECTED_ISOLCPUS=$1 ISCPUS=${CGROUP2}/cpuset.cpus.isolated ISOLCPUS=$(cat $ISCPUS) - HKICPUS=$(cat /sys/devices/system/cpu/isolated) LASTISOLCPU= SCHED_DOMAINS=/sys/kernel/debug/sched/domains if [[ $EXPECTED_ISOLCPUS = . ]]@@ -835,11 +834,6 @@ check_isolcpus() ISOLCPUS=
tools/testing/selftests/cgroup/test_cpuset_prs.sh check_isolcpus() · line 839
Quoted 10 lines
tools/testing/selftests/cgroup/test_cpuset_prs.sh +0
-5EXPECTED_ISOLCPUS=$EXPECTED_SDOMAIN - # - # The inverse of HK_TYPE_DOMAIN cpumask in $HKICPUS should match $ISOLCPUS - # - [[ "$ISOLCPUS" != "$HKICPUS" ]] && return 1 - # # Use the sched domain in debugfs to check isolated CPUs, if available #
The /sys/devices/system/cpu/isolated had recently be changed to report the boot time isolcpus=domain value. So the check is no longer valid and should be removed. Of course the position of the check is also problematic. So it should be removed.
PATCH v2 2/3 cgroup/cpuset: Preserve boot-isolated CPUs on partition release
In reply to Guopeng ZhangFrom: Guopeng Zhang <[email protected]>
isolated_cpus tracks CPUs isolated with isolcpus= as well as CPUs in isolated cpuset partitions. When an isolated partition is released, isolated_cpus_update() removes its whole CPU mask. This also clears CPUs which were already isolated at boot.
This can be reproduced on a cgroup v2 system booted with isolcpus=domain,15:
cd /sys/fs/cgroup
echo +cpuset > cgroup.subtree_control mkdir cpuset-repro
echo 15 > cpuset-repro/cpuset.cpus
echo isolated > cpuset-repro/cpuset.cpus.partition echo member > cpuset-repro/cpuset.cpus.partition cat cpuset.cpus.isolated
CPU 15 is absent before the change. It must remain in cpuset.cpus.isolated after the partition is released.
Update isolated_cpus one CPU at a time and keep CPUs outside the boot-time domain housekeeping mask isolated.
--- kernel/cgroup/cpuset.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-)
kernel/cgroup/cpuset.c +29 -10
@@ -1259,6 +1259,28 @@ static void reset_partition_data(struct cpuset *cs)cpumask_copy (cs->effective_cpus, parent->effective_cpus); } +/* Return true if isolated_cpus changes. */ +static bool isolated_cpu_update (int new_prs,int cpu) +{ +lockdep_assert_held (&callback_lock); +lockdep_assert_held (&cpuset_mutex); + +if (new_prs ==PRS_ISOLATED ) { +if (cpumask_test_cpu (cpu, isolated_cpus)) +return false; +cpumask_set_cpu (cpu, isolated_cpus); +return true; + } + +/* CPUs isolated at boot must remain isolated. */ +if (!cpumask_test_cpu (cpu, +housekeeping_cpumask (HK_TYPE_DOMAIN_BOOT )) || + !cpumask_test_cpu (cpu, isolated_cpus)) +return false; +cpumask_clear_cpu (cpu, isolated_cpus); +return true; +} +/* * isolated_cpus_update - Update the isolated_cpus mask * @old_prs: old partition_root_state
@@ -1267,19 +1289,16 @@ static void reset_partition_data(struct cpuset *cs)*/ static void isolated_cpus_update (int old_prs,int new_prs,struct cpumask *xcpus) { +bool updated = false; +int cpu; +WARN_ON_ONCE (old_prs == new_prs);lockdep_assert_held (&callback_lock);lockdep_assert_held (&cpuset_mutex); -if (new_prs ==PRS_ISOLATED ) { -if (cpumask_subset (xcpus, isolated_cpus)) -return ; -cpumask_or (isolated_cpus, isolated_cpus, xcpus); - }else { -if (!cpumask_intersects (xcpus, isolated_cpus)) -return ; -cpumask_andnot (isolated_cpus, isolated_cpus, xcpus); - } - update_housekeeping = true; +for_each_cpu (cpu, xcpus) + updated |=isolated_cpu_update (new_prs, cpu); +if (updated) + update_housekeeping = true; }/*
Signature · 1 line
2.43.0
Quoted 80 lines · Guopeng Zhang
From: Guopeng Zhang <[email protected]>
isolated_cpustracks CPUs isolated with isolcpus= as well as CPUs in isolated cpuset partitions. When an isolated partition is released,isolated_cpus_update()removes its whole CPU mask. This also clears CPUs which were already isolated at boot.This can be reproduced on a cgroup v2 system booted with isolcpus=domain,15:
cd
/sys/fs/cgroup
echo +cpuset > cgroup.subtree_control mkdir cpuset-repro
echo 15 > cpuset-repro/cpuset.cpus
echo isolated > cpuset-repro/cpuset.cpus.partition echo member > cpuset-repro/cpuset.cpus.partition cat cpuset.cpus.isolatedCPU 15 is absent before the change. It must remain in cpuset.cpus.isolated after the partition is released.
Update
isolated_cpusone CPU at a time and keep CPUs outside the boot-time domain housekeeping mask isolated.Fixes c188f33c864e ("cgroup/cpuset: Account for boot time isolated CPUs")Signed-off-by Guopeng Zhang <[email protected]>--- kernel/cgroup/cpuset.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-)kernel/cgroup/cpuset.c +25
-7@@ -1259,6 +1259,28 @@ static void reset_partition_data(struct cpuset *cs)cpumask_copy (cs->effective_cpus, parent->effective_cpus); } +/* Return true if isolated_cpus changes. */ +static bool isolated_cpu_update (int new_prs,int cpu) +{ +lockdep_assert_held (&callback_lock); +lockdep_assert_held (&cpuset_mutex); + +if (new_prs ==PRS_ISOLATED ) { +if (cpumask_test_cpu (cpu, isolated_cpus)) +return false; +cpumask_set_cpu (cpu, isolated_cpus); +return true; + } + +/* CPUs isolated at boot must remain isolated. */ +if (!cpumask_test_cpu (cpu, +housekeeping_cpumask (HK_TYPE_DOMAIN_BOOT )) || + !cpumask_test_cpu (cpu, isolated_cpus)) +return false; +cpumask_clear_cpu (cpu, isolated_cpus); +return true; +} +/* * isolated_cpus_update - Update the isolated_cpus mask * @old_prs: old partition_root_state @@ -1267,19 +1289,16 @@ static void reset_partition_data(struct cpuset *cs)*/ static void isolated_cpus_update (int old_prs,int new_prs,struct cpumask *xcpus) { +bool updated = false; +int cpu; +WARN_ON_ONCE (old_prs == new_prs);lockdep_assert_held (&callback_lock);lockdep_assert_held (&cpuset_mutex); -if (new_prs ==PRS_ISOLATED ) { -if (cpumask_subset (xcpus, isolated_cpus)) -return ; -cpumask_or (isolated_cpus, isolated_cpus, xcpus); - }else { -if (!cpumask_intersects (xcpus, isolated_cpus)) -return ;
kernel/cgroup/cpuset.c static void reset_partition_data(struct cpuset *cs) · line 1304
Quoted 10 lines
kernel/cgroup/cpuset.c +4
-3-cpumask_andnot (isolated_cpus, isolated_cpus, xcpus); - } - update_housekeeping = true; +for_each_cpu (cpu, xcpus) + updated |=isolated_cpu_update (new_prs, cpu); +if (updated) + update_housekeeping = true; }/*
The code is technically correct. However, handling it one CPU at a time is less efficient from my point of view. It can be handled more efficiently from the cpumask level. I do notice that the new isolated_cpu_update() helper is being used in a later patch in your larger series. Maybe that is the reason why you do it this way.
I am OK with this change.
PATCH v2 3/3 selftests/cgroup: Add test for preserving boot-isolated CPUs
In reply to Guopeng ZhangFrom: Guopeng Zhang <[email protected]>
Put a CPU isolated at boot into an isolated partition, change the partition back to member and check that the CPU remains isolated.
--- .../selftests/cgroup/test_cpuset_prs.sh | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+)
tools/testing/selftests/cgroup/test_cpuset_prs.sh +58 -0
@@ -1155,6 +1155,63 @@ test_isolated() pause 0.05 } +# +# Select an online CPU isolated from scheduler domains at boot. +# $1: test name used in the skip message +# +get_boot_isolated_cpu() +{ + TEST_NAME=$1 + BOOT_ISOLATED_FILE=/sys/devices/system/cpu/isolated + + [[ -r $BOOT_ISOLATED_FILE ]] || { + echo "$TEST_NAME test SKIPPED: boot isolation state unavailable" + return 1 + } + BOOT_CPUS=$(cat $BOOT_ISOLATED_FILE) + [[ -n "$BOOT_CPUS" ]] || { + echo "$TEST_NAME test SKIPPED: no boot-isolated CPU" + return 1 + } + + BOOT_CPU=$(echo "$BOOT_CPUS" | sed -e 's/[,-].*//') + CPU_ONLINE=/sys/devices/system/cpu/cpu${BOOT_CPU}/online + [[ ! -e $CPU_ONLINE || $(cat $CPU_ONLINE) -eq 1 ]] || { + echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is offline" + return 1 + } +} + +# +# A CPU isolated at boot must stay isolated after it is released by a dynamic +# isolated partition. +# +test_boot_isolated() +{ + TEST_NAME="Boot-isolated CPU partition release" + get_boot_isolated_cpu "$TEST_NAME" || return 0 + echo "Running $TEST_NAME test ..." + + cd $CGROUP2/test + echo member > cpuset.cpus.partition + echo $BOOT_CPU > cpuset.cpus + [[ $(cat cpuset.cpus.effective) = "$BOOT_CPU" ]] || { + echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is unavailable" + echo "" > cpuset.cpus + cd $CGROUP2 + return 0 + } + test_partition isolated + test_partition member + check_isolcpus "." || { + echo "Boot-isolated CPU $BOOT_CPU was lost after partition release" + exit 1 + } + echo "" > cpuset.cpus + cd $CGROUP2 + echo "$TEST_NAME test PASSED." +} + # # Wait for inotify event for the given file and read it # $1: cgroup file to wait for
@@ -1226,5 +1283,6 @@ trap cleanup 0 2 3 6 run_state_test TEST_MATRIX run_remote_state_test REMOTE_TEST_MATRIX test_isolated +test_boot_isolated test_inotify echo "All tests PASSED."
Signature · 1 line
2.43.0
Re: [PATCH v2 3/3] selftests/cgroup: Add test for preserving boot-isolated CPUs
Quoted 79 lines · Guopeng Zhang
From: Guopeng Zhang <[email protected]>
Put a CPU isolated at boot into an isolated partition, change the partition back to member and check that the CPU remains isolated.
Signed-off-by Guopeng Zhang <[email protected]>--- .../selftests/cgroup/test_cpuset_prs.sh | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+)tools/testing/selftests/cgroup/test_cpuset_prs.sh +57
-0@@ -1155,6 +1155,63 @@ test_isolated() pause 0.05 } +# +# Select an online CPU isolated from scheduler domains at boot. +# $1: test name used in the skip message +# +get_boot_isolated_cpu() +{ + TEST_NAME=$1 + BOOT_ISOLATED_FILE=/sys/devices/system/cpu/isolated + + [[ -r $BOOT_ISOLATED_FILE ]] || { + echo "$TEST_NAME test SKIPPED: boot isolation state unavailable" + return 1 + } + BOOT_CPUS=$(cat $BOOT_ISOLATED_FILE) + [[ -n "$BOOT_CPUS" ]] || { + echo "$TEST_NAME test SKIPPED: no boot-isolated CPU" + return 1 + } + + BOOT_CPU=$(echo "$BOOT_CPUS" | sed -e 's/[,-].*//') + CPU_ONLINE=/sys/devices/system/cpu/cpu${BOOT_CPU}/online + [[ ! -e $CPU_ONLINE || $(cat $CPU_ONLINE) -eq 1 ]] || { + echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is offline" + return 1 + } +} + +# +# A CPU isolated at boot must stay isolated after it is released by a dynamic +# isolated partition. +# +test_boot_isolated() +{ + TEST_NAME="Boot-isolated CPU partition release" + get_boot_isolated_cpu "$TEST_NAME" || return 0 + echo "Running $TEST_NAME test ..." + + cd $CGROUP2/test + echo member > cpuset.cpus.partition + echo $BOOT_CPU > cpuset.cpus + [[ $(cat cpuset.cpus.effective) = "$BOOT_CPU" ]] || { + echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is unavailable" + echo "" > cpuset.cpus + cd $CGROUP2 + return 0 + } + test_partition isolated + test_partition member + check_isolcpus "." || { + echo "Boot-isolated CPU $BOOT_CPU was lost after partition release" + exit 1 + } + echo "" > cpuset.cpus + cd $CGROUP2 + echo "$TEST_NAME test PASSED." +} + # # Wait for inotify event for the given file and read it # $1: cgroup file to wait for
tools/testing/selftests/cgroup/test_cpuset_prs.sh trap cleanup 0 2 3 6 · line 1288
Quoted 6 lines
tools/testing/selftests/cgroup/test_cpuset_prs.sh +1
-0@@ -1226,5 +1283,6 @@ trap cleanup 0 2 3 6 run_state_test TEST_MATRIX run_remote_state_test REMOTE_TEST_MATRIX test_isolated +test_boot_isolated test_inotify echo "All tests PASSED."
Tejun Heo
maintainerHello,
Quoted 4 lines · Guopeng Zhang
Guopeng Zhang (3):
selftests/cgroup: Drop invalid boot isolation comparison cgroup/cpuset: Preserve boot-isolated CPUs on partition release selftests/cgroup: Add test for preserving boot-isolated CPUs
Applied 1-3 to cgroup/for-7.3-fixes with Waiman's review tags added.
Thanks.
--
tejun