diff mbox series

[v2] cpuset_regression_test: Fix test, if cpuset groups exist already

Message ID 20211124080413.768368-1-lkml@jv-coder.de
State Accepted
Headers show
Series [v2] cpuset_regression_test: Fix test, if cpuset groups exist already | expand

Commit Message

Joerg Vehlow Nov. 24, 2021, 8:04 a.m. UTC
From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

Fix three errors:
 1. read -d is not posix, but not even required,
    because find and read work line-based
 2. Setting cpuset.cpus to an empty string is not allowed.
    -> If there are cpuset groups defined already, we need at least two cpus.
    One is used for the test, the other one is used for existing groups.
 3. Existing cgroup hierarchies were not handled correctly.
    When setting the cpuset.cpus, it must be done first for parent groups,
    otherwise cpu constraints for can be violated.

Fixes: 6950e96eabb2 ("cpuset_regression_test: Allow running, if groups exist")
Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---

Changes to v1:
 - Renamed breadth-first to parent-first
 - find -depth does not really do a depth-first search, so
   I also added cpuset_find_child_first
 - Resetting of backuped cpuset values could fail for nested child
   groups with cpu constraints. This is fixed by also resetting all
   groups to all cpus before restoring the backup 
 - Removed inline comments 


 .../cpuset/cpuset_regression_test.sh          | 86 ++++++++++++++++---
 1 file changed, 74 insertions(+), 12 deletions(-)

Comments

Richard Palethorpe Nov. 24, 2021, 8:24 a.m. UTC | #1
Hello Joerg,

Joerg Vehlow <lkml@jv-coder.de> writes:

> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>
> Fix three errors:
>  1. read -d is not posix, but not even required,
>     because find and read work line-based
>  2. Setting cpuset.cpus to an empty string is not allowed.
>     -> If there are cpuset groups defined already, we need at least two cpus.
>     One is used for the test, the other one is used for existing groups.
>  3. Existing cgroup hierarchies were not handled correctly.
>     When setting the cpuset.cpus, it must be done first for parent groups,
>     otherwise cpu constraints for can be violated.
>
> Fixes: 6950e96eabb2 ("cpuset_regression_test: Allow running, if groups exist")
> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>

Looks Good!

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
Joerg Vehlow Jan. 10, 2022, 11 a.m. UTC | #2
Hi,


Am 11/24/2021 um 9:24 AM schrieb Richard Palethorpe:
> Hello Joerg,
> 
> Joerg Vehlow <lkml@jv-coder.de> writes:
> 
>> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>>
>> Fix three errors:
>>   1. read -d is not posix, but not even required,
>>      because find and read work line-based
>>   2. Setting cpuset.cpus to an empty string is not allowed.
>>      -> If there are cpuset groups defined already, we need at least two cpus.
>>      One is used for the test, the other one is used for existing groups.
>>   3. Existing cgroup hierarchies were not handled correctly.
>>      When setting the cpuset.cpus, it must be done first for parent groups,
>>      otherwise cpu constraints for can be violated.
>>
>> Fixes: 6950e96eabb2 ("cpuset_regression_test: Allow running, if groups exist")
>> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> 
> Looks Good!
> 
> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
> 

a little ping for merging this

Joerg
Richard Palethorpe Jan. 10, 2022, 11:57 a.m. UTC | #3
Hello,

Joerg Vehlow <lkml@jv-coder.de> writes:

> Hi,
>
>
> Am 11/24/2021 um 9:24 AM schrieb Richard Palethorpe:
>> Hello Joerg,
>> Joerg Vehlow <lkml@jv-coder.de> writes:
>> 
>>> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>>>
>>> Fix three errors:
>>>   1. read -d is not posix, but not even required,
>>>      because find and read work line-based
>>>   2. Setting cpuset.cpus to an empty string is not allowed.
>>>      -> If there are cpuset groups defined already, we need at least two cpus.
>>>      One is used for the test, the other one is used for existing groups.
>>>   3. Existing cgroup hierarchies were not handled correctly.
>>>      When setting the cpuset.cpus, it must be done first for parent groups,
>>>      otherwise cpu constraints for can be violated.
>>>
>>> Fixes: 6950e96eabb2 ("cpuset_regression_test: Allow running, if groups exist")
>>> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>> Looks Good!
>> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
>> 
>
> a little ping for merging this
>
> Joerg

Thanks, pushed!
diff mbox series

Patch

diff --git a/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh b/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
index cc6bfdc64..a6806b7b0 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
@@ -21,32 +21,79 @@  TST_MIN_KVER="3.18"
 LOCAL_MOUNTPOINT="cpuset_test"
 BACKUP_DIRECTORY="cpuset_backup"
 
