cgroup_regression_test.sh: fixed test_5

PATCH Superseded Cristian Marussi 2 participants 3 messages Dec 6, 2018 – Dec 11, 2018

Share & tools

Cristian Marussi

author

2018-12-06 16:41 UTC #0 lore raw

PATCH v2 1/1 cgroup_regression_test.sh: fixed test_5

test_5 checked for possible regressions using a pair of cgroups mounts operations designed to expose a kernel crash; the trigger being the attempt to co-mount and mount the same cgroup subsystem onto two distinct fs hierarchies: the expected failure in the second mount attempt was not properly handled in 2.6.29-rc2 and lead to a kernel crash.
Unfortunately the test assumed that the randomly chosen subsystems were NOT already mounted somewhere when attempting the first co-mount:
this assumption is falsified when userspace is configured to mount all available subsystems at /sysfs on boot (systemd).
So the test was failing straight away during the setup phase:

cgroup_regression_test 5 TFAIL : ltpapicmd.c:188: mount pids and hugetlb failed

Being not trivial to forcibly release and unmount the populated /sysfs cgroups once booted, the script has been reviewed to detect this condition upfront and cope with it dynamically:

- if not already mounted: co-mount + failing mount (as before)
- already mounted: use existing mntpoint + failing co-mount

Since the original fix was on a 2.6.29 kernel the surrounding cgroup code has changed a lot and so the patch was no more trivially 'revertable' for testing purposes: as such this reviewed test script has been verified using a QEMU x86_64 instance running a Kernel 2.6.39 with and without the known fix as detailed in test_5 comments.

Signed-off-by Cristian Marussi <[email protected]>

---
 .../cgroup/cgroup_regression_test.sh          | 55 ++++++++++++++-----
 1 file changed, 40 insertions(+), 15 deletions(-)

testcases/kernel/controllers/cgroup/cgroup_regression_test.sh +40 ~~-15~~

index 30d0dbfbc..6d946ecd8 100755
@@ -253,6 +253,10 @@ test_4()
 #---------------------------------------------------------------------------
 test_5()
 {
+	local mounted
+	local failing
+	local t_mntpoint
+
 	lines=`cat /proc/cgroups | wc -l`
 	if [ $lines -le 2 ]; then
 		tst_resm TCONF "require at least 2 cgroup subsystems"

@@ -262,31 +266,51 @@ test_5()
 	subsys1=`tail -n 1 /proc/cgroups | awk '{ print $1 }'`
 	subsys2=`tail -n 2 /proc/cgroups | head -1 | awk '{ print $1 }'`
 
-	mount -t cgroup -o $subsys1,$subsys2 xxx cgroup/
-	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "mount $subsys1 and $subsys2 failed"
-		failed=1
-		return
+	# Accounting here for the fact that the chosen subsystems could
+	# have been already previously mounted at boot time: in such a
+	# case we must skip the initial co-mount step (which would
+	# fail anyway) and properly re-organize the $t_mntpoint and
+	# $failing params to be used in the following expected-to-fail
+	# mount action. Note that the subsysN name itself will be listed
+	# amongst mounts options.
+	cat /proc/mounts | grep cgroup | grep -q $subsys1 2> /dev/null && mounted=$subsys1
+	[ -z "$mounted" ] && cat /proc/mounts | grep cgroup | grep -q $subsys2 2> /dev/null && mounted=$subsys2
+	if [ -z "$mounted" ]; then
+		t_mntpoint=cgroup
+		failing=$subsys1
+		mount -t cgroup -o $subsys1,$subsys2 xxx $t_mntpoint/
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "mount $subsys1 and $subsys2 failed"
+			failed=1
+			return
+		fi
+	else
+		# Use the pre-esistent mountpoint as $t_mntpoint and use a
+		# co-mount with $failing: this way the 2nd mount will
+		# also fail (as expected) in this 'mirrored' configuration.
+		t_mntpoint=$(cat /proc/mounts | grep cgroup | grep $mounted | awk '{ print $2 }')
+		failing=$subsys1,$subsys2
 	fi
 
-	# This 2nd mount should fail
-	mount -t cgroup -o $subsys1 xxx cgroup/ 2> /dev/null
+	# This 2nd mount has been properly configured to fail
+	mount -t cgroup -o $failing xxx $t_mntpoint/ 2> /dev/null
 	if [ $? -eq 0 ]; then
-		tst_resm TFAIL "mount $subsys1 should fail"
-		umount cgroup/
+		tst_resm TFAIL "mount $failing should fail"
+		# Do NOT unmount pre-existent mountpoints...
+		[ -z "$mounted" ] && umount $t_mntpoint
 		failed=1
 		return
 	fi
 
-	mkdir cgroup/0
+	mkdir $t_mntpoint/0
 	# Otherwise we can't attach task
 	if [ "$subsys1" = cpuset -o "$subsys2" = cpuset ]; then
-		echo 0 > cgroup/0/cpuset.cpus 2> /dev/null
-		echo 0 > cgroup/0/cpuset.mems 2> /dev/null
+		echo 0 > $t_mntpoint/0/cpuset.cpus 2> /dev/null
+		echo 0 > $t_mntpoint/0/cpuset.mems 2> /dev/null
 	fi
 
 	sleep 100 &
-	echo $! > cgroup/0/tasks
+	echo $! > $t_mntpoint/0/tasks
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then

@@ -296,8 +320,9 @@ test_5()
 	# clean up
 	/bin/kill -SIGTERM $! > /dev/null
 	wait $!
-	rmdir cgroup/0
-	umount cgroup/
+	rmdir $t_mntpoint/0
+	# Do NOT unmount pre-existent mountpoints...
+	[ -z "$mounted" ] && umount $t_mntpoint
 }
 
 #---------------------------------------------------------------------------

Signature · 1 line

2.17.1

Petr Vorel

2018-12-11 11:16 UTC #1 lore raw

PATCH v2 1/1 cgroup_regression_test.sh: fixed test_5

Hi Cristian,

Quoted 18 lines

test_5 checked for possible regressions using a pair of cgroups mounts operations designed to expose a kernel crash; the trigger being the attempt to co-mount and mount the same cgroup subsystem onto two distinct fs hierarchies: the expected failure in the second mount attempt was not properly handled in 2.6.29-rc2 and lead to a kernel crash.
Unfortunately the test assumed that the randomly chosen subsystems were NOT already mounted somewhere when attempting the first co-mount:
this assumption is falsified when userspace is configured to mount all available subsystems at /sysfs on boot (systemd).
So the test was failing straight away during the setup phase:
cgroup_regression_test 5 TFAIL : ltpapicmd.c:188: mount pids and hugetlb failed
Being not trivial to forcibly release and unmount the populated /sysfs cgroups once booted, the script has been reviewed to detect this condition upfront and cope with it dynamically:

Quoted 8 lines

- if not already mounted: co-mount + failing mount (as before)
- already mounted: use existing mntpoint + failing co-mount
Since the original fix was on a 2.6.29 kernel the surrounding cgroup code has changed a lot and so the patch was no more trivially 'revertable' for testing purposes: as such this reviewed test script has been verified using a QEMU x86_64 instance running a Kernel 2.6.39 with and without the known fix as detailed in test_5 comments.

Thanks for your testing.

Quoted 1 line

Signed-off-by Cristian Marussi <[email protected]>

Acked-by Petr Vorel <[email protected]>

Thanks for your patch! Two tiny details bellow.
...

testcases/kernel/controllers/cgroup/cgroup_regression_test.sh test_4() · line 258

Quoted 8 lines

+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
testcases/kernel/controllers/cgroup/cgroup_regression_test.sh +3 -0
@@ -253,6 +253,10 @@ test_4()
 #---------------------------------------------------------------------------
 test_5()
 {
+	local mounted
+	local failing
+	local t_mntpoint

I'd use just mntpoint. But that's nitpicking, up to you.

testcases/kernel/controllers/cgroup/cgroup_regression_test.sh test_5() · line 277

Quoted 2 lines

testcases/kernel/controllers/cgroup/cgroup_regression_test.sh +2 -0

+	cat /proc/mounts | grep cgroup | grep -q $subsys1 2> /dev/null && mounted=$subsys1
+	[ -z "$mounted" ] && cat /proc/mounts | grep cgroup | grep -q $subsys2 2> /dev/null && mounted=$subsys2

Is stderr redirection really needed?

Kind regards,
Petr

Cristian Marussi

author

2018-12-11 12:38 UTC #2 lore raw

PATCH v2 1/1 cgroup_regression_test.sh: fixed test_5

Quoted 28 lines · Petr Vorel

Hi Cristian,
Quoted 18 lines
test_5 checked for possible regressions using a pair of cgroups mounts operations designed to expose a kernel crash; the trigger being the attempt to co-mount and mount the same cgroup subsystem onto two distinct fs hierarchies: the expected failure in the second mount attempt was not properly handled in 2.6.29-rc2 and lead to a kernel crash.
Unfortunately the test assumed that the randomly chosen subsystems were NOT already mounted somewhere when attempting the first co-mount:
this assumption is falsified when userspace is configured to mount all available subsystems at /sysfs on boot (systemd).
So the test was failing straight away during the setup phase:
cgroup_regression_test 5 TFAIL : ltpapicmd.c:188: mount pids and hugetlb failed
Being not trivial to forcibly release and unmount the populated /sysfs cgroups once booted, the script has been reviewed to detect this condition upfront and cope with it dynamically:
Quoted 8 lines
- if not already mounted: co-mount + failing mount (as before)
- already mounted: use existing mntpoint + failing co-mount
Since the original fix was on a 2.6.29 kernel the surrounding cgroup code has changed a lot and so the patch was no more trivially 'revertable' for testing purposes: as such this reviewed test script has been verified using a QEMU x86_64 instance running a Kernel 2.6.39 with and without the known fix as detailed in test_5 comments.

Quoted 6 lines

Thanks for your testing.
Quoted 1 line
Signed-off-by Cristian Marussi <[email protected]>
Acked-by Petr Vorel <[email protected]>
Thanks for your patch! Two tiny details bellow.

Thanks for you review.

Quoted 10 lines

...
testcases/kernel/controllers/cgroup/cgroup_regression_test.sh test_4() · line 258
Quoted 8 lines
+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
testcases/kernel/controllers/cgroup/cgroup_regression_test.sh +3 -0
@@ -253,6 +253,10 @@ test_4()
 #---------------------------------------------------------------------------
 test_5()
 {
+	local mounted
+	local failing
+	local t_mntpoint
I'd use just mntpoint. But that's nitpicking, up to you.

I'll do.

Quoted 4 lines

testcases/kernel/controllers/cgroup/cgroup_regression_test.sh test_5() · line 277
Quoted 3 lines
testcases/kernel/controllers/cgroup/cgroup_regression_test.sh +2 -0
+	cat /proc/mounts | grep cgroup | grep -q $subsys1 2> /dev/null && mounted=$subsys1
+	[ -z "$mounted" ] && cat /proc/mounts | grep cgroup | grep -q $subsys2 2> /dev/null && mounted=$subsys2
Is stderr redirection really needed?

I added it in v2 probably for some paranoid reasons I cannot even recall now...:D..so no it's not really needed. I'm removing it.

v3 is oncoming..

Quoted 4 lines

Kind regards,
Petr

Thanks

Regards

Cristian