[LTP] [PATCH v2 0/4] cgroup tests newlib-porting

[LTP] [PATCH v2 0/4] cgroup tests newlib-porting

[Cristian Marussi]

This series converts cgroup/ regression tests to LTP newlib and performs
a general cleanup as follows:

- remove bashism from all tests' scripts
- mostly remove usage of global vars
- convert to SPDX headers
- expose a controllers' common /proc/mounts parsing function
- rename tests' helpers prefixing them with a test-specific tag to avoid
  name clashes on install: I have not renamed the tests' helpers using
  more specific meaningful name, since all of them performs really
  trivial ops needed and meaningful in the context of the regression-test
  subcase they serve; so I thought it would have been better to keep their
  names as they are now, bound to the subcase they help.

Cristian Marussi (4):
  [LTP] cgroup_regression_test.sh ported to newlib
  [LTP] cgroup_regression_test.sh cleanup
  [LTP] cgroup_regression_test.sh: added helper
  [LTP] cgroup: helpers various fixes

[LTP] [PATCH v2 1/4] cgroup_regression_test.sh ported to newlib

[Cristian Marussi]

Ported to newlib and removed most global vars usage.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 .../cgroup/cgroup_regression_test.sh          | 217 +++++++++---------
 1 file changed, 110 insertions(+), 107 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
index 6cfc63866..1b4dfb45e 100755
--- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
@@ -22,71 +22,83 @@
 ##                                                                            ##
 ################################################################################
 
-cd $LTPROOT/testcases/bin
+TST_TESTFUNC=do_test
+TST_SETUP=do_setup
+TST_CLEANUP=do_cleanup
+TST_CNT=10
+TST_NEEDS_ROOT=1
+TST_NEEDS_CMDS="dmesg mountpoint mount umount cat kill find mkdir rmdir grep"
 
-export TCID="cgroup_regression_test"
-export TST_TOTAL=10
-export TST_COUNT=1
+. tst_test.sh
 
-failed=0
+do_setup()
+{
+	cd $LTPROOT/testcases/bin
 
-if tst_kvcmp -lt "2.6.29"; then
-	tst_brkm TCONF ignored "test must be run with kernel 2.6.29 or newer"
-	exit 32
-fi
+	mkdir cgroup/
 
-if [ ! -f /proc/cgroups ]; then
-	tst_brkm TCONF ignored "Kernel does not support for control groups; skipping testcases";
-	exit 32
-fi
+	if tst_kvcmp -lt "2.6.29"; then
+		tst_brk TCONF ignored "test must be run with kernel 2.6.29 or newer"
+	fi
 
-if [ "x$(id -ru)" != x0 ]; then
-	tst_brkm TCONF ignored "Test must be run as root"
-	exit 32
-fi
+	if [ ! -f /proc/cgroups ]; then
+		tst_brk TCONF ignored "Kernel does not support for control groups; skipping testcases";
+	fi
 
-dmesg -c > /dev/null
-nr_bug=`dmesg | grep -c "kernel BUG"`
-nr_null=`dmesg | grep -c "kernel NULL pointer dereference"`
-nr_warning=`dmesg | grep -c "^WARNING"`
-nr_lockdep=`dmesg | grep -c "possible recursive locking detected"`
+	dmesg -c > /dev/null
+	NR_BUG=`dmesg | grep -c "kernel BUG"`
+	NR_NULL=`dmesg | grep -c "kernel NULL pointer dereference"`
+	NR_WARNING=`dmesg | grep -c "^WARNING"`
+	NR_LOCKDEP=`dmesg | grep -c "possible recursive locking detected"`
+}
+
+do_cleanup()
+{
+	cd $LTPROOT/testcases/bin
+
+	if mountpoint -q cgroup/
+	then
+		find cgroup/ -maxdepth 1 -depth -exec rmdir {} +
+		umount cgroup/
+		rmdir cgroup/
+	fi
+}
 
 # check_kernel_bug - check if some kind of kernel bug happened
 check_kernel_bug()
 {
-	new_bug=`dmesg | grep -c "kernel BUG"`
-	new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
-	new_warning=`dmesg | grep -c "^WARNING"`
-	new_lockdep=`dmesg | grep -c "possible recursive locking detected"`
+	local new_bug=`dmesg | grep -c "kernel BUG"`
+	local new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
+	local new_warning=`dmesg | grep -c "^WARNING"`
+	local new_lockdep=`dmesg | grep -c "possible recursive locking detected"`
 
 	# no kernel bug is detected
-	if [ $new_bug -eq $nr_bug -a $new_warning -eq $nr_warning -a \
-	     $new_null -eq $nr_null -a $new_lockdep -eq $nr_lockdep ]; then
+	if [ $new_bug -eq $NR_BUG -a $new_warning -eq $NR_WARNING -a \
+	     $new_null -eq $NR_NULL -a $new_lockdep -eq $NR_LOCKDEP ]; then
 		return 1
 	fi
 
 	# some kernel bug is detected
-	if [ $new_bug -gt $nr_bug ]; then
-		tst_resm TFAIL "kernel BUG was detected!"
+	if [ $new_bug -gt $NR_BUG ]; then
+		tst_res TFAIL "kernel BUG was detected!"
 	fi
-	if [ $new_warning -gt $nr_warning ]; then
-		tst_resm TFAIL "kernel WARNING was detected!"
+	if [ $new_warning -gt $NR_WARNING ]; then
+		tst_res TFAIL "kernel WARNING was detected!"
 	fi
-	if [ $new_null -gt $nr_null ]; then
-		tst_resm TFAIL "kernel NULL pointer dereference!"
+	if [ $new_null -gt $NR_NULL ]; then
+		tst_res TFAIL "kernel NULL pointer dereference!"
 	fi
-	if [ $new_lockdep -gt $nr_lockdep ]; then
-		tst_resm TFAIL "kernel lockdep warning was detected!"
+	if [ $new_lockdep -gt $NR_LOCKDEP ]; then
+		tst_res TFAIL "kernel lockdep warning was detected!"
 	fi
 
-	nr_bug=$new_bug
-	nr_null=$new_null
-	nr_warning=$new_warning
-	nr_lockdep=$new_lockdep
+	NR_BUG=$new_bug
+	NR_NULL=$new_null
+	NR_WARNING=$new_warning
+	NR_LOCKDEP=$new_lockdep
 
 	echo "check_kernel_bug found something!"
 	dmesg
-	failed=1
 	return 0
 }
 
@@ -107,8 +119,7 @@ test_1()
 
 	mount -t cgroup -o none,name=foo cgroup cgroup/
 	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "failed to mount cgroup filesystem"
-		failed=1
+		tst_res TFAIL "failed to mount cgroup filesystem"
 		/bin/kill -SIGTERM $!
 		return
 	fi
@@ -116,7 +127,7 @@ test_1()
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 
 	/bin/kill -SIGTERM $!
@@ -132,11 +143,13 @@ test_1()
 #---------------------------------------------------------------------------
 test_2()
 {
+	local val1
+	local val2
+
 	mount -t cgroup -o none,name=foo cgroup cgroup/
 	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "Failed to mount cgroup filesystem"
-		failed=1
-		return 1
+		tst_res TFAIL "Failed to mount cgroup filesystem"
+		return
 	fi
 
 	echo 0 > cgroup/notify_on_release
@@ -148,16 +161,15 @@ test_2()
 	val2=`cat cgroup/1/notify_on_release`
 
 	if [ $val1 -ne 0 -o $val2 -ne 1 ]; then
-		tst_resm TFAIL "wrong notify_on_release value"
-		failed=1
+		tst_res TFAIL "wrong notify_on_release value"
 	else
-		tst_resm TPASS "notify_on_release is inherited"
+		tst_res TPASS "notify_on_release is inherited"
 	fi
 
 	rmdir cgroup/0 cgroup/1
 	umount cgroup/
 
-	return $failed
+	return
 }
 
 #---------------------------------------------------------------------------
@@ -173,14 +185,14 @@ test_3()
 	local cpu_subsys_path
 
 	if [ ! -e /proc/sched_debug ]; then
-		tst_resm TCONF "CONFIG_SCHED_DEBUG is not enabled"
+		tst_res TCONF "CONFIG_SCHED_DEBUG is not enabled"
 		return
 	fi
 
 	if grep -q -w "cpu" /proc/cgroups ; then
 		cpu_subsys_path=$(grep -w cpu /proc/mounts | awk '{ print $2 }')
 	else
-		tst_resm TCONF "CONFIG_CGROUP_SCHED is not enabled"
+		tst_res TCONF "CONFIG_CGROUP_SCHED is not enabled"
 		return
 	fi
 
@@ -188,8 +200,7 @@ test_3()
 	if [ -z "$cpu_subsys_path" ]; then
 		mount -t cgroup -o cpu xxx cgroup/
 		if [ $? -ne 0 ]; then
-			tst_resm TFAIL "Failed to mount cpu subsys"
-			failed=1
+			tst_res TFAIL "Failed to mount cpu subsys"
 			return
 		fi
 		cpu_subsys_path=cgroup
@@ -207,7 +218,7 @@ test_3()
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 
 	rmdir $cpu_subsys_path/* 2> /dev/null
@@ -223,15 +234,17 @@ test_3()
 #---------------------------------------------------------------------------
 test_4()
 {
+	local lines
+
 	if [ ! -e /proc/lockdep ]; then
-		tst_resm TCONF "CONFIG_LOCKDEP is not enabled"
+		tst_res TCONF "CONFIG_LOCKDEP is not enabled"
 		return
 	fi
 
 	# MAX_LOCKDEP_SUBCLASSES is 8, so number of subsys should be > 8
 	lines=`cat /proc/cgroups | wc -l`
 	if [ $lines -le 9 ]; then
-		tst_resm TCONF "require more than 8 cgroup subsystems"
+		tst_res TCONF "require more than 8 cgroup subsystems"
 		return
 	fi
 
@@ -242,11 +255,10 @@ test_4()
 
 	dmesg | grep -q "MAX_LOCKDEP_SUBCLASSES too low"
 	if [ $? -eq 0 ]; then
-		tst_resm TFAIL "lockdep BUG was found"
-		failed=1
+		tst_res TFAIL "lockdep BUG was found"
 		return
 	else
-		tst_resm TPASS "no lockdep BUG was found"
+		tst_res TPASS "no lockdep BUG was found"
 	fi
 }
 
@@ -264,14 +276,14 @@ test_5()
 	local failing
 	local mntpoint
 
-	lines=`cat /proc/cgroups | wc -l`
+	local lines=`cat /proc/cgroups | wc -l`
 	if [ $lines -le 2 ]; then
-		tst_resm TCONF "require at least 2 cgroup subsystems"
+		tst_res TCONF "require at least 2 cgroup subsystems"
 		return
 	fi
 
-	subsys1=`tail -n 1 /proc/cgroups | awk '{ print $1 }'`
-	subsys2=`tail -n 2 /proc/cgroups | head -1 | awk '{ print $1 }'`
+	local subsys1=`tail -n 1 /proc/cgroups | awk '{ print $1 }'`
+	local subsys2=`tail -n 2 /proc/cgroups | head -1 | awk '{ print $1 }'`
 
 	# Accounting here for the fact that the chosen subsystems could
 	# have been already previously mounted at boot time: in such a
@@ -287,8 +299,7 @@ test_5()
 		failing=$subsys1
 		mount -t cgroup -o $subsys1,$subsys2 xxx $mntpoint/
 		if [ $? -ne 0 ]; then
-			tst_resm TFAIL "mount $subsys1 and $subsys2 failed"
-			failed=1
+			tst_res TFAIL "mount $subsys1 and $subsys2 failed"
 			return
 		fi
 	else
@@ -302,10 +313,9 @@ test_5()
 	# This 2nd mount has been properly configured to fail
 	mount -t cgroup -o $failing xxx $mntpoint/ 2> /dev/null
 	if [ $? -eq 0 ]; then
-		tst_resm TFAIL "mount $failing should fail"
+		tst_res TFAIL "mount $failing should fail"
 		# Do NOT unmount pre-existent mountpoints...
-		[ -z "$mounted" ] && umount $mntpoint
-		failed=1
+		[ -z "$mounted" ] && umount $mntpoint/
 		return
 	fi
 
@@ -321,7 +331,7 @@ test_5()
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 
 	# clean up
@@ -329,7 +339,7 @@ test_5()
 	wait $!
 	rmdir $mntpoint/0
 	# Do NOT unmount pre-existent mountpoints...
-	[ -z "$mounted" ] && umount $mntpoint
+	[ -z "$mounted" ] && umount $mntpoint/
 }
 
 #---------------------------------------------------------------------------
@@ -342,15 +352,15 @@ test_6()
 {
 	grep -q -w "ns" /proc/cgroups
 	if [ $? -ne 0 ]; then
-		tst_resm TCONF "CONFIG_CGROUP_NS"
+		tst_res TCONF "CONFIG_CGROUP_NS"
 		return
 	fi
 
 	# run the test for 30 secs
 	./test_6_1.sh &
-	pid1=$!
+	local pid1=$!
 	./test_6_2 &
-	pid2=$!
+	local pid2=$!
 
 	sleep 30
 	/bin/kill -SIGUSR1 $pid1
@@ -360,7 +370,7 @@ test_6()
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 
 	# clean up
@@ -379,12 +389,12 @@ test_6()
 #---------------------------------------------------------------------------
 test_7_1()
 {
-	subsys_path=$(grep -w $subsys /proc/mounts | cut -d ' ' -f 2)
+	local subsys=$1
+	local subsys_path=$(grep -w $subsys /proc/mounts | cut -d ' ' -f 2)
 	if [ -z "$subsys_path" ]; then
 		mount -t cgroup -o $subsys xxx cgroup/
 		if [ $? -ne 0 ]; then
-			tst_resm TFAIL "failed to mount $subsys"
-			failed=1
+			tst_res TFAIL "failed to mount $subsys"
 			return
 		fi
 		subsys_path=cgroup
@@ -407,10 +417,11 @@ test_7_1()
 
 test_7_2()
 {
+	local subsys=$1
+
 	mount -t cgroup -o none,name=foo cgroup cgroup/
 	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "failed to mount cgroup"
-		failed=1
+		tst_res TFAIL "failed to mount cgroup"
 		return
 	fi
 
@@ -441,19 +452,19 @@ test_7_2()
 
 test_7()
 {
-	lines=`cat /proc/cgroups | wc -l`
+	local lines=`cat /proc/cgroups | wc -l`
 	if [ $lines -le 2 ]; then
-		tst_resm TCONF "require at least 2 cgroup subsystems"
+		tst_res TCONF "require at least 2 cgroup subsystems"
 		slt_result $SLT_Untested
 		return
 	fi
 
-	subsys=`tail -n 1 /proc/cgroups | awk '{ print $1 }'`
+	local subsys=`tail -n 1 /proc/cgroups | awk '{ print $1 }'`
 
 	# remount to add new subsystems to the hierarchy
-	i=1
+	local i=1
 	while [ $i -le 2 ] ; do
-		test_7_$i
+		test_7_$i $subsys
 		if [ $? -ne 0 ]; then
 			return
 		fi
@@ -465,7 +476,7 @@ test_7()
 		: $(( i += 1 ))
 	done
 
-	tst_resm TPASS "no kernel bug was found"
+	tst_res TPASS "no kernel bug was found"
 }
 
 #---------------------------------------------------------------------------
@@ -478,22 +489,20 @@ test_8()
 {
 	mount -t cgroup -o none,name=foo cgroup cgroup/
 	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "failed to mount cgroup filesystem"
-		failed=1
+		tst_res TFAIL "failed to mount cgroup filesystem"
 		return
 	fi
 
 	./getdelays -C cgroup/tasks > /dev/null 2>&1
 	if [ $? -eq 0 ]; then
-		tst_resm TFAIL "should have failed to get cgroupstat of tasks file"
+		tst_res TFAIL "should have failed to get cgroupstat of tasks file"
 		umount cgroup/
-		failed=1
 		return
 	fi
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 
 	umount cgroup/
@@ -510,9 +519,9 @@ test_8()
 test_9()
 {
 	./test_9_1.sh &
-	pid1=$!
+	local pid1=$!
 	./test_9_2.sh &
-	pid2=$!
+	local pid2=$!
 
 	sleep 30
 	/bin/kill -SIGUSR1 $pid1 $pid2
@@ -523,7 +532,7 @@ test_9()
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel warning was found"
+		tst_res TPASS "no kernel warning was found"
 	fi
 }
 
@@ -537,9 +546,9 @@ test_9()
 test_10()
 {
 	./test_10_1.sh &
-	pid1=$!
+	local pid1=$!
 	./test_10_2.sh &
-	pid2=$!
+	local pid2=$!
 
 	sleep 30
 	/bin/kill -SIGUSR1 $pid1 $pid2
@@ -552,21 +561,15 @@ test_10()
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel warning was found"
+		tst_res TPASS "no kernel warning was found"
 	fi
 }
 
-# main
-
-mkdir cgroup/
-
-for ((cur = 1; cur <= $TST_TOTAL; cur++))
+do_test()
 {
-	export TST_COUNT=$cur
+	local cur=$1
 
 	test_$cur
 }
 
-find cgroup/ -maxdepth 1 -depth -exec rmdir {} +
-
-exit $failed
+tst_run
-- 
2.17.1

[LTP] [PATCH v2 1/4] cgroup_regression_test.sh ported to newlib

[Petr Vorel]

Hi Cristian,

...
> --- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
>  {
>  	grep -q -w "ns" /proc/cgroups
>  	if [ $? -ne 0 ]; then
> -		tst_resm TCONF "CONFIG_CGROUP_NS"
> +		tst_res TCONF "CONFIG_CGROUP_NS"
>  		return
>  	fi

This could be just

grep -q -w "ns" /proc/cgroups || tst_res TCONF "CONFIG_CGROUP_NS is not enabled"

But CONFIG_CGROUP_NS was removed in a77aea92010a ("cgroup: remove the ns_cgroup")
in v3.0. We don't have TST_MAX_KVER, so we cannot use TST_MAX_KVER="3.0", but
maybe compare kernel version with tst_kvcmp (see doc) and adjust warning for new kernels.

Kind regards,
Petr

[LTP] [PATCH v2 1/4] cgroup_regression_test.sh ported to newlib

[Cristian Marussi]

Hi

On 21/12/2018 14:25, Petr Vorel wrote:
> Hi Cristian,
> 
> ...
>> --- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
>>  {
>>  	grep -q -w "ns" /proc/cgroups
>>  	if [ $? -ne 0 ]; then
>> -		tst_resm TCONF "CONFIG_CGROUP_NS"
>> +		tst_res TCONF "CONFIG_CGROUP_NS"
>>  		return
>>  	fi
> 
> This could be just
> 
> grep -q -w "ns" /proc/cgroups || tst_res TCONF "CONFIG_CGROUP_NS is not enabled"
> 
> But CONFIG_CGROUP_NS was removed in a77aea92010a ("cgroup: remove the ns_cgroup")
> in v3.0. We don't have TST_MAX_KVER, so we cannot use TST_MAX_KVER="3.0", but
> maybe compare kernel version with tst_kvcmp (see doc) and adjust warning for new kernels.

Fine I'll do. In fact I could see it working properly only when testing on QEMU
2.6.39.

Regards

Cristian
> 
> Kind regards,
> Petr
> 

[LTP] [PATCH v2 1/4] cgroup_regression_test.sh ported to newlib

[Cristian Marussi]

Hi

On 21/12/2018 15:20, Cristian Marussi wrote:
> Hi
> 
> On 21/12/2018 14:25, Petr Vorel wrote:
>> Hi Cristian,
>>
>> ...
>>> --- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
>>>  {
>>>  	grep -q -w "ns" /proc/cgroups
>>>  	if [ $? -ne 0 ]; then
>>> -		tst_resm TCONF "CONFIG_CGROUP_NS"
>>> +		tst_res TCONF "CONFIG_CGROUP_NS"
>>>  		return
>>>  	fi
>>
>> This could be just
>>
>> grep -q -w "ns" /proc/cgroups || tst_res TCONF "CONFIG_CGROUP_NS is not enabled"
>>
>> But CONFIG_CGROUP_NS was removed in a77aea92010a ("cgroup: remove the ns_cgroup")
>> in v3.0. We don't have TST_MAX_KVER, so we cannot use TST_MAX_KVER="3.0", but
>> maybe compare kernel version with tst_kvcmp (see doc) and adjust warning for new kernels.
> 
> Fine I'll do. In fact I could see it working properly only when testing on QEMU
> 2.6.39.

Done with test_kvcmp BUT regarding the grep --q ... || tst_res, it turns out it
needs anyway a return afterwards to end the testcase function ...so I've left
the if block (even if I could have appended the return after the || tst_res)

Regards

Cristian
> 
> Regards
> 
> Cristian
>>
>> Kind regards,
>> Petr
>>
> 
> 

[LTP] [PATCH v2 1/4] cgroup_regression_test.sh ported to newlib

[Petr Vorel]

Hi Cristian,

...
> +++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
...
> -cd $LTPROOT/testcases/bin
You left this even after second commit. It's not needed.

> +TST_TESTFUNC=do_test
> +TST_SETUP=do_setup
> +TST_CLEANUP=do_cleanup
> +TST_CNT=10
> +TST_NEEDS_ROOT=1
> +TST_NEEDS_CMDS="dmesg mountpoint mount umount cat kill find mkdir rmdir grep"
Test needs also using temporary directory (as it creates directories):
TST_NEEDS_TMPDIR=1


> -export TCID="cgroup_regression_test"
> -export TST_TOTAL=10
> -export TST_COUNT=1
> +. tst_test.sh

> -failed=0
> +do_setup()
> +{
> +	cd $LTPROOT/testcases/bin
And here.

...
> +do_cleanup()
> +{
> +	cd $LTPROOT/testcases/bin
One more time.

...
> -for ((cur = 1; cur <= $TST_TOTAL; cur++))
> +do_test()
>  {
> -	export TST_COUNT=$cur
> +	local cur=$1

>  	test_$cur
>  }

do_test() function is not needed at all, looping can be done automatically:
Remove underscore from test functions (s/test_([0-9]+)/test$1/)
and change setup: TST_TESTFUNC=test

See
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#user-content-2-3-1-basic-test-interface


Kind regards,
Petr

[LTP] [PATCH v2 1/4] cgroup_regression_test.sh ported to newlib

[Cristian Marussi]

Hi Petr

On 21/12/2018 14:35, Petr Vorel wrote:
> Hi Cristian,
> 
> ...
>> +++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
> ...
>> -cd $LTPROOT/testcases/bin
> You left this even after second commit. It's not needed.

because I misunderstood your indication in the previous mail

>> * Remove absolute (/bin/kill) and relative paths and cd into
$LTPROOT/testcases/bin

getting it like "remove A and remove B AND then CD into..." o.O

my bad ...:D I'll fix it

> 
>> +TST_TESTFUNC=do_test
>> +TST_SETUP=do_setup
>> +TST_CLEANUP=do_cleanup
>> +TST_CNT=10
>> +TST_NEEDS_ROOT=1
>> +TST_NEEDS_CMDS="dmesg mountpoint mount umount cat kill find mkdir rmdir grep"
> Test needs also using temporary directory (as it creates directories):
> TST_NEEDS_TMPDIR=1

Fine. I missed that.

> 
> 
>> -export TCID="cgroup_regression_test"
>> -export TST_TOTAL=10
>> -export TST_COUNT=1
>> +. tst_test.sh
> 
>> -failed=0
>> +do_setup()
>> +{
>> +	cd $LTPROOT/testcases/bin
> And here.
> 
o.O

> ...
>> +do_cleanup()
>> +{
>> +	cd $LTPROOT/testcases/bin
> One more time.
> 
o.O

> ...
>> -for ((cur = 1; cur <= $TST_TOTAL; cur++))
>> +do_test()
>>  {
>> -	export TST_COUNT=$cur
>> +	local cur=$1
> 
>>  	test_$cur
>>  }
> 
> do_test() function is not needed at all, looping can be done automatically:
> Remove underscore from test functions (s/test_([0-9]+)/test$1/)
> and change setup: TST_TESTFUNC=test
> 
> See
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#user-content-2-3-1-basic-test-interface
> 
Sure.

> 
> Kind regards,
> Petr
> 

Regards

Cristian

[LTP] [PATCH v2 2/4] cgroup_regression_test.sh cleanup

[Cristian Marussi]

Cleanups:
 - removed absolute/relative commands invocations
 - avoid bashism
  + removed bash shebang
  + giving up 'kill' bash-builtin: this needs redefining
    used sigspec strings

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 .../cgroup/cgroup_regression_test.sh          | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
index 1b4dfb45e..4e702f2f9 100755
--- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
@@ -1,4 +1,4 @@
-#! /bin/bash
+#! /bin/sh
 
 ################################################################################
 ##                                                                            ##
@@ -114,13 +114,13 @@ check_kernel_bug()
 #---------------------------------------------------------------------------
 test_1()
 {
-	./fork_processes &
+	fork_processes &
 	sleep 1
 
 	mount -t cgroup -o none,name=foo cgroup cgroup/
 	if [ $? -ne 0 ]; then
 		tst_res TFAIL "failed to mount cgroup filesystem"
-		/bin/kill -SIGTERM $!
+		kill -TERM $!
 		return
 	fi
 	cat cgroup/tasks > /dev/null
@@ -130,7 +130,7 @@ test_1()
 		tst_res TPASS "no kernel bug was found"
 	fi
 
-	/bin/kill -SIGTERM $!
+	kill -TERM $!
 	wait $!
 	umount cgroup/
 }
@@ -212,7 +212,7 @@ test_3()
 	pid2=$!
 
 	sleep 30
-	/bin/kill -SIGUSR1 $pid1 $pid2
+	kill -USR1 $pid1 $pid2
 	wait $pid1
 	wait $pid2
 
@@ -335,7 +335,7 @@ test_5()
 	fi
 
 	# clean up
-	/bin/kill -SIGTERM $! > /dev/null
+	kill -TERM $! > /dev/null
 	wait $!
 	rmdir $mntpoint/0
 	# Do NOT unmount pre-existent mountpoints...
@@ -363,8 +363,8 @@ test_6()
 	local pid2=$!
 
 	sleep 30
-	/bin/kill -SIGUSR1 $pid1
-	/bin/kill -SIGTERM $pid2
+	kill -USR1 $pid1
+	kill -TERM $pid2
 	wait $pid1
 	wait $pid2
 
@@ -409,7 +409,7 @@ test_7_1()
 
 	if [ "$subsys_path" = "cgroup" ]; then
 		mount -t cgroup -o remount xxx cgroup/ 2> /dev/null
-		/bin/kill -SIGTERM $!
+		kill -TERM $!
 		wait $!
 		umount cgroup/
 	fi
@@ -432,7 +432,7 @@ test_7_2()
 	# remount with some subsystems removed
 	# since 2.6.28, this remount will fail
 	mount -t cgroup -o remount,$subsys xxx cgroup/ 2> /dev/null
-	/bin/kill -SIGTERM $!
+	kill -TERM $!
 	wait $!
 	umount cgroup/
 
@@ -493,7 +493,7 @@ test_8()
 		return
 	fi
 
-	./getdelays -C cgroup/tasks > /dev/null 2>&1
+	getdelays -C cgroup/tasks > /dev/null 2>&1
 	if [ $? -eq 0 ]; then
 		tst_res TFAIL "should have failed to get cgroupstat of tasks file"
 		umount cgroup/
@@ -524,7 +524,7 @@ test_9()
 	local pid2=$!
 
 	sleep 30
-	/bin/kill -SIGUSR1 $pid1 $pid2
+	kill -USR1 $pid1 $pid2
 	wait $pid1
 	wait $pid2
 
@@ -551,7 +551,7 @@ test_10()
 	local pid2=$!
 
 	sleep 30
-	/bin/kill -SIGUSR1 $pid1 $pid2
+	kill -USR1 $pid1 $pid2
 	wait $pid1
 	wait $pid2
 
-- 
2.17.1

[LTP] [PATCH v2 3/4] cgroup_regression_test.sh: added helper

[Cristian Marussi]

A common function 'get_cgroup_mountpoint' is added to help
parse /proc/mounts in search of specific cgroup subsystems
mountpoints. It is added as part of a common cgroup_lib.sh.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 .../cgroup/cgroup_regression_test.sh          | 17 +++++++---
 testcases/kernel/controllers/cgroup_lib.sh    | 33 +++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)
 create mode 100644 testcases/kernel/controllers/cgroup_lib.sh

diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
index 4e702f2f9..a66ed71e1 100755
--- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
@@ -30,6 +30,7 @@ TST_NEEDS_ROOT=1
 TST_NEEDS_CMDS="dmesg mountpoint mount umount cat kill find mkdir rmdir grep"
 
 . tst_test.sh
+. cgroup_lib.sh
 
 do_setup()
 {
@@ -190,7 +191,7 @@ test_3()
 	fi
 
 	if grep -q -w "cpu" /proc/cgroups ; then
-		cpu_subsys_path=$(grep -w cpu /proc/mounts | awk '{ print $2 }')
+		cpu_subsys_path=$(get_cgroup_mountpoint "cpu")
 	else
 		tst_res TCONF "CONFIG_CGROUP_SCHED is not enabled"
 		return
@@ -292,8 +293,8 @@ test_5()
 	# $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 && mounted=$subsys1
-	[ -z "$mounted" ] && cat /proc/mounts | grep cgroup | grep -q $subsys2 && mounted=$subsys2
+	get_cgroup_mountpoint $subsys1 >/dev/null && mounted=$subsys1
+	[ -z "$mounted" ] && get_cgroup_mountpoint $subsys2 >/dev/null && mounted=$subsys2
 	if [ -z "$mounted" ]; then
 		mntpoint=cgroup
 		failing=$subsys1
@@ -306,7 +307,7 @@ test_5()
 		# Use the pre-esistent mountpoint as $mntpoint and use a
 		# co-mount with $failing: this way the 2nd mount will
 		# also fail (as expected) in this 'mirrored' configuration.
-		mntpoint=$(cat /proc/mounts | grep cgroup | grep $mounted | awk '{ print $2 }')
+		mntpoint=$(get_cgroup_mountpoint $mounted)
 		failing=$subsys1,$subsys2
 	fi
 
@@ -390,7 +391,13 @@ test_6()
 test_7_1()
 {
 	local subsys=$1
-	local subsys_path=$(grep -w $subsys /proc/mounts | cut -d ' ' -f 2)
+	# we should be careful to select a $subsys_path which is related to
+	# cgroup only: if cgroup debugging is enabled a 'debug' $subsys
+	# could be passed here as params and this will lead to ambiguity and
+	# errors when grepping simply for 'debug' in /proc/mounts since we'll
+	# find also /sys/kernel/debug. Helper takes care of this.
+	local subsys_path=$(get_cgroup_mountpoint $subsys)
+
 	if [ -z "$subsys_path" ]; then
 		mount -t cgroup -o $subsys xxx cgroup/
 		if [ $? -ne 0 ]; then
diff --git a/testcases/kernel/controllers/cgroup_lib.sh b/testcases/kernel/controllers/cgroup_lib.sh
new file mode 100644
index 000000000..5cdf55363
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup_lib.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2018-2019 ARM Ltd. All Rights Reserved.
+
+#
+# Helper to parse /proc/mounts to find the mountpoints (if any)
+# associated with the cgroup subsystem passed as param.
+#
+# It expects as single argument the cgroup subsytem for
+# which to search in /proc/mounts the mountpoint (if any).
+#
+# - returns true|false depending if any mountpoint has been found.
+# - echos back the mountpoint itself if any found
+#
+get_cgroup_mountpoint()
+{
+	local mntpoint
+	local line
+	local ret=1
+	local subsystem=$1
+
+	# fail straight away with no args
+	[ $# -eq 0 ] && return $ret
+
+	line=$(grep cgroup /proc/mounts | grep -w $subsystem)
+	ret=$?
+	# extract mountpoint if any exist
+	[ $ret = 0 ] && mntpoint=$(echo $line | awk '{ print $2 }') && echo $mntpoint
+
+	return $ret
+}
+
-- 
2.17.1

[LTP] [PATCH v2 4/4] cgroup: helpers various fixes

[Cristian Marussi]

Regarding cgroup/ helpers:
- converted Copyright headers to SPDX
- removed residual bashism
- renamed each helper to include a tag prefix (cgroup_regress_)
  in order to avoid possible name clashes once installed all together
  into testcases/bin

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 testcases/kernel/controllers/cgroup/Makefile  | 22 ++------
 .../cgroup/cgroup_regress_test_10_1.sh        | 14 +++++
 .../cgroup/cgroup_regress_test_10_2.sh        | 12 +++++
 .../cgroup/cgroup_regress_test_3_1.sh         | 14 +++++
 .../cgroup/cgroup_regress_test_3_2.sh         | 11 ++++
 .../cgroup/cgroup_regress_test_6_1.sh         | 13 +++++
 .../cgroup/cgroup_regress_test_6_2.c          | 37 +++++++++++++
 .../cgroup/cgroup_regress_test_9_1.sh         | 14 +++++
 .../cgroup/cgroup_regress_test_9_2.sh         | 13 +++++
 .../cgroup/cgroup_regression_test.sh          | 43 +++++----------
 .../controllers/cgroup/fork_processes.c       | 27 +++-------
 .../kernel/controllers/cgroup/getdelays.c     |  1 +
 .../kernel/controllers/cgroup/test_10_1.sh    | 33 ------------
 .../kernel/controllers/cgroup/test_10_2.sh    | 31 -----------
 .../kernel/controllers/cgroup/test_3_1.sh     | 33 ------------
 .../kernel/controllers/cgroup/test_3_2.sh     | 30 -----------
 .../kernel/controllers/cgroup/test_6_1.sh     | 32 ------------
 .../kernel/controllers/cgroup/test_6_2.c      | 52 -------------------
 .../kernel/controllers/cgroup/test_9_1.sh     | 33 ------------
 .../kernel/controllers/cgroup/test_9_2.sh     | 32 ------------
 20 files changed, 150 insertions(+), 347 deletions(-)
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_10_1.sh
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_10_2.sh
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_3_1.sh
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_3_2.sh
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_6_1.sh
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_9_1.sh
 create mode 100755 testcases/kernel/controllers/cgroup/cgroup_regress_test_9_2.sh
 delete mode 100755 testcases/kernel/controllers/cgroup/test_10_1.sh
 delete mode 100755 testcases/kernel/controllers/cgroup/test_10_2.sh
 delete mode 100755 testcases/kernel/controllers/cgroup/test_3_1.sh
 delete mode 100755 testcases/kernel/controllers/cgroup/test_3_2.sh
 delete mode 100755 testcases/kernel/controllers/cgroup/test_6_1.sh
 delete mode 100644 testcases/kernel/controllers/cgroup/test_6_2.c
 delete mode 100755 testcases/kernel/controllers/cgroup/test_9_1.sh
 delete mode 100755 testcases/kernel/controllers/cgroup/test_9_2.sh

diff --git a/testcases/kernel/controllers/cgroup/Makefile b/testcases/kernel/controllers/cgroup/Makefile
index 0cd60adae..8075ae80d 100644
--- a/testcases/kernel/controllers/cgroup/Makefile
+++ b/testcases/kernel/controllers/cgroup/Makefile
@@ -1,24 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2009, Cisco Systems Inc.
+# Author: Ngie Cooper, July 2009
 #
 #    kernel/controllers/cgroup test suite Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
 
 top_srcdir		?= ../../../..
 
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_10_1.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_10_1.sh
new file mode 100755
index 000000000..d7bc5feb2
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_10_1.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+while true
+do
+	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
+	mkdir cgroup/0 > /dev/null 2>&1
+	rmdir cgroup/0 > /dev/null 2>&1
+	umount cgroup/ > /dev/null 2>&1
+done
+
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_10_2.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_10_2.sh
new file mode 100755
index 000000000..b1222422d
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_10_2.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+while true
+do
+	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
+	umount cgroup/ > /dev/null 2>&1
+done
+
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_3_1.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_3_1.sh
new file mode 100755
index 000000000..a23afab4c
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_3_1.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+
+path=$1
+
+while true
+do
+	mkdir $path/0
+	rmdir $path/0
+done
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_3_2.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_3_2.sh
new file mode 100755
index 000000000..e1836a3e2
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_3_2.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+while true
+do
+	cat /proc/sched_debug > /dev/null
+done
+
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_1.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_1.sh
new file mode 100755
index 000000000..3ca5e3154
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_1.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+while true
+do
+	mount -t cgroup -o ns xxx cgroup/ > /dev/null 2>&1
+	rmdir cgroup/[1-9]* > /dev/null 2>&1
+	umount cgroup/ > /dev/null 2>&1
+done
+
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
new file mode 100644
index 000000000..9e0db9915
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2009 FUJITSU LIMITED
+ *
+ * Author: Li Zefan <lizf@cn.fujitsu.com>
+ */
+
+#define _GNU_SOURCE
+
+#include <sched.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include "test.h"
+
+#define DEFAULT_USEC	30000
+
+int foo(void __attribute__ ((unused)) * arg)
+{
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	int usec;
+
+	if (argc == 2)
+		usec = atoi(argv[1]);
+	else
+		usec = DEFAULT_USEC;
+
+	while (1) {
+		usleep(usec);
+		ltp_clone_quick(CLONE_NEWNS, foo, NULL);
+	}
+
+	tst_exit();
+}
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_9_1.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_9_1.sh
new file mode 100755
index 000000000..4b6741a91
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_9_1.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+while true
+do
+#	mount -t cgroup -o debug xxx cgroup/ > /dev/null 2>&1
+	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
+	cat cgroup/release_agent > /dev/null 2>&1
+	umount cgroup/ > /dev/null 2>&1
+done
+
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_9_2.sh b/testcases/kernel/controllers/cgroup/cgroup_regress_test_9_2.sh
new file mode 100755
index 000000000..b9c05d032
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_9_2.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+
+trap exit USR1
+while true
+do
+#	mount -t cgroup -o debug xxx cgroup/ > /dev/null 2>&1
+	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
+	umount cgroup/ > /dev/null 2>&1
+done
+
diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
index a66ed71e1..21760f178 100755
--- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
@@ -1,26 +1,7 @@
-#! /bin/sh
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
 
 TST_TESTFUNC=do_test
 TST_SETUP=do_setup
@@ -207,9 +188,9 @@ test_3()
 		cpu_subsys_path=cgroup
 	fi
 
-	./test_3_1.sh $cpu_subsys_path &
+	cgroup_regress_test_3_1.sh $cpu_subsys_path &
 	pid1=$!
-	./test_3_2.sh &
+	cgroup_regress_test_3_2.sh &
 	pid2=$!
 
 	sleep 30
@@ -358,9 +339,9 @@ test_6()
 	fi
 
 	# run the test for 30 secs
-	./test_6_1.sh &
+	cgroup_regress_test_6_1.sh &
 	local pid1=$!
-	./test_6_2 &
+	cgroup_regress_test_6_2 &
 	local pid2=$!
 
 	sleep 30
@@ -525,9 +506,9 @@ test_8()
 #---------------------------------------------------------------------------
 test_9()
 {
-	./test_9_1.sh &
+	cgroup_regress_test_9_1.sh &
 	local pid1=$!
-	./test_9_2.sh &
+	cgroup_regress_test_9_2.sh &
 	local pid2=$!
 
 	sleep 30
@@ -552,9 +533,9 @@ test_9()
 #---------------------------------------------------------------------------
 test_10()
 {
-	./test_10_1.sh &
+	cgroup_regress_test_10_1.sh &
 	local pid1=$!
-	./test_10_2.sh &
+	cgroup_regress_test_10_2.sh &
 	local pid2=$!
 
 	sleep 30
diff --git a/testcases/kernel/controllers/cgroup/fork_processes.c b/testcases/kernel/controllers/cgroup/fork_processes.c
index b4bb77101..6f2498bb8 100644
--- a/testcases/kernel/controllers/cgroup/fork_processes.c
+++ b/testcases/kernel/controllers/cgroup/fork_processes.c
@@ -1,24 +1,9 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2009 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2009 FUJITSU LIMITED
+ *
+ * Author: Li Zefan <lizf@cn.fujitsu.com>
+ */
 
 #include <sys/types.h>
 #include <sys/wait.h>
diff --git a/testcases/kernel/controllers/cgroup/getdelays.c b/testcases/kernel/controllers/cgroup/getdelays.c
index 4e87a13ca..57f37a5f8 100644
--- a/testcases/kernel/controllers/cgroup/getdelays.c
+++ b/testcases/kernel/controllers/cgroup/getdelays.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /* getdelays.c
  *
  * Utility to get per-pid and per-tgid delay accounting statistics
diff --git a/testcases/kernel/controllers/cgroup/test_10_1.sh b/testcases/kernel/controllers/cgroup/test_10_1.sh
deleted file mode 100755
index 2a7763b09..000000000
--- a/testcases/kernel/controllers/cgroup/test_10_1.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-for ((; ;))
-{
-	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
-	mkdir cgroup/0 > /dev/null 2>&1
-	rmdir cgroup/0 > /dev/null 2>&1
-	umount cgroup/ > /dev/null 2>&1
-}
-
diff --git a/testcases/kernel/controllers/cgroup/test_10_2.sh b/testcases/kernel/controllers/cgroup/test_10_2.sh
deleted file mode 100755
index a6e50d883..000000000
--- a/testcases/kernel/controllers/cgroup/test_10_2.sh
+++ /dev/null
@@ -1,31 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-for ((; ;))
-{
-	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
-	umount cgroup/ > /dev/null 2>&1
-}
-
diff --git a/testcases/kernel/controllers/cgroup/test_3_1.sh b/testcases/kernel/controllers/cgroup/test_3_1.sh
deleted file mode 100755
index dcc78a6b1..000000000
--- a/testcases/kernel/controllers/cgroup/test_3_1.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-
-path=$1
-
-for ((; ;))
-{
-	mkdir $path/0
-	rmdir $path/0
-}
diff --git a/testcases/kernel/controllers/cgroup/test_3_2.sh b/testcases/kernel/controllers/cgroup/test_3_2.sh
deleted file mode 100755
index 63bafaa46..000000000
--- a/testcases/kernel/controllers/cgroup/test_3_2.sh
+++ /dev/null
@@ -1,30 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-for ((; ;))
-{
-	cat /proc/sched_debug > /dev/null
-}
-
diff --git a/testcases/kernel/controllers/cgroup/test_6_1.sh b/testcases/kernel/controllers/cgroup/test_6_1.sh
deleted file mode 100755
index 4eb6b3293..000000000
--- a/testcases/kernel/controllers/cgroup/test_6_1.sh
+++ /dev/null
@@ -1,32 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-for ((; ;))
-{
-	mount -t cgroup -o ns xxx cgroup/ > /dev/null 2>&1
-	rmdir cgroup/[1-9]* > /dev/null 2>&1
-	umount cgroup/ > /dev/null 2>&1
-}
-
diff --git a/testcases/kernel/controllers/cgroup/test_6_2.c b/testcases/kernel/controllers/cgroup/test_6_2.c
deleted file mode 100644
index df85b1f91..000000000
--- a/testcases/kernel/controllers/cgroup/test_6_2.c
+++ /dev/null
@@ -1,52 +0,0 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2009 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
-
-#define _GNU_SOURCE
-
-#include <sched.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include "test.h"
-
-#define DEFAULT_USEC	30000
-
-int foo(void __attribute__ ((unused)) * arg)
-{
-	return 0;
-}
-
-int main(int argc, char **argv)
-{
-	int usec;
-
-	if (argc == 2)
-		usec = atoi(argv[1]);
-	else
-		usec = DEFAULT_USEC;
-
-	while (1) {
-		usleep(usec);
-		ltp_clone_quick(CLONE_NEWNS, foo, NULL);
-	}
-
-	tst_exit();
-}
diff --git a/testcases/kernel/controllers/cgroup/test_9_1.sh b/testcases/kernel/controllers/cgroup/test_9_1.sh
deleted file mode 100755
index b7c26a46c..000000000
--- a/testcases/kernel/controllers/cgroup/test_9_1.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-for ((; ;))
-{
-#	mount -t cgroup -o debug xxx cgroup/ > /dev/null 2>&1
-	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
-	cat cgroup/release_agent > /dev/null 2>&1
-	umount cgroup/ > /dev/null 2>&1
-}
-
diff --git a/testcases/kernel/controllers/cgroup/test_9_2.sh b/testcases/kernel/controllers/cgroup/test_9_2.sh
deleted file mode 100755
index 2d7d38675..000000000
--- a/testcases/kernel/controllers/cgroup/test_9_2.sh
+++ /dev/null
@@ -1,32 +0,0 @@
-#! /bin/bash
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-trap exit SIGUSR1
-for ((; ;))
-{
-#	mount -t cgroup -o debug xxx cgroup/ > /dev/null 2>&1
-	mount -t cgroup xxx cgroup/ > /dev/null 2>&1
-	umount cgroup/ > /dev/null 2>&1
-}
-
-- 
2.17.1

[LTP] [PATCH v2 4/4] cgroup: helpers various fixes

[Petr Vorel]

Hi Cristian,

> diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
> new file mode 100644
When you add new file, you need to put it also into .gitignore.
+ Remove /test_6_2 from it (original file).

...
> +++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2009 FUJITSU LIMITED
> + *
> + * Author: Li Zefan <lizf@cn.fujitsu.com>
> + */
> +
> +#define _GNU_SOURCE
Is _GNU_SOURCE really needed?

> +#include <sched.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include "test.h"
Please don't use legacy API.

> +
> +#define DEFAULT_USEC	30000
> +
> +int foo(void __attribute__ ((unused)) * arg)
> +{
> +	return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int usec;
> +
> +	if (argc == 2)
> +		usec = atoi(argv[1]);
> +	else
> +		usec = DEFAULT_USEC;
> +
> +	while (1) {
> +		usleep(usec);
> +		ltp_clone_quick(CLONE_NEWNS, foo, NULL);
> +	}
> +
> +	tst_exit();
> +}

...

> +++ b/testcases/kernel/controllers/cgroup/getdelays.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /* getdelays.c

There is build warning, can you please fix it:

getdelays.c: In function ‘get_family_id’:
getdelays.c:177:14: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
  int id = 0, rc;
              ^~
getdelays.c: In function ‘main’:
getdelays.c:418:7: warning: variable ‘i’ set but not used [-Wunused-but-set-variable]
   int i;
       ^

Kind regards,
Petr

[LTP] [PATCH v2 4/4] cgroup: helpers various fixes

[Cristian Marussi]

Hi

On 21/12/2018 14:01, Petr Vorel wrote:
> Hi Cristian,
> 
>> diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
>> new file mode 100644
> When you add new file, you need to put it also into .gitignore.
> + Remove /test_6_2 from it (original file).
> 

Didn't know. I'll do that.

> ...
>> +++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
>> @@ -0,0 +1,37 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2009 FUJITSU LIMITED
>> + *
>> + * Author: Li Zefan <lizf@cn.fujitsu.com>
>> + */
>> +
>> +#define _GNU_SOURCE
> Is _GNU_SOURCE really needed?
> 

I'll have a better look.

>> +#include <sched.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include "test.h"
> Please don't use legacy API.
> 

Ok.

>> +
>> +#define DEFAULT_USEC	30000
>> +
>> +int foo(void __attribute__ ((unused)) * arg)
>> +{
>> +	return 0;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	int usec;
>> +
>> +	if (argc == 2)
>> +		usec = atoi(argv[1]);
>> +	else
>> +		usec = DEFAULT_USEC;
>> +
>> +	while (1) {
>> +		usleep(usec);
>> +		ltp_clone_quick(CLONE_NEWNS, foo, NULL);
>> +	}
>> +
>> +	tst_exit();
>> +}
> 
> ...
> 
>> +++ b/testcases/kernel/controllers/cgroup/getdelays.c
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>  /* getdelays.c
> 
> There is build warning, can you please fix it:
> 
> getdelays.c: In function ‘get_family_id’:
> getdelays.c:177:14: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
>   int id = 0, rc;
>               ^~
> getdelays.c: In function ‘main’:
> getdelays.c:418:7: warning: variable ‘i’ set but not used [-Wunused-but-set-variable]
>    int i;
>        ^
> 

Fine.

> Kind regards,
> Petr
> 

Regards

Cristian

[LTP] [PATCH v2 4/4] cgroup: helpers various fixes

[Cristian Marussi]

Hi

On 21/12/2018 14:01, Petr Vorel wrote:
> Hi Cristian,
> 
>> diff --git a/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
>> new file mode 100644
> When you add new file, you need to put it also into .gitignore.
> + Remove /test_6_2 from it (original file).
> 
> ...
>> +++ b/testcases/kernel/controllers/cgroup/cgroup_regress_test_6_2.c
>> @@ -0,0 +1,37 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2009 FUJITSU LIMITED
>> + *
>> + * Author: Li Zefan <lizf@cn.fujitsu.com>
>> + */
>> +
>> +#define _GNU_SOURCE
> Is _GNU_SOURCE really needed?
> 

Yes..needed to compile the CLONE_NEWNS flag passed down the newlib function
ltp_clone_quick()

>> +#include <sched.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include "test.h"
> Please don't use legacy API.

Done.
> 
>> +
>> +#define DEFAULT_USEC	30000
>> +
>> +int foo(void __attribute__ ((unused)) * arg)
>> +{
>> +	return 0;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	int usec;
>> +
>> +	if (argc == 2)
>> +		usec = atoi(argv[1]);
>> +	else
>> +		usec = DEFAULT_USEC;
>> +
>> +	while (1) {
>> +		usleep(usec);
>> +		ltp_clone_quick(CLONE_NEWNS, foo, NULL);
>> +	}
>> +
>> +	tst_exit();
>> +}
> 
> ...
> 
>> +++ b/testcases/kernel/controllers/cgroup/getdelays.c
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>  /* getdelays.c
> 
> There is build warning, can you please fix it:
> 
> getdelays.c: In function ‘get_family_id’:
> getdelays.c:177:14: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
>   int id = 0, rc;
>               ^~
> getdelays.c: In function ‘main’:
> getdelays.c:418:7: warning: variable ‘i’ set but not used [-Wunused-but-set-variable]
>    int i;
>        ^
> 
Done.

Regards

Cristian

> Kind regards,
> Petr
> 

[LTP] [PATCH v2 0/4] cgroup tests newlib-porting

[Petr Vorel]

Hi Cristian,

thanks for your work, good work. I have some comments bellow.

> This series converts cgroup/ regression tests to LTP newlib and performs
> a general cleanup as follows:

> - remove bashism from all tests' scripts
> - mostly remove usage of global vars
> - convert to SPDX headers
You left big copyright in cpuacct.sh.

> - expose a controllers' common /proc/mounts parsing function
> - rename tests' helpers prefixing them with a test-specific tag to avoid
>   name clashes on install: I have not renamed the tests' helpers using
>   more specific meaningful name, since all of them performs really
>   trivial ops needed and meaningful in the context of the regression-test
>   subcase they serve; so I thought it would have been better to keep their
>   names as they are now, bound to the subcase they help.

> Cristian Marussi (4):
>   [LTP] cgroup_regression_test.sh ported to newlib
>   [LTP] cgroup_regression_test.sh cleanup
>   [LTP] cgroup_regression_test.sh: added helper
>   [LTP] cgroup: helpers various fixes

Can you please squash it into single commit?
Or in two (first cleanup & rewrite of group_regression_test.sh, second for
helpers). It does not bring much to split it like this.

I guess fork_processes.c and getdelays.c can be using new C API as well.
You don't have to do it, but it can bring benefits (using tst_res(), info about
command).

During cleanup please delete useless comments (getdelays.c: Compile with ...).
I personally take SPDX license as sign that test has been cleanup from bad code
and useless comments and ported into new API.

NOTE: you can add your copyright to the files.

Kind regards,
Petr

[LTP] [PATCH v2 0/4] cgroup tests newlib-porting

[Cristian Marussi]

Hi Petr,

On 21/12/2018 13:39, Petr Vorel wrote:
> Hi Cristian,
> 
> thanks for your work, good work. I have some comments bellow.

Thanks for you review.

> 
>> This series converts cgroup/ regression tests to LTP newlib and performs
>> a general cleanup as follows:
> 
>> - remove bashism from all tests' scripts
>> - mostly remove usage of global vars
>> - convert to SPDX headers
> You left big copyright in cpuacct.sh.

In this patch I have NOT touched anything else than the content of
testcases/kernel/controllers/cgroup which are files related to the
cgroup_regression_test.sh testcase; the file you mentioned is in
testcases/kernel/controllers/cpuacct/ ... am I missing something ?

> 
>> - expose a controllers' common /proc/mounts parsing function
>> - rename tests' helpers prefixing them with a test-specific tag to avoid
>>   name clashes on install: I have not renamed the tests' helpers using
>>   more specific meaningful name, since all of them performs really
>>   trivial ops needed and meaningful in the context of the regression-test
>>   subcase they serve; so I thought it would have been better to keep their
>>   names as they are now, bound to the subcase they help.
> 
>> Cristian Marussi (4):
>>   [LTP] cgroup_regression_test.sh ported to newlib
>>   [LTP] cgroup_regression_test.sh cleanup
>>   [LTP] cgroup_regression_test.sh: added helper
>>   [LTP] cgroup: helpers various fixes
> 
> Can you please squash it into single commit?
> Or in two (first cleanup & rewrite of group_regression_test.sh, second for
> helpers). It does not bring much to split it like this.

Ok I'll squash..I splitted thinking it could have been complained of being not
splitted :D

> 
> I guess fork_processes.c and getdelays.c can be using new C API as well.
> You don't have to do it, but it can bring benefits (using tst_res(), info about
> command).

I'll review those too...I focused only on the shell-scripts logic ... not sure
why in fact.


> 
> During cleanup please delete useless comments (getdelays.c: Compile with ...).
> I personally take SPDX license as sign that test has been cleanup from bad code
> and useless comments and ported into new API.
> 

ok

> NOTE: you can add your copyright to the files.
> 
> Kind regards,
> Petr
> 

Regards

Cristian

[LTP] [PATCH v2 0/4] cgroup tests newlib-porting

[Petr Vorel]

Hi Cristian,

> > You left big copyright in cpuacct.sh.
> In this patch I have NOT touched anything else than the content of
> testcases/kernel/controllers/cgroup which are files related to the
> cgroup_regression_test.sh testcase; the file you mentioned is in
> testcases/kernel/controllers/cpuacct/ ... am I missing something ?
Sorry, I was wrong.


> > Can you please squash it into single commit?
> > Or in two (first cleanup & rewrite of group_regression_test.sh, second for
> > helpers). It does not bring much to split it like this.

> Ok I'll squash..I splitted thinking it could have been complained of being not
> splitted :D
For small changes we like to have it atomic (split), but rewrite is IMHO better
do it in one commit or two (group_regression_test.sh and then the rest - that's
up to you). I focus too much on details for tests which are for very old kernel.


> > I guess fork_processes.c and getdelays.c can be using new C API as well.
> > You don't have to do it, but it can bring benefits (using tst_res(), info about
> > command).

> I'll review those too...I focused only on the shell-scripts logic ... not sure
> why in fact.
I'd prefer to convert it to new C API or leave it as they are.


Kind regards,
Petr

[LTP] [PATCH v2 0/4] cgroup tests newlib-porting

[Cristian Marussi]

Hi

On 21/12/2018 15:47, Petr Vorel wrote:
> Hi Cristian,
> 
>>> You left big copyright in cpuacct.sh.
>> In this patch I have NOT touched anything else than the content of
>> testcases/kernel/controllers/cgroup which are files related to the
>> cgroup_regression_test.sh testcase; the file you mentioned is in
>> testcases/kernel/controllers/cpuacct/ ... am I missing something ?
> Sorry, I was wrong.
> 
> 
>>> Can you please squash it into single commit?
>>> Or in two (first cleanup & rewrite of group_regression_test.sh, second for
>>> helpers). It does not bring much to split it like this.
> 
>> Ok I'll squash..I splitted thinking it could have been complained of being not
>> splitted :D
> For small changes we like to have it atomic (split), but rewrite is IMHO better
> do it in one commit or two (group_regression_test.sh and then the rest - that's
> up to you). I focus too much on details for tests which are for very old kernel.
> 
> 
>>> I guess fork_processes.c and getdelays.c can be using new C API as well.
>>> You don't have to do it, but it can bring benefits (using tst_res(), info about
>>> command).
> 
>> I'll review those too...I focused only on the shell-scripts logic ... not sure
>> why in fact.
> I'd prefer to convert it to new C API or leave it as they are.

I removed usage/references of the old API from fork_processes while getdelay it
was completely unrelated from LTP API; in fact both are helpers launched by the
LTP test scripts merely as background processes without any direct feedback
reported to the testcase caller.

How could I use the new C API in this case ?

Regards

Cristian

> 
> 
> Kind regards,
> Petr
>