+cpu_num=
 root_cpuset_dir=
 cpu_exclusive="cpuset.cpu_exclusive"
 cpus="cpuset.cpus"
 old_cpu_exclusive_value=1
 
-# cpuset_backup_and_update <backup_dir> <what> <value>
+# Check if there are cpuset groups
+cpuset_has_groups()
+{
+	find ${root_cpuset_dir} -mindepth 1 -type d -exec echo 1 \; -quit
+}
+
+# cpuset_find_parent_first <what>
+# Do a parent first find of <what>
+cpuset_find_parent_first()
+{
+	local what=$1
+
+	find . -mindepth 2 -name ${what} |
+	    awk '{print length($0) " " $0}' |
+	    sort -n | cut -d " " -f 2-
+}
+
+# cpuset_find_child_first <what>
+# Do a child first find of <what>
+cpuset_find_child_first()
+{
+	local what=$1
+
+	find . -mindepth 2 -name ${what} |
+	    awk '{print length($0) " " $0}' |
+	    sort -nr | cut -d " " -f 2-
+}
+
+# cpuset_backup_and_update <backup_dir> <what>
 # Create backup of the values of a specific file (<what>)
-# in all cpuset groups and set the value to <value>
+# in all cpuset groups and set the value to 1
 # The backup is written to <backup_dir> in the same structure
 # as in the cpuset filesystem
 cpuset_backup_and_update()
 {
 	local backup_dir=$1
 	local what=$2
-	local value=$3
 	local old_dir=$PWD
+	local cpu_max=$((cpu_num - 1))
+	local res
 
 	cd ${root_cpuset_dir}
-	find . -mindepth 2 -name ${what} -print0 |
-	while IFS= read -r -d '' file; do
+
+
+	cpuset_find_parent_first ${what} |
+	while read -r file; do
+		[ "$(cat "${file}")" = "" ] && continue
+
 		mkdir -p "$(dirname "${backup_dir}/${file}")"
 		cat "${file}" > "${backup_dir}/${file}"
-		echo "${value}" > "${file}"
+		echo "0-$cpu_max" > "${file}" || exit 1
+	done
+	if [ $? -ne 0 ]; then
+		cd $old_dir
+		return 1
+	fi
+
+	cpuset_find_child_first ${what} |
+	while read -r file; do
+		[ "$(cat "${file}")" = "" ] && continue
+		echo "1" > "${file}"
 	done
+	res=$?
 
 	cd $old_dir
+
+	return $res
 }
 
 # cpuset_restore <backup_dir> <what>
@@ -57,10 +104,17 @@  cpuset_restore()
 	local backup_dir=$1
 	local what=$2
 	local old_dir=$PWD
+	local cpu_max=$((cpu_num - 1))
 
 	cd ${backup_dir}
-	find . -mindepth 2 -name ${what} -print0 |
-	while IFS= read -r -d '' file; do
+
+	cpuset_find_parent_first ${what} |
+	while read -r file; do
+		echo "0-$cpu_max" > "${root_cpuset_dir}/${file}"
+	done
+
+	cpuset_find_child_first ${what} |
+	while read -r file; do
 		cat "${file}" > "${root_cpuset_dir}/${file}"
 	done
 
@@ -91,10 +145,18 @@  setup()
 		tst_brk TBROK "Both cpuset.cpu_exclusive and cpu_exclusive do not exist"
 	fi
 
-	# Ensure that no group explicitely uses a cpu,
-	# otherwise setting cpuset.cpus for the testgroup will fail
-	mkdir ${BACKUP_DIRECTORY}
-	cpuset_backup_and_update "${PWD}/${BACKUP_DIRECTORY}" ${cpus} ""
+	# Ensure that we can use cpu 0 exclusively
+	if [ "$(cpuset_has_groups)" = "1" ]; then
+		cpu_num=$(tst_getconf _NPROCESSORS_ONLN)
+		if [ $cpu_num -lt 2 ]; then
+			tst_brk TCONF "There are already cpuset groups, so at least two cpus are required."
+		fi
+
+		# Use cpu 1 for all existing cpuset cgroups
+		mkdir ${BACKUP_DIRECTORY}
+		cpuset_backup_and_update "${PWD}/${BACKUP_DIRECTORY}" ${cpus}
+		[ $? -ne 0 ] && tst_brk TBROK "Unable to prepare existing cpuset cgroups"
+	fi
 
 	old_cpu_exclusive_value=$(cat ${root_cpuset_dir}/${cpu_exclusive})
 	if [ "${old_cpu_exclusive_value}" != "1" ];then