All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/4] memcg/functional: rewrite
@ 2016-06-14 15:24 Stanislav Kholmanskikh
  2016-06-14 15:24 ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Stanislav Kholmanskikh
  2016-06-22 13:34 ` [LTP] [PATCH 1/4] memcg/functional: rewrite Cyril Hrubis
  0 siblings, 2 replies; 10+ messages in thread
From: Stanislav Kholmanskikh @ 2016-06-14 15:24 UTC (permalink / raw)
  To: ltp

* Use the LTP API for shell tests
* Remove memcg_getpagesize binary, since its functionality
  can be provided by 'getconf PAGESIZE'
* Move common code to memcg_lib.sh

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/controllers/memcg/.gitignore      |    1 -
 .../memcg/functional/memcg_function_test.sh        |   56 +++------
 .../memcg/functional/memcg_getpagesize.c           |   30 -----
 .../controllers/memcg/functional/memcg_lib.sh      |  128 ++++++++++++++------
 .../functional/memcg_max_usage_in_bytes_test.sh    |   49 +-------
 .../functional/memcg_memsw_limit_in_bytes_test.sh  |   54 ++-------
 .../memcg_move_charge_at_immigrate_test.sh         |   40 +------
 .../memcg/functional/memcg_stat_test.sh            |   49 +-------
 .../memcg/functional/memcg_usage_in_bytes_test.sh  |   47 +------
 .../memcg/functional/memcg_use_hierarchy_test.sh   |   39 +-----
 10 files changed, 148 insertions(+), 345 deletions(-)
 delete mode 100644 testcases/kernel/controllers/memcg/functional/memcg_getpagesize.c

diff --git a/testcases/kernel/controllers/memcg/.gitignore b/testcases/kernel/controllers/memcg/.gitignore
index 6f62034..8730251 100644
--- a/testcases/kernel/controllers/memcg/.gitignore
+++ b/testcases/kernel/controllers/memcg/.gitignore
@@ -1,5 +1,4 @@
 /control/mem_process
-/functional/memcg_getpagesize
 /functional/memcg_process
 /regression/memcg_test_1
 /regression/memcg_test_2
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
index 2c2f32a..38fb311 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
@@ -25,11 +25,18 @@
 ##                                                                            ##
 ################################################################################
 
-export TCID="memcg_function_test"
-export TST_TOTAL=38
-export TST_COUNT=0
+TCID="memcg_function_test"
+TST_TOTAL=38
 
-. memcg_lib.sh || exit 1
+shmmax_cleanup()
+{
+	if [ -n "$shmmax" ]; then
+		echo "$shmmax" > /proc/sys/kernel/shmmax
+	fi
+}
+LOCAL_CLEANUP=shmmax_cleanup
+
+. memcg_lib.sh
 
 # Case 1 - 10: Test the management and counting of memory
 testcase_1()
@@ -205,7 +212,7 @@ testcase_28()
 # Case 29 - 35: Test memory.force_empty
 testcase_29()
 {
-	$TEST_PATH/memcg_process --mmap-anon -s $PAGESIZE &
+	memcg_process --mmap-anon -s $PAGESIZE &
 	pid=$!
 	sleep 1
 	echo $pid > tasks
@@ -226,7 +233,7 @@ testcase_29()
 
 testcase_30()
 {
-	$TEST_PATH/memcg_process --mmap-lock2 -s $PAGESIZE &
+	memcg_process --mmap-lock2 -s $PAGESIZE &
 	pid=$!
 	sleep 1
 	echo $pid > tasks
@@ -293,40 +300,9 @@ testcase_38()
 
 shmmax=`cat /proc/sys/kernel/shmmax`
 if [ $shmmax -lt $HUGEPAGESIZE ]; then
-	echo $(($HUGEPAGESIZE)) > /proc/sys/kernel/shmmax
+	ROD "$HUGEPAGESIZE" \> /proc/sys/kernel/shmmax
 fi
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
-
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
-
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
+run_tests
 
-	cleanup
-done
-
-echo $shmmax > /proc/sys/kernel/shmmax
-
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
+tst_exit
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_getpagesize.c b/testcases/kernel/controllers/memcg/functional/memcg_getpagesize.c
deleted file mode 100644
index 557b540..0000000
--- a/testcases/kernel/controllers/memcg/functional/memcg_getpagesize.c
+++ /dev/null
@@ -1,30 +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>                                     */
-/*                                                                            */
-/******************************************************************************/
-
-#include <unistd.h>
-#include <stdio.h>
-
-int main(void)
-{
-	printf("%d\n", getpagesize());
-	return 0;
-}
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index c90d0a2..5e42fac 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -22,26 +22,53 @@
 ##                                                                            ##
 ################################################################################
 
+. test.sh
+
 if [ "x$(grep -w memory /proc/cgroups | cut -f4)" != "x1" ]; then
-	echo "WARNING:";
-	echo "Either Kernel does not support for memory resource controller or feature not enabled";
-	echo "Skipping all memcgroup testcases....";
-	exit 0
+	tst_brkm TCONF "Kernel does not support the memory resource controller"
 fi
 
-cd $LTPROOT/testcases/bin
+PAGESIZE=$(getconf PAGESIZE)
+if [ $? -ne 0 ]; then
+	tst_brkm TBROK "getconf PAGESIZE failed"
+fi
 
-TEST_PATH=$PWD
-PAGESIZE=`./memcg_getpagesize`
-HUGEPAGESIZE=`grep Hugepagesize /proc/meminfo | awk '{ print $2 }'`
+HUGEPAGESIZE=$(awk '/Hugepagesize/ {print $2}' /proc/meminfo)
 [ -z $HUGEPAGESIZE ] && HUGEPAGESIZE=0
 HUGEPAGESIZE=$(( $HUGEPAGESIZE * 1024 ))
 PASS=0
 FAIL=1
 orig_memory_use_hierarchy=""
 
-cur_id=0
-failed=0
+MEMSW_USAGE_FLAG=0
+MEMSW_LIMIT_FLAG=0
+
+tst_tmpdir
+TMP_DIR="$PWD"
+
+cleanup()
+{
+	if [ -n "$LOCAL_CLEANUP" ]; then
+		$LOCAL_CLEANUP
+	fi
+
+	killall -9 memcg_process 2> /dev/null
+	wait
+
+	cd "$TMP_DIR"
+
+	if [ -n "$TEST_ID" -a -d "/dev/memcg/$TEST_ID" ]; then
+		rmdir "/dev/memcg/$TEST_ID"
+	fi
+
+	if [ -d "/dev/memcg" ]; then
+		umount /dev/memcg
+		rmdir /dev/memcg
+	fi
+
+	tst_rmdir
+}
+TST_CLEANUP=cleanup
 
 # Record the test result of a test case
 # $1 - The result of the test case, $PASS or $FAIL
@@ -55,7 +82,6 @@ result()
 		tst_resm TPASS "$info"
 	else
 		tst_resm TFAIL "$info"
-		: $(( failed += 1 ))
 	fi
 }
 
@@ -83,7 +109,7 @@ warmup()
 {
 	pid=$1
 
-	echo "Warming up for test: $cur_id, pid: $pid"
+	tst_resm TINFO "Warming up pid: $pid"
 	kill -s USR1 $pid 2> /dev/null
 	sleep 1
 	kill -s USR1 $pid 2> /dev/null
@@ -91,10 +117,10 @@ warmup()
 
 	kill -0 $pid
 	if [ $? -ne 0 ]; then
-		result $FAIL "cur_id=$cur_id"
+		result $FAIL ""
 		return 1
 	else
-		echo "Process is still here after warm up: $pid"
+		tst_resm TINFO "Process is still here after warm up: $pid"
 	fi
 
 	return 0
@@ -109,8 +135,8 @@ warmup()
 # $5 - check after free ?
 test_mem_stat()
 {
-	echo "Running $TEST_PATH/memcg_process $1 -s $2"
-	$TEST_PATH/memcg_process $1 -s $2 &
+	tst_resm TINFO "Running memcg_process $1 -s $2"
+	memcg_process $1 -s $2 &
 	sleep 1
 
 	warmup $!
@@ -142,8 +168,8 @@ test_mem_stat()
 # $5 - check after free ?
 test_max_usage_in_bytes()
 {
-	echo "Running $TEST_PATH/memcg_process $1 -s $2"
-	$TEST_PATH/memcg_process $1 -s $2 &
+	tst_resm TINFO "Running memcg_process $1 -s $2"
+	memcg_process $1 -s $2 &
 	sleep 1
 
 	warmup $!
@@ -173,8 +199,8 @@ test_max_usage_in_bytes()
 # $2 - the -s parameter of 'process', such as 4096
 malloc_free_memory()
 {
-	echo "Running $TEST_PATH/memcg_process $1 -s $2"
-	$TEST_PATH/memcg_process $1 -s $2 &
+	tst_resm TINFO "Running memcg_process $1 -s $2"
+	memcg_process $1 -s $2 &
 	sleep 1
 
 	echo $! > tasks
@@ -218,7 +244,7 @@ test_proc_kill()
 		fi
 	fi
 
-	$TEST_PATH/memcg_process $2 -s $3 &
+	memcg_process $2 -s $3 &
 	pid=$!
 	sleep 1
 	echo $pid > tasks
@@ -271,6 +297,8 @@ test_limit_in_bytes()
 	fi
 }
 
+# Never used, so untested
+#
 # Test memory controller doesn't charge hugepage
 # $1 - the value of /proc/sys/vm/nr_hugepages
 # $2 - the parameters of 'process', --mmap-file or --shm
@@ -278,7 +306,7 @@ test_limit_in_bytes()
 # $4 - 0: expected failure, 1: expected success
 test_hugepage()
 {
-	TMP_FILE=$TEST_PATH/tmp
+	TMP_FILE="$TMP_DIR/tmp"
 	nr_hugepages=`cat /proc/sys/vm/nr_hugepages`
 
 	mkdir /hugetlb
@@ -286,7 +314,7 @@ test_hugepage()
 
 	echo $1 > /proc/sys/vm/nr_hugepages
 
-	$TEST_PATH/memcg_process $2 --hugepage -s $3 > $TMP_FILE 2>&1 &
+	memcg_process $2 --hugepage -s $3 > $TMP_FILE 2>&1 &
 	sleep 1
 
 	kill -s USR1 $! 2> /dev/null
@@ -333,8 +361,8 @@ test_subgroup()
 	echo $1 > memory.limit_in_bytes
 	echo $2 > subgroup/memory.limit_in_bytes
 
-	echo "Running $TEST_PATH/memcg_process --mmap-anon -s $PAGESIZE"
-	$TEST_PATH/memcg_process --mmap-anon -s $PAGESIZE &
+	tst_resm TINFO "Running memcg_process --mmap-anon -s $PAGESIZE"
+	memcg_process --mmap-anon -s $PAGESIZE &
 	sleep 1
 
 	warmup $!
@@ -369,7 +397,8 @@ test_move_charge()
 {
 	mkdir subgroup_a
 
-	$TEST_PATH/memcg_process $1 -s $2 &
+	tst_resm TINFO "Running memcg_process $1 -s $2"
+	memcg_process $1 -s $2 &
 	sleep 1
 	warmup $!
 	if [ $? -ne 0 ]; then
@@ -400,8 +429,10 @@ test_move_charge()
 	rmdir subgroup_a subgroup_b
 }
 
-cleanup()
+cleanup_test()
 {
+	TEST_ID="$1"
+
 	if [ -n "$orig_memory_use_hierarchy" ];then
 		echo $orig_memory_use_hierarchy > \
 		     /dev/memcg/memory.use_hierarchy
@@ -413,18 +444,22 @@ cleanup()
 	fi
 
 	killall -9 memcg_process 2>/dev/null
-	if [ -e /dev/memcg ]; then
-		umount /dev/memcg 2>/dev/null
-		rmdir /dev/memcg 2>/dev/null
-	fi
+	wait
+
+	ROD cd "$TMP_DIR"
+
+	ROD rmdir "/dev/memcg/$TEST_ID"
+	TEST_ID=""
+	ROD umount /dev/memcg
+	ROD rmdir /dev/memcg
 }
 
-do_mount()
+setup_test()
 {
-	cleanup
+	TEST_ID="$1"
 
-	mkdir /dev/memcg 2> /dev/null
-	mount -t cgroup -omemory memcg /dev/memcg
+	ROD mkdir /dev/memcg
+	ROD mount -t cgroup -omemory memcg /dev/memcg
 
 	# The default value for memory.use_hierarchy is 0 and some of tests
 	# (memcg_stat_test.sh and memcg_use_hierarchy_test.sh) expect it so
@@ -442,4 +477,27 @@ do_mount()
 				"to 0 failed"
 		fi
 	fi
+
+	ROD mkdir "/dev/memcg/$TEST_ID"
+	ROD cd "/dev/memcg/$TEST_ID"
+}
+
+# Run all the test cases
+run_tests()
+{
+	for i in $(seq 1 $TST_TOTAL); do
+		setup_test $i
+
+		if [ -e memory.memsw.limit_in_bytes ]; then
+			MEMSW_LIMIT_FLAG=1
+		fi
+
+		if [ -e memory.memsw.max_usage_in_bytes ]; then
+			MEMSW_USAGE_FLAG=1
+		fi
+
+		testcase_$i
+
+		cleanup_test $i
+	done
 }
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
index 9e286f5..91112be 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
@@ -26,13 +26,10 @@
 # History:      2012/01/17 - Created.
 #
 
-export TCID="memcg_max_usage_in_bytes_test"
-export TST_TOTAL=4
-export TST_COUNT=0
+TCID="memcg_max_usage_in_bytes_test"
+TST_TOTAL=4
 
-. memcg_lib.sh || exit 1
-
-MEM_SWAP_FLAG=0
+. memcg_lib.sh
 
 # Test memory.max_usage_in_bytes
 testcase_1()
@@ -44,7 +41,7 @@ testcase_1()
 # Test memory.memsw.max_usage_in_bytes
 testcase_2()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_USAGE_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -65,7 +62,7 @@ testcase_3()
 # Test reset memory.memsw.max_usage_in_bytes
 testcase_4()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_USAGE_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -76,39 +73,7 @@ testcase_4()
 		"memory.memsw.max_usage_in_bytes" $((PAGESIZE*1024)) 1
 }
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
-
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
-
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	if [ -e memory.memsw.max_usage_in_bytes ]; then
-		MEM_SWAP_FLAG=1
-	fi
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
+run_tests
 
-	cleanup
-done
+tst_exit
 
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_memsw_limit_in_bytes_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_memsw_limit_in_bytes_test.sh
index f49508c..23eb095 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_memsw_limit_in_bytes_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_memsw_limit_in_bytes_test.sh
@@ -26,13 +26,10 @@
 # History:      2012/01/3 - Created.
 #
 
-export TCID="memcg_memsw_limit_in_bytes_test"
-export TST_TOTAL=12
-export TST_COUNT=0
+TCID="memcg_memsw_limit_in_bytes_test"
+TST_TOTAL=12
 
-. memcg_lib.sh || exit 1
-
-MEM_SWAP_FLAG=0
+. memcg_lib.sh
 
 testcase_1()
 {
@@ -76,7 +73,7 @@ testcase_8()
 
 testcase_9()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_LIMIT_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -94,7 +91,7 @@ testcase_9()
 
 testcase_10()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_LIMIT_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -106,7 +103,7 @@ testcase_10()
 
 testcase_11()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_LIMIT_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -118,7 +115,7 @@ testcase_11()
 
 testcase_12()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_LIMIT_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -128,39 +125,6 @@ testcase_12()
 	result $(( !($? != 0) )) "return value is $?"
 }
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
-
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
-
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	if [ -e memory.memsw.limit_in_bytes ]; then
-		MEM_SWAP_FLAG=1
-	fi
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
-
-	cleanup
-done
+run_tests
 
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
+tst_exit
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_move_charge_at_immigrate_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_move_charge_at_immigrate_test.sh
index 406bd07..aeb7d0f 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_move_charge_at_immigrate_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_move_charge_at_immigrate_test.sh
@@ -26,11 +26,10 @@
 # History:      2012/01/16 - Created.
 #
 
-export TCID="memcg_move_charge_at_immigrate_test"
-export TST_TOTAL=4
-export TST_COUNT=0
+TCID="memcg_move_charge_at_immigrate_test"
+TST_TOTAL=4
 
-. memcg_lib.sh || exit 1
+. memcg_lib.sh
 
 # Test disable moving charges
 testcase_1()
@@ -58,35 +57,6 @@ testcase_4()
 	test_move_charge "--mmap-anon --shm" $PAGESIZE 3 $PAGESIZE $PAGESIZE 0 0
 }
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
+run_tests
 
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
-
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
-
-	cleanup
-done
-
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
+tst_exit
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh
index 0151f85..1f28aa4 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh
@@ -26,13 +26,10 @@
 # History:      2012/01/16 - Created.
 #
 
-export TCID="memcg_stat_test"
-export TST_TOTAL=8
-export TST_COUNT=0
+TCID="memcg_stat_test"
+TST_TOTAL=8
 
-. memcg_lib.sh || exit 1
-
-MEM_SWAP_FLAG=0
+. memcg_lib.sh
 
 # Test cache
 testcase_1()
@@ -93,7 +90,7 @@ testcase_6()
 # Test hierarchical_memsw_limit with enabling hierarchical accounting
 testcase_7()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_LIMIT_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -116,7 +113,7 @@ testcase_7()
 # Test hierarchical_memsw_limit with disabling hierarchical accounting
 testcase_8()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_LIMIT_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -136,39 +133,7 @@ testcase_8()
 	rmdir subgroup
 }
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
-
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
-
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	if [ -e memory.memsw.limit_in_bytes ]; then
-		MEM_SWAP_FLAG=1
-	fi
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
+run_tests
 
-	cleanup
-done
+tst_exit
 
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh
index a4d0542..ab86318 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh
@@ -26,13 +26,10 @@
 # History:      2012/01/16 - Created.
 #
 
-export TCID="memcg_usage_in_bytes_test"
-export TST_TOTAL=2
-export TST_COUNT=0
+TCID="memcg_usage_in_bytes_test"
+TST_TOTAL=2
 
-. memcg_lib.sh || exit 1
-
-MEM_SWAP_FLAG=0
+. memcg_lib.sh
 
 # Test memory.usage_in_bytes
 testcase_1()
@@ -44,7 +41,7 @@ testcase_1()
 # Test memory.memsw.usage_in_bytes
 testcase_2()
 {
-	if [ $MEM_SWAP_FLAG -eq 0 ]; then
+	if [ "$MEMSW_USAGE_FLAG" -eq 0 ]; then
 		tst_resm TCONF "mem+swap is not enabled"
 		return
 	fi
@@ -55,39 +52,7 @@ testcase_2()
 		"memory.memsw.usage_in_bytes" $((PAGESIZE*1024)) 0
 }
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
-
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
-
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	if [ -e memory.memsw.usage_in_bytes ]; then
-		MEM_SWAP_FLAG=1
-	fi
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
+run_tests
 
-	cleanup
-done
+tst_exit
 
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
index f48c50a..04b456d 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
@@ -26,11 +26,10 @@
 # History:      2012/01/14 - Created.
 #
 
-export TCID="memcg_use_hierarchy_test"
-export TST_TOTAL=3
-export TST_COUNT=0
+TCID="memcg_use_hierarchy_test"
+TST_TOTAL=3
 
-. memcg_lib.sh || exit 1
+. memcg_lib.sh
 
 # test if one of the ancestors goes over its limit, the proces will be killed
 testcase_1()
@@ -67,35 +66,7 @@ testcase_3()
 	rmdir subgroup
 }
 
-# Run all the test cases
-for i in $(seq 1 $TST_TOTAL)
-do
-	export TST_COUNT=$(( $TST_COUNT + 1 ))
-	cur_id=$i
+run_tests
 
-	do_mount
-	if [ $? -ne 0 ]; then
-		echo "Cannot create memcg"
-		exit 1
-	fi
+tst_exit
 
-	# prepare
-	mkdir /dev/memcg/$i 2> /dev/null
-	cd /dev/memcg/$i
-
-	# run the case
-	testcase_$i
-
-	# clean up
-	sleep 1
-	cd $TEST_PATH
-	rmdir /dev/memcg/$i
-
-	cleanup
-done
-
-if [ $failed -ne 0 ]; then
-	exit $failed
-else
-	exit 0
-fi
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup
  2016-06-14 15:24 [LTP] [PATCH 1/4] memcg/functional: rewrite Stanislav Kholmanskikh
@ 2016-06-14 15:24 ` Stanislav Kholmanskikh
  2016-06-14 15:24   ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Stanislav Kholmanskikh
  2016-06-22 13:35   ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Cyril Hrubis
  2016-06-22 13:34 ` [LTP] [PATCH 1/4] memcg/functional: rewrite Cyril Hrubis
  1 sibling, 2 replies; 10+ messages in thread
From: Stanislav Kholmanskikh @ 2016-06-14 15:24 UTC (permalink / raw)
  To: ltp

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 .../controllers/memcg/functional/memcg_process.c   |   49 ++++++++++----------
 1 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_process.c b/testcases/kernel/controllers/memcg/functional/memcg_process.c
index c12fae1..4ad2d6f 100644
--- a/testcases/kernel/controllers/memcg/functional/memcg_process.c
+++ b/testcases/kernel/controllers/memcg/functional/memcg_process.c
@@ -35,20 +35,20 @@
 #include <string.h>
 #include <unistd.h>
 
-int fd;
+static int fd;
 
-int flag_exit;
-int flag_allocated;
+static volatile int flag_exit;
+static volatile int flag_allocated;
 
-int opt_mmap_anon;
-int opt_mmap_file;
-int opt_mmap_lock1;
-int opt_mmap_lock2;
-int opt_shm;
-int opt_hugepage;
+static int opt_mmap_anon;
+static int opt_mmap_file;
+static int opt_mmap_lock1;
+static int opt_mmap_lock2;
+static int opt_shm;
+static int opt_hugepage;
 
-int key_id;			/* used with opt_shm */
-unsigned long memsize;
+static int key_id;			/* used with opt_shm */
+static unsigned long memsize;
 
 #define FILE_HUGEPAGE	"/hugetlb/hugepagefile"
 
@@ -59,7 +59,7 @@ unsigned long memsize;
 #define SHM		(SCHAR_MAX + 5)
 #define HUGEPAGE	(SCHAR_MAX + 6)
 
-const struct option long_opts[] = {
+static const struct option long_opts[] = {
 	{"mmap-anon", 0, NULL, MMAP_ANON},
 	{"mmap-file", 0, NULL, MMAP_FILE},
 	{"mmap-lock1", 0, NULL, MMAP_LOCK1},
@@ -74,7 +74,7 @@ const struct option long_opts[] = {
 /*
  * process_options: read options from user input
  */
-void process_options(int argc, char *argv[])
+static void process_options(int argc, char *argv[])
 {
 	int c;
 	char *end;
@@ -117,7 +117,7 @@ void process_options(int argc, char *argv[])
 /*
  * touch_memory: force allocating phy memory
  */
-void touch_memory(char *p, int size)
+static void touch_memory(char *p, int size)
 {
 	int i;
 	int pagesize = getpagesize();
@@ -126,7 +126,7 @@ void touch_memory(char *p, int size)
 		p[i] = 0xef;
 }
 
-void mmap_anon()
+static void mmap_anon(void)
 {
 	static char *p;
 
@@ -142,7 +142,7 @@ void mmap_anon()
 	}
 }
 
-void mmap_file()
+static void mmap_file(void)
 {
 	static char *p;
 	static int fd_hugepage;
@@ -177,7 +177,7 @@ void mmap_file()
 	}
 }
 
-void mmap_lock1()
+static void mmap_lock1(void)
 {
 	static char *p;
 
@@ -193,7 +193,7 @@ void mmap_lock1()
 	}
 }
 
-void mmap_lock2()
+static void mmap_lock2(void)
 {
 	static char *p;
 
@@ -215,7 +215,7 @@ void mmap_lock2()
 	}
 }
 
-void shm()
+static void shm(void)
 {
 	static char *p;
 	static int shmid;
@@ -258,7 +258,7 @@ void shm()
 /*
  * sigint_handler: handle SIGINT by set the exit flag.
  */
-void sigint_handler(int __attribute__ ((unused)) signo)
+static void sigint_handler(int __attribute__ ((unused)) signo)
 {
 	flag_exit = 1;
 }
@@ -272,7 +272,7 @@ void sigint_handler(int __attribute__ ((unused)) signo)
  * When we receive SIGUSR again, we will free all the allocated
  * memory.
  */
-void sigusr_handler(int __attribute__ ((unused)) signo)
+static void sigusr_handler(int __attribute__ ((unused)) signo)
 {
 	if (opt_mmap_anon)
 		mmap_anon();
@@ -303,14 +303,15 @@ int main(int argc, char *argv[])
 	memset(&sigint_action, 0, sizeof(sigint_action));
 	memset(&sigusr_action, 0, sizeof(sigusr_action));
 
-	/* TODO: add error handling below. */
 	sigemptyset(&sigint_action.sa_mask);
 	sigint_action.sa_handler = &sigint_handler;
-	sigaction(SIGINT, &sigint_action, NULL);
+	if (sigaction(SIGINT, &sigint_action, NULL))
+		err(1, "sigaction(SIGINT)");
 
 	sigemptyset(&sigusr_action.sa_mask);
 	sigusr_action.sa_handler = &sigusr_handler;
-	sigaction(SIGUSR1, &sigusr_action, NULL);
+	if (sigaction(SIGUSR1, &sigusr_action, NULL))
+		err(1, "sigaction(SIGUSR1)");
 
 	process_options(argc, argv);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/4] memcg/functional: use checkpoints
  2016-06-14 15:24 ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Stanislav Kholmanskikh
@ 2016-06-14 15:24   ` Stanislav Kholmanskikh
  2016-06-14 15:24     ` [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed Stanislav Kholmanskikh
                       ` (2 more replies)
  2016-06-22 13:35   ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Cyril Hrubis
  1 sibling, 3 replies; 10+ messages in thread
From: Stanislav Kholmanskikh @ 2016-06-14 15:24 UTC (permalink / raw)
  To: ltp

This patch implements the idea proposed in [1], [2], that
we use the checkpoint interface to let memcg_process inform
the parent process that it's ready.

In my enviroment it saves ~50 secs of time.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 .../memcg/functional/memcg_function_test.sh        |    4 ++--
 .../controllers/memcg/functional/memcg_lib.sh      |   15 ++++++++-------
 .../controllers/memcg/functional/memcg_process.c   |    6 ++++++
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
index 38fb311..b94e940 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
@@ -214,7 +214,7 @@ testcase_29()
 {
 	memcg_process --mmap-anon -s $PAGESIZE &
 	pid=$!
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 	echo $pid > tasks
 	kill -s USR1 $pid 2> /dev/null
 	sleep 1
@@ -235,7 +235,7 @@ testcase_30()
 {
 	memcg_process --mmap-lock2 -s $PAGESIZE &
 	pid=$!
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 	echo $pid > tasks
 	kill -s USR1 $pid 2> /dev/null
 	sleep 1
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index 5e42fac..b191ea6 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -22,6 +22,7 @@
 ##                                                                            ##
 ################################################################################
 
+TST_NEEDS_CHECKPOINTS=1
 . test.sh
 
 if [ "x$(grep -w memory /proc/cgroups | cut -f4)" != "x1" ]; then
@@ -137,7 +138,7 @@ test_mem_stat()
 {
 	tst_resm TINFO "Running memcg_process $1 -s $2"
 	memcg_process $1 -s $2 &
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 
 	warmup $!
 	if [ $? -ne 0 ]; then
@@ -170,7 +171,7 @@ test_max_usage_in_bytes()
 {
 	tst_resm TINFO "Running memcg_process $1 -s $2"
 	memcg_process $1 -s $2 &
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 
 	warmup $!
 	if [ $? -ne 0 ]; then
@@ -201,7 +202,7 @@ malloc_free_memory()
 {
 	tst_resm TINFO "Running memcg_process $1 -s $2"
 	memcg_process $1 -s $2 &
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 
 	echo $! > tasks
 	kill -s USR1 $! 2> /dev/null
@@ -246,7 +247,7 @@ test_proc_kill()
 
 	memcg_process $2 -s $3 &
 	pid=$!
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 	echo $pid > tasks
 
 	kill -s USR1 $pid 2> /dev/null
@@ -315,7 +316,7 @@ test_hugepage()
 	echo $1 > /proc/sys/vm/nr_hugepages
 
 	memcg_process $2 --hugepage -s $3 > $TMP_FILE 2>&1 &
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 
 	kill -s USR1 $! 2> /dev/null
 	sleep 1
@@ -363,7 +364,7 @@ test_subgroup()
 
 	tst_resm TINFO "Running memcg_process --mmap-anon -s $PAGESIZE"
 	memcg_process --mmap-anon -s $PAGESIZE &
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 
 	warmup $!
 	if [ $? -ne 0 ]; then
@@ -399,7 +400,7 @@ test_move_charge()
 
 	tst_resm TINFO "Running memcg_process $1 -s $2"
 	memcg_process $1 -s $2 &
-	sleep 1
+	TST_CHECKPOINT_WAIT 0
 	warmup $!
 	if [ $? -ne 0 ]; then
 		rmdir subgroup_a
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_process.c b/testcases/kernel/controllers/memcg/functional/memcg_process.c
index 4ad2d6f..fbb76e2 100644
--- a/testcases/kernel/controllers/memcg/functional/memcg_process.c
+++ b/testcases/kernel/controllers/memcg/functional/memcg_process.c
@@ -34,6 +34,8 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
 
 static int fd;
 
@@ -315,6 +317,10 @@ int main(int argc, char *argv[])
 
 	process_options(argc, argv);
 
+	tst_reinit();
+
+	TST_CHECKPOINT_WAKE(0);
+
 	while (!flag_exit)
 		sleep(1);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed
  2016-06-14 15:24   ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Stanislav Kholmanskikh
@ 2016-06-14 15:24     ` Stanislav Kholmanskikh
  2016-06-22 13:51       ` Cyril Hrubis
  2016-06-22 13:38     ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Cyril Hrubis
  2016-06-22 13:40     ` Cyril Hrubis
  2 siblings, 1 reply; 10+ messages in thread
From: Stanislav Kholmanskikh @ 2016-06-14 15:24 UTC (permalink / raw)
  To: ltp

On some systems it may take slightly more than one second
to kill the memcg_process. So let's check several times if the
process is alive.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
Changes since [1]:
 * 'sleep 1' next to memcg_process & is substituted by patch 3
   of the series
 * use /proc/pid and /proc/pid/status to check wheter the process
   exists and if it exists whether it's in Z state


[1] http://lists.linux.it/pipermail/ltp/2016-May/001840.html


 .../controllers/memcg/functional/memcg_lib.sh      |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index b191ea6..46bad42 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -251,10 +251,19 @@ test_proc_kill()
 	echo $pid > tasks
 
 	kill -s USR1 $pid 2> /dev/null
-	sleep 1
 
-	ps -p $pid > /dev/null 2> /dev/null
-	if [ $? -ne 0 ]; then
+	tpk_pid_exists=1
+	for tpk_iter in $(seq 20); do
+		if [ ! -d "/proc/$pid" ] ||
+			grep -q 'Z (zombie)' "/proc/$pid/status"; then
+			tpk_pid_exists=0
+			break
+		fi
+
+		tst_sleep 250ms
+	done
+
+	if [ $tpk_pid_exists -eq 0 ]; then
 		wait $pid
 		ret=$?
 		if [ $ret -eq 1 ]; then
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 1/4] memcg/functional: rewrite
  2016-06-14 15:24 [LTP] [PATCH 1/4] memcg/functional: rewrite Stanislav Kholmanskikh
  2016-06-14 15:24 ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Stanislav Kholmanskikh
@ 2016-06-22 13:34 ` Cyril Hrubis
  2016-08-19 14:11   ` Stanislav Kholmanskikh
  1 sibling, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2016-06-22 13:34 UTC (permalink / raw)
  To: ltp

Hi!
>  # Record the test result of a test case
>  # $1 - The result of the test case, $PASS or $FAIL
> @@ -55,7 +82,6 @@ result()
>  		tst_resm TPASS "$info"
>  	else
>  		tst_resm TFAIL "$info"
> -		: $(( failed += 1 ))
>  	fi

Can we get rid of the result() function?

Since the failures are now counted in the test.sh library it does not
make a sense to define special result reporting function.

> @@ -83,7 +109,7 @@ warmup()
>  {
>  	pid=$1
>  
> -	echo "Warming up for test: $cur_id, pid: $pid"
> +	tst_resm TINFO "Warming up pid: $pid"
>  	kill -s USR1 $pid 2> /dev/null
>  	sleep 1
>  	kill -s USR1 $pid 2> /dev/null
> @@ -91,10 +117,10 @@ warmup()
>  
>  	kill -0 $pid
>  	if [ $? -ne 0 ]; then
> -		result $FAIL "cur_id=$cur_id"
> +		result $FAIL ""
                              ^
Shouldn't we print here something as:

"process died after warmup"

Or even better wait the pid and print the exit value as well.


Otherwise this looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup
  2016-06-14 15:24 ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Stanislav Kholmanskikh
  2016-06-14 15:24   ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Stanislav Kholmanskikh
@ 2016-06-22 13:35   ` Cyril Hrubis
  1 sibling, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2016-06-22 13:35 UTC (permalink / raw)
  To: ltp

Hi!
This one is obviously OK.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/4] memcg/functional: use checkpoints
  2016-06-14 15:24   ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Stanislav Kholmanskikh
  2016-06-14 15:24     ` [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed Stanislav Kholmanskikh
@ 2016-06-22 13:38     ` Cyril Hrubis
  2016-06-22 13:40     ` Cyril Hrubis
  2 siblings, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2016-06-22 13:38 UTC (permalink / raw)
  To: ltp

Hi!
> This patch implements the idea proposed in [1], [2], that
> we use the checkpoint interface to let memcg_process inform
> the parent process that it's ready.
> 
> In my enviroment it saves ~50 secs of time.

Nice :)

BTW, we should also change the test so that the allocation is not done
from the signal handlers, but that can be done later.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/4] memcg/functional: use checkpoints
  2016-06-14 15:24   ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Stanislav Kholmanskikh
  2016-06-14 15:24     ` [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed Stanislav Kholmanskikh
  2016-06-22 13:38     ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Cyril Hrubis
@ 2016-06-22 13:40     ` Cyril Hrubis
  2 siblings, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2016-06-22 13:40 UTC (permalink / raw)
  To: ltp

Hi!
> This patch implements the idea proposed in [1], [2], that
> we use the checkpoint interface to let memcg_process inform
> the parent process that it's ready.

Also you probably forget to add links for [1] and [2] here.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed
  2016-06-14 15:24     ` [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed Stanislav Kholmanskikh
@ 2016-06-22 13:51       ` Cyril Hrubis
  0 siblings, 0 replies; 10+ messages in thread
From: Cyril Hrubis @ 2016-06-22 13:51 UTC (permalink / raw)
  To: ltp

Hi!
Looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH 1/4] memcg/functional: rewrite
  2016-06-22 13:34 ` [LTP] [PATCH 1/4] memcg/functional: rewrite Cyril Hrubis
@ 2016-08-19 14:11   ` Stanislav Kholmanskikh
  0 siblings, 0 replies; 10+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-19 14:11 UTC (permalink / raw)
  To: ltp

Hi!

On 06/22/2016 04:34 PM, Cyril Hrubis wrote:
> Hi!
>>  # Record the test result of a test case
>>  # $1 - The result of the test case, $PASS or $FAIL
>> @@ -55,7 +82,6 @@ result()
>>  		tst_resm TPASS "$info"
>>  	else
>>  		tst_resm TFAIL "$info"
>> -		: $(( failed += 1 ))
>>  	fi
> 
> Can we get rid of the result() function?
> 
> Since the failures are now counted in the test.sh library it does not
> make a sense to define special result reporting function.

Yes, we can.

One thing to notice that in the current code there are many places like
this:

echo 1.0 > memory.limit_in_bytes 2> /dev/null
result $(( !($? != 0) )) "return value is $?"

I don't think it will be a good idea to transform them all to:

echo 1.0 > memory.limit_in_bytes 2> /dev/null
if [ $? -ne 0 ]; then
   tst_resm TPASS "return value is $?"
else
   tst_resm TFAIL "return value is 0"
fi

A possible solution could be using help functions similar to ROD():

SHOULD_FAIL echo 1.0 \> memory.limit_in_bytes

which will output:

TPASS: echo 1.0 > memory.limit_in_bytes failed as expected

I have an RFC patch for that. I'll send it shortly.

> 
>> @@ -83,7 +109,7 @@ warmup()
>>  {
>>  	pid=$1
>>  
>> -	echo "Warming up for test: $cur_id, pid: $pid"
>> +	tst_resm TINFO "Warming up pid: $pid"
>>  	kill -s USR1 $pid 2> /dev/null
>>  	sleep 1
>>  	kill -s USR1 $pid 2> /dev/null
>> @@ -91,10 +117,10 @@ warmup()
>>  
>>  	kill -0 $pid
>>  	if [ $? -ne 0 ]; then
>> -		result $FAIL "cur_id=$cur_id"
>> +		result $FAIL ""
>                               ^
> Shouldn't we print here something as:
> 
> "process died after warmup"
> 
> Or even better wait the pid and print the exit value as well.

Ok. Will do.

> 
> 
> Otherwise this looks fine.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-08-19 14:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-14 15:24 [LTP] [PATCH 1/4] memcg/functional: rewrite Stanislav Kholmanskikh
2016-06-14 15:24 ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Stanislav Kholmanskikh
2016-06-14 15:24   ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Stanislav Kholmanskikh
2016-06-14 15:24     ` [LTP] [PATCH V3 4/4] memcg/functional: check several times if the process is killed Stanislav Kholmanskikh
2016-06-22 13:51       ` Cyril Hrubis
2016-06-22 13:38     ` [LTP] [PATCH 3/4] memcg/functional: use checkpoints Cyril Hrubis
2016-06-22 13:40     ` Cyril Hrubis
2016-06-22 13:35   ` [LTP] [PATCH 2/4] memcg/functional/memcg_process: cleanup Cyril Hrubis
2016-06-22 13:34 ` [LTP] [PATCH 1/4] memcg/functional: rewrite Cyril Hrubis
2016-08-19 14:11   ` Stanislav Kholmanskikh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.