Skip to contentSkip to conversation

controllers/cpuacct: rewrote testcases

PATCH Superseded Cedric Hnyda 2 participants 4 messages Nov 9, 2015 – Nov 10, 2015

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 controllers/cpuacct: rewrote testcases

Rewrote cpuacct testcases since they were outdated. Removed old files
Added 6 tests for cpuacct.

---
 runtest/controllers                                |   7 +
 testcases/kernel/controllers/cpuacct/.gitignore    |   1 -
 testcases/kernel/controllers/cpuacct/README        |  51 ----
 testcases/kernel/controllers/cpuacct/cpuacct.sh    | 221 ++++++++++++++
 .../kernel/controllers/cpuacct/cpuacct_setup.sh    | 122 --------
 .../kernel/controllers/cpuacct/cpuacct_task.sh     |  30 ++
 .../kernel/controllers/cpuacct/cpuacct_task01.c    |  17 --
 .../controllers/cpuacct/cpuacct_testplan.txt       |  44 ---
 .../kernel/controllers/cpuacct/run_cpuacct_test.sh | 340 ---------------------
 9 files changed, 258 insertions(+), 575 deletions(-)
 delete mode 100644 testcases/kernel/controllers/cpuacct/.gitignore
 delete mode 100644 testcases/kernel/controllers/cpuacct/README
 create mode 100755 testcases/kernel/controllers/cpuacct/cpuacct.sh
 delete mode 100755 testcases/kernel/controllers/cpuacct/cpuacct_setup.sh
 create mode 100755 testcases/kernel/controllers/cpuacct/cpuacct_task.sh
 delete mode 100644 testcases/kernel/controllers/cpuacct/cpuacct_task01.c
 delete mode 100644 testcases/kernel/controllers/cpuacct/cpuacct_testplan.txt
 delete mode 100755 testcases/kernel/controllers/cpuacct/run_cpuacct_test.sh
runtest/controllers +7 -0
index 6d9daf4..161e539 100644
@@ -197,6 +197,13 @@ cgroup_fj_stress_memory_1_100_100_2 run_cgroup_test_fj.sh stress memory 1 100 10
 
 controllers	test_controllers.sh
 
+cpuacct_1_1 cpuacct.sh 1 1
+cpuacct_1_1 cpuacct.sh 1 10
+cpuacct_1_1 cpuacct.sh 10 10
+cpuacct_1_1 cpuacct.sh 1 100
+cpuacct_1_1 cpuacct.sh 100 1
+cpuacct_1_1 cpuacct.sh 100 100
+
 cpuset_base_ops	cpuset_base_ops_testset.sh
 cpuset_inherit	cpuset_inherit_testset.sh
 cpuset_exclusive	cpuset_exclusive_test.sh
testcases/kernel/controllers/cpuacct/.gitignore deleted +0 -1
deleted file mode 100644
index 0dfdad3..0000000
@@ -1 +0,0 @@
-/cpuacct_task01
testcases/kernel/controllers/cpuacct/README deleted +0 -51

51 lines not on this page · raw message

testcases/kernel/controllers/cpuacct/cpuacct.sh new file +221 -0

221 lines not on this page · raw message

testcases/kernel/controllers/cpuacct/cpuacct_setup.sh deleted +0 -122

122 lines not on this page · raw message

testcases/kernel/controllers/cpuacct/cpuacct_task.sh new file +30 -0
new file mode 100755
index 0000000..54a9d65
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+################################################################################
+##                                                                            ##
+## Copyright (c) 2015 SUSE                                                    ##
+##                                                                            ##
+## 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: Cedric Hnyda <[email protected]>                                     ##
+##                                                                            ##
+################################################################################
+
+echo $$ > $1
+i=0
+while :
+do
+	i=$i
+done
testcases/kernel/controllers/cpuacct/cpuacct_task01.c deleted +0 -17
deleted file mode 100644
index 67736e1..0000000
@@ -1,17 +0,0 @@
-#include <stdio.h>
-#include <stdlib.h>
-
-int main(void)
-{
-	FILE *fp;
-
-	while (1) {
-		fp = fopen("txt.x", "w");
-		if (fp == NULL) {
-			perror("fopen failed");
-			exit(1);
-		}
-		fclose(fp);
-	}
-	return 0;
-}
testcases/kernel/controllers/cpuacct/cpuacct_testplan.txt deleted +0 -44

44 lines not on this page · raw message

testcases/kernel/controllers/cpuacct/run_cpuacct_test.sh deleted +0 -340

340 lines not on this page · raw message

Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/1 controllers/cpuacct: rewrote testcases

Hi!

runtest/controllers cgroup_fj_stress_memory_1_100_100_2 run_cgroup_test_fj.sh stress memory 1 100 10 · line 205

Quoted 6 lines
runtest/controllers +6 -0
+cpuacct_1_1 cpuacct.sh 1 1
+cpuacct_1_1 cpuacct.sh 1 10
+cpuacct_1_1 cpuacct.sh 10 10
+cpuacct_1_1 cpuacct.sh 1 100
+cpuacct_1_1 cpuacct.sh 100 1
+cpuacct_1_1 cpuacct.sh 100 100

^
These should be cpuacct_100_100, etc., right?

Quoted 52 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +49 -0
@@ -0,0 +1,221 @@
+#!/bin/bash
+
+################################################################################
+##                                                                            ##
+## Copyright (c) 2015 SUSE                                                    ##
+##                                                                            ##
+## 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: Cedric Hnyda <[email protected]>                                     ##
+##                                                                            ##
+################################################################################
+
+# Usage
+# ./cpuacct.sh nbsubgroup nbprocess
+#
+# 1) nbsubgroup : number of subgroup to create
+# 2) nbprocess : number of process to attach to each subgroup
+#
+# Description
+#
+# 1) Find if cpuacct is mounted, if not mounted, cpuacct will be mounted
+# 2) Create a subgroup ltp_test in cpuacct
+# 3) Create nbsubgroup subgroup in ltp_test and attach them nbprocess process
+# 4) Check that every subgroup's tasks file is not empty (test1)
+# 5) Kill all created process
+# 6) Check that ltp_test/subgroup*/cpuacct.usage != 0 (test2)
+# 7) Check that sum ltp_test/subgroup*/cpuacct.usage = ltp_test/cpuacct.usage
+# (test3)
+# 8) cleanup
+
+
+mounted=1
+mount_point=""
+max=$1
+nbprocess=$2
+

testcases/kernel/controllers/cpuacct/cpuacct.sh line 59

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +10 -0
+cd $LTPROOT/testcases/bin
+
+cnt=1
+for arg; do
+	if [ $cnt -gt 1 ]; then
+		NAME+="_"
+		NAME+=$arg
+	fi
+	cnt=$(( $cnt + 1 ))
+done

Hmm, for cycle over two arguments is kind of overkill if you could have done just do TCID="cpuacct_$1_$2"

testcases/kernel/controllers/cpuacct/cpuacct.sh line 67

Quoted 7 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +7 -0
+PREFIX="cpuacct_"
+
+export TCID=$PREFIX$1$NAME
+export TESTROOT=`pwd`
+export TST_TOTAL=3;
+export TST_COUNT=1;
+TMPFILE="$TESTROOT/tmp_tasks"

Please use tst_tmpdir and tst_rmdir from test.sh instead. The $PWD is not guaranteed to be writeable.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 79

Quoted 12 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +12 -0
+status=0
+
+verify_result()
+{
+	result=$1
+	expected=$2
+	message=$3
+	if [ "$result" -ne "$expected" ]; then
+		tst_resm TINFO "expected $expected, got $result"
+		cleanup;
+		tst_brkm TCONF NULL $message
+		exit 32

Please use the test.sh test library. The tst_brkm from the library calls exit and behaves the same as the C function. The only reason we actually suppport the tst_* binaries are testcases that still use them.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 86

Quoted 7 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +7 -0
+	fi
+}
+
+mount_cpuacct()
+{
+	mount -t cgroup -o cpuacct none $mount_point
+	verify_result $? 0 "Error occured while mounting cgroup"

Interesting idea. Maybe we should add such function to the test.sh library. Something that would call tst_brkm on non zero $?. Something as:

mount -t cgroup -o cpuacct none $mount_point tst_brkm_err TCONF "Failed to mount cgroup $mount_point"

and the test.sh would have:

tst_brkm_err()

{
	if [ $? -ne 0 ]; then
		tst_brkm $@
	fi
}

testcases/kernel/controllers/cpuacct/cpuacct.sh line 98

Quoted 12 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +12 -0
+}
+
+umount_cpuacct()
+{
+	tst_resm TINFO "Umounting cpuacct"
+	umount $mount_point
+	verify_result $? 0 "Error occured while umounting cgroup"
+}
+
+do_mkdir()
+{
+	path=$1

This is pretty much useless, can we do just mkdir -p $1 instead?

testcases/kernel/controllers/cpuacct/cpuacct.sh line 100

Quoted 2 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +2 -0
+	mkdir -p $path
+	verify_result $? 0 "Error occured with mkdir"

This is TBROK rather than TCONF.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 107

Quoted 7 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +7 -0
+}
+
+do_rmdir()
+{
+	path=$1
+	rmdir $path
+	verify_result $? 0 "Error occured with rmdir"

Here as well.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 119

Quoted 12 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +12 -0
+}
+
+setup()
+{
+	mount_point=`grep -w cpuacct /proc/mounts | cut -f 2 | cut -d " " -f2`
+	tst_resm TINFO "cpuacct: $mount_point"
+	if [ "$mount_point" = "" ]; then
+		mounted=0
+		mount_point=/dev/cgroup
+	fi
+
+	testpath=$mount_point/ltp_test

^
Maybe we can do ltp_$TCID here so it's clear which test has created it.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 123

Quoted 4 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +4 -0
+	if [ -e $testpath ]; then
+		rmdir $testpath/subgroup_*;
+		rmdir $testpath;
+	fi

I would omit cleanups in setup like this.

Quoted 9 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +9 -0
+	if [ "$mounted" -eq "0" ]; then
+		do_mkdir $mount_point
+		mount_cpuacct
+	fi
+	do_mkdir $testpath
+	for i in `seq 1 $max`
+	do
+		do_mkdir $testpath/subgroup_$i
+	done

testcases/kernel/controllers/cpuacct/cpuacct.sh line 143

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +10 -0
+
+	for i in `seq 1 $max`
+	do
+		for j in `seq 1 $nbprocess`
+		do
+			cpuacct_task.sh $testpath/subgroup_$i/tasks &
+		done
+	done
+
+	sleep 1

So we sleep here to get count someting on the cpu counters. Maybe it deserves a little comment, something as

# sleep a little to accumulate some statistics
Quoted 5 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +5 -0
+}
+
+cleanup()
+{
+	tst_resm TINFO "removing created directories"

testcases/kernel/controllers/cpuacct/cpuacct.sh line 158

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +10 -0
+	rmdir $testpath/subgroup_*;
+	rmdir $testpath;
+	killall -9 cpuacct_task.sh 1> /dev/null 2>&1;
+	if [ "$mounted" -ne 1 ] ; then
+		umount $mount_point
+		do_rmdir $mount_point
+	fi
+}
+
+##########################  main   #######################

Please no comments as this one.

Quoted 9 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +9 -0
+setup;
+
+error=0
+tst_resm TINFO "killing created process"
+for j in `seq 1 $max`
+do
+	cat $testpath/subgroup_$j/tasks > $TMPFILE
+	nlines=`cat $TMPFILE | wc -l`
+	if [ "$nlines" -eq "0" ]; then

testcases/kernel/controllers/cpuacct/cpuacct.sh line 178

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +10 -0
+		error=1
+		status=1
+	fi
+	for i in `seq 1 $nlines`
+	do
+		cur_pid=`sed -n "$i""p" $TMPFILE`
+		if [ -e /proc/$cur_pid/ ];then
+			kill "$cur_pid"
+		fi
+	done

Eh, why don't you do just:

for pid in cat $TMPFILE; do
if [ -e /proc/$pid/; then
kill $pid
fi
done

testcases/kernel/controllers/cpuacct/cpuacct.sh line 176

Quoted 1 line
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +1 -0
+done

Ah, you started the processes in the setup. It's kind of confusing to start test by killing processes. I would rather start them here than in the setup.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 177

Quoted 1 line
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +1 -0
+sleep 1

But why do we sleep here? To wait for the processes to terminate? In that case we should really rather do wait for all the pids we have killed.

Quoted 8 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +8 -0
+## check that every subgroup has at least one process in tasks file
+if [ "$error" -eq "1" ]; then
+	tst_resm TFAIL "Some subgroup didn't have any pid in their tasks file"
+else
+	tst_resm TPASS "subgroups' tasks files are not empty"
+fi
+
+acc=0

testcases/kernel/controllers/cpuacct/cpuacct.sh line 200

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +10 -0
+error=0
+for i in `seq 1 $max`
+do
+    tmp=`cat $testpath/subgroup_$i/cpuacct.usage`
+    if [ "$tmp" -eq "0" ]; then
+    	error=1
+    	status=1
+    fi
+    ((acc = acc + tmp))
+done

Please decide on consitent way of indentation and use it. This part mixes four spaces and tabs.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 208

Quoted 7 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +7 -0
+## check that cpuacct.usage != 0 for every subgroup
+((TST_COUNT = TST_COUNT + 1))
+if [ "$error" -eq "1" ]; then
+	tst_resm TFAIL "cpuacct.usage should not be equal to 0"
+else
+	tst_resm TPASS "cpuacct.usage is not equal to 0 for every subgroup"
+fi

I'm not 100% sure that this assertion will always hold. I will have to look into this.

testcases/kernel/controllers/cpuacct/cpuacct.sh line 211

Quoted 2 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +2 -0
+## check that ltp_subgroup/cpuacct.usage == sum ltp_subgroup/subgroup*/cpuacct.usage
+((TST_COUNT = TST_COUNT + 1))

This shoudn't be done here (the test.sh library does things like that for you).

testcases/kernel/controllers/cpuacct/cpuacct.sh line 221

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +10 -0
+ref=`cat $testpath/cpuacct.usage`
+if [ "$ref" != "$acc" ]; then
+	tst_resm TFAIL "ltp_test/cpuacct.usage not equal to ltp_test/subgroup*/cpuacct.usage"
+	status=1
+else
+	tst_resm TPASS "ltp_test/cpuacct.usage equal to ltp_test/subgroup*/cpuacct.usage"
+fi
+
+cleanup;
+exit $status

If you used the tst_resm from . test.sh library you do not need to store the status and use it here. Just call tst_exit as in the C library.

Quoted 15 lines
testcases/kernel/controllers/cpuacct/cpuacct_task.sh new file +9 -0
new file mode 100755
index 0000000..54a9d65
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+################################################################################
+##                                                                            ##
+## Copyright (c) 2015 SUSE                                                    ##
+##                                                                            ##
+## 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          ##

testcases/kernel/controllers/cpuacct/cpuacct_task.sh line 19

Quoted 10 lines
testcases/kernel/controllers/cpuacct/cpuacct_task.sh new file +10 -0
+## (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    ##

Please fix this overflow by moving "Foundation," to the previous line.

Quoted 11 lines
testcases/kernel/controllers/cpuacct/cpuacct_task.sh new file +11 -0
+##                                                                            ##
+## Author: Cedric Hnyda <[email protected]>                                     ##
+##                                                                            ##
+################################################################################
+
+echo $$ > $1
+i=0
+while :
+do
+	i=$i
+done
Signature · 2 lines
Cyril Hrubis
[email protected]
#2 lore raw

PATCH 1/1 controllers/cpuacct: rewrote testcases

Hi!

Quoted 15 lines

testcases/kernel/controllers/cpuacct/cpuacct.sh line 86

Quoted 7 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +7 -0
+	fi
+}
+
+mount_cpuacct()
+{
+	mount -t cgroup -o cpuacct none $mount_point
+	verify_result $? 0 "Error occured while mounting cgroup"

Interesting idea. Maybe we should add such function to the test.sh library. Something that would call tst_brkm on non zero $?. Something as:

mount -t cgroup -o cpuacct none $mount_point tst_brkm_err TCONF "Failed to mount cgroup $mount_point"

Quoted 8 lines

and the test.sh would have:

tst_brkm_err()

{
	if [ $? -ne 0 ]; then
		tst_brkm $@
	fi
}

And I nearly forgotten that we added even something better. We have ROD function that works as:

ROD mount -t cgroup -o cpuacct none $mount_point

(ROD is acronym for run or die and it does exactly the thing you need)

FYI: Have a look at:

https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#23-writing-a-testcase-in-shell

Signature · 2 lines
Cyril Hrubis
[email protected]
#3 lore raw

PATCH 1/1 controllers/cpuacct: rewrote testcases

In reply to Cyril Hrubis

Hi!

Quoted 10 lines

testcases/kernel/controllers/cpuacct/cpuacct.sh line 208

Quoted 7 lines
testcases/kernel/controllers/cpuacct/cpuacct.sh new file +7 -0
+## check that cpuacct.usage != 0 for every subgroup
+((TST_COUNT = TST_COUNT + 1))
+if [ "$error" -eq "1" ]; then
+	tst_resm TFAIL "cpuacct.usage should not be equal to 0"
+else
+	tst_resm TPASS "cpuacct.usage is not equal to 0 for every subgroup"
+fi

I'm not 100% sure that this assertion will always hold. I will have to look into this.

I've given it some thoughts and what we can do is to create the helper process in a way that it kill itself after it spends defined time executing code.

#include <stdio.h>
#include <sys/time.h>

int main(void)

{
        struct itimerval it = {.it_value = {.tv_sec = 0, .tv_usec = 10000}};

        setitimer(ITIMER_VIRTUAL, &it, NULL);

	for (;;);

        return 0;
}

This code sets up timer that sends a signal to a process after 1ms of execution.

Now we can run as many of these as we need and then we can do wait on the pids in the main process. That way we would have guaranteed that the execution time is slightly above 1ms.

Signature · 2 lines
Cyril Hrubis
[email protected]