* [LTP] [PATCH v3 0/1] memcg_stress newlib porting @ 2019-01-04 10:54 Cristian Marussi 2019-01-04 10:54 ` [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib Cristian Marussi 0 siblings, 1 reply; 11+ messages in thread From: Cristian Marussi @ 2019-01-04 10:54 UTC (permalink / raw) To: ltp This converts memcg stress tests and its helper to LTP newlib and performs a general cleanup. This patch is dependent on [1] since it factors out a common helper to cgroup_lib.sh (introduced in [1]) Changes ------- v2->v3: - cleaned up cgroup_lib.sh helper v1->v2: - Restored the original usage of SIGKILL to terminate memcp_process_stress helpers after having a look at what said in previous commit: 9b9a6bb10258bc975bd1707b418699264d3bdcef Cristian Marussi (1): [LTP] memcg_stress_test.sh: ported to newlib [1]: http://lists.linux.it/pipermail/ltp/2018-December/010385.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-01-04 10:54 [LTP] [PATCH v3 0/1] memcg_stress newlib porting Cristian Marussi @ 2019-01-04 10:54 ` Cristian Marussi 2019-01-28 16:32 ` Petr Vorel 0 siblings, 1 reply; 11+ messages in thread From: Cristian Marussi @ 2019-01-04 10:54 UTC (permalink / raw) To: ltp Ported to newlib framework and general cleanup. Moved an helper function out into cgroup_lib.sh. Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> --- testcases/kernel/controllers/cgroup_lib.sh | 16 +++ .../kernel/controllers/memcg/stress/Makefile | 21 +-- .../memcg/stress/memcg_process_stress.c | 34 ++--- .../memcg/stress/memcg_stress_test.sh | 135 ++++++++---------- 4 files changed, 92 insertions(+), 114 deletions(-) diff --git a/testcases/kernel/controllers/cgroup_lib.sh b/testcases/kernel/controllers/cgroup_lib.sh index 8c19c6c3e..dbf5199e6 100644 --- a/testcases/kernel/controllers/cgroup_lib.sh +++ b/testcases/kernel/controllers/cgroup_lib.sh @@ -21,3 +21,19 @@ get_cgroup_mountpoint() echo $mntpoint return 0 } + +# Check if given subsystem is supported and enabled +# is_cgroup_subsystem_available_and_enabled SUBSYSTEM +# RETURN: 0 if subsystem supported and enabled, otherwise 1 +is_cgroup_subsystem_available_and_enabled() +{ + local val + local subsystem=$1 + + [ $# -eq 0 ] && tst_brk TBROK "is_cgroup_subsystem_available_and_enabled: subsystem not defined" + + val=$(grep -w $subsystem /proc/cgroups | awk '{ print $4 }') + [ "x$val" = "x1" ] && return 0 + + return 1 +} diff --git a/testcases/kernel/controllers/memcg/stress/Makefile b/testcases/kernel/controllers/memcg/stress/Makefile index a0456ac5d..773363cfc 100644 --- a/testcases/kernel/controllers/memcg/stress/Makefile +++ b/testcases/kernel/controllers/memcg/stress/Makefile @@ -1,24 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2009, Cisco Systems Inc. +# Author: Ngie Cooper, September 2009 # # kernel/controllers/memcg/stress testcase 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, September 2009 -# top_srcdir ?= ../../../../.. diff --git a/testcases/kernel/controllers/memcg/stress/memcg_process_stress.c b/testcases/kernel/controllers/memcg/stress/memcg_process_stress.c index 870575c26..6af550012 100644 --- a/testcases/kernel/controllers/memcg/stress/memcg_process_stress.c +++ b/testcases/kernel/controllers/memcg/stress/memcg_process_stress.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/mman.h> #include <err.h> @@ -91,14 +76,15 @@ int main(int argc, char *argv[]) if (interval <= 0) interval = 1; - /* TODO (garrcoop): add error handling. */ memset(&sigint_action, 0, sizeof(sigint_action)); sigint_action.sa_handler = &sigint_handler; - sigaction(SIGINT, &sigint_action, NULL); + if (sigaction(SIGINT, &sigint_action, NULL)) + err(1, "sigaction(%s) failed", "SIGINT"); memset(&sigusr_action, 0, sizeof(sigusr_action)); sigusr_action.sa_handler = &sigusr_handler; - sigaction(SIGUSR1, &sigusr_action, NULL); + if (sigaction(SIGUSR1, &sigusr_action, NULL)) + err(1, "sigaction(%s) failed", "SIGUSR1"); while (!flag_exit) { sleep(interval); diff --git a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh index af1a708a8..652d99e55 100755 --- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh +++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh @@ -1,56 +1,56 @@ -#! /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> ## -## Restructure for LTP: Shi Weihua <shiwh@cn.fujitsu.com> ## -## Added memcg enable/disable functinality: Rishikesh K Rajak ## -## <risrajak@linux.vnet.ibm.com ## -## ## -################################################################################ - -cd $LTPROOT/testcases/bin -export TCID="memcg_stress_test" -export TST_TOTAL=2 -export TST_COUNT=0 - -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 -fi - -RUN_TIME=$(( 15 * 60 )) - -cleanup() +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) 2009 FUJITSU LIMITED +# Copyright (c) 2018-2019 ARM Ltd. All Rights Reserved. +# +# Author: Li Zefan <lizf@cn.fujitsu.com> +# Restructure for LTP: Shi Weihua <shiwh@cn.fujitsu.com> +# Added memcg enable/disable functionality: Rishikesh K Rajak <risrajak@linux.vnet.ibm.com + +TST_TESTFUNC=testcase_ +TST_SETUP=do_setup +TST_CLEANUP=do_cleanup +TST_CNT=2 +TST_NEEDS_ROOT=1 +TST_NEEDS_CMDS="mount umount cat kill mkdir rmdir grep awk cut" + +# Each test case runs for 900 secs when everything fine... +# ...so the default 5mins timeout is not enough. +LTP_TIMEOUT_MUL=7 + +. tst_test.sh +. cgroup_lib.sh + +do_setup() { - if [ -e /dev/memcg ]; then - umount /dev/memcg 2>/dev/null - rmdir /dev/memcg 2>/dev/null + if ! is_cgroup_subsystem_available_and_enabled "memory";then + tst_res TWARN "Either Kernel does not support MEMORY resource controller or feature not enabled" + tst_brk TCONF ignored "Skipping all memory cgroup testcases...." fi + + echo 3 > /proc/sys/vm/drop_caches + sleep 2 + local mem_free=`cat /proc/meminfo | grep MemFree | awk '{ print $2 }'` + local swap_free=`cat /proc/meminfo | grep SwapFree | awk '{ print $2 }'` + + MEM=$(( $mem_free + $swap_free / 2 )) + MEM=$(( $MEM / 1024 )) + RUN_TIME=$(( 15 * 60 )) + + tst_res TINFO "Calculated available memory $MEM MB" } +do_cleanup() +{ + if [ -e /dev/memcg ]; then + umount /dev/memcg 2> /dev/null + rmdir /dev/memcg 2> /dev/null + fi +} do_mount() { - cleanup; + do_cleanup mkdir /dev/memcg 2> /dev/null mount -t cgroup -omemory memcg /dev/memcg @@ -65,62 +65,53 @@ do_mount() # $4 - How long does this test run ? in second run_stress() { - do_mount; + local i + + do_mount for i in $(seq 0 $(($1-1))) do mkdir /dev/memcg/$i 2> /dev/null - ./memcg_process_stress $2 $3 & - eval pid$i=$! + memcg_process_stress $2 $3 & + eval local pid$i=$! eval echo \$pid$i > /dev/memcg/$i/tasks done for i in $(seq 0 $(($1-1))) do - eval /bin/kill -s SIGUSR1 \$pid$i 2> /dev/null + eval kill -USR1 \$pid$i 2> /dev/null done sleep $4 for i in $(seq 0 $(($1-1))) do - eval /bin/kill -s SIGKILL \$pid$i 2> /dev/null + eval kill -KILL \$pid$i 2> /dev/null eval wait \$pid$i rmdir /dev/memcg/$i 2> /dev/null done - cleanup; + do_cleanup } testcase_1() { - run_stress 150 $(( ($mem-150) / 150 )) 5 $RUN_TIME + tst_res TINFO "testcase 1 started...it will run for $RUN_TIME secs" + + run_stress 150 $(( ($MEM - 150) / 150 )) 5 $RUN_TIME - tst_resm TPASS "stress test 1 passed" + tst_res TPASS "stress test 1 passed" } testcase_2() { - run_stress 1 $mem 5 $RUN_TIME + tst_res TINFO "testcase 2 started...it will run for $RUN_TIME secs" - tst_resm TPASS "stress test 2 passed" -} - -echo 3 > /proc/sys/vm/drop_caches -sleep 2 -mem_free=`cat /proc/meminfo | grep MemFree | awk '{ print $2 }'` -swap_free=`cat /proc/meminfo | grep SwapFree | awk '{ print $2 }'` + run_stress 1 $MEM 5 $RUN_TIME -mem=$(( $mem_free + $swap_free / 2 )) -mem=$(( mem / 1024 )) - -date -export TST_COUNT=$(( $TST_COUNT + 1 )) -testcase_1 -export TST_COUNT=$(( $TST_COUNT + 1 )) -testcase_2 -date + tst_res TPASS "stress test 2 passed" +} -exit 0 +tst_run -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-01-04 10:54 ` [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib Cristian Marussi @ 2019-01-28 16:32 ` Petr Vorel 2019-01-29 17:50 ` Petr Vorel 0 siblings, 1 reply; 11+ messages in thread From: Petr Vorel @ 2019-01-28 16:32 UTC (permalink / raw) To: ltp Hi Cristian, thanks for your patch. I was thinking just to fix minor formatting issues, but eval in run_stress() deserve to be rewritten. > Ported to newlib framework and general cleanup. > Moved an helper function out into cgroup_lib.sh. .. > + [ "x$val" = "x1" ] && return 0 drop x, it's portable enough like this:: [ "$val" = "1" ] && return 0 ... > +++ b/testcases/kernel/controllers/memcg/stress/Makefile > @@ -1,24 +1,9 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (C) 2009, Cisco Systems Inc. > +# Author: Ngie Cooper, September 2009 > # kernel/controllers/memcg/stress testcase suite Makefile. Delete this as well. > +++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh ... > + if ! is_cgroup_subsystem_available_and_enabled "memory";then > + tst_res TWARN "Either Kernel does not support MEMORY resource controller or feature not enabled" > + tst_brk TCONF ignored "Skipping all memory cgroup testcases...." TWARN followed by TCONF does not make sense. Also skip is redundant. Please, avoid using dots (even in comments). This is enough: if ! is_cgroup_subsystem_available_and_enabled "memory"; then tst_res TCONF "Either Kernel does not support MEMORY resource controller or feature not enabled" fi ... > + > + echo 3 > /proc/sys/vm/drop_caches > + sleep 2 > + local mem_free=`cat /proc/meminfo | grep MemFree | awk '{ print $2 }'` > + local swap_free=`cat /proc/meminfo | grep SwapFree | awk '{ print $2 }'` Test started using awk, which might not be everywhere (even it's quite basic. TST_NEEDS_CMDS="awk" would be good for tests. And also TST_NEEDS_CMDS="$TST_NEEDS_CMDS awk" would be good for cgroup_lib.sh. > + > + MEM=$(( $mem_free + $swap_free / 2 )) > + MEM=$(( $MEM / 1024 )) > + RUN_TIME=$(( 15 * 60 )) > + > + tst_res TINFO "Calculated available memory $MEM MB" > } > +do_cleanup() > +{ > + if [ -e /dev/memcg ]; then > + umount /dev/memcg 2> /dev/null > + rmdir /dev/memcg 2> /dev/null > + fi > +} > do_mount() > { > - cleanup; > + do_cleanup > mkdir /dev/memcg 2> /dev/null > mount -t cgroup -omemory memcg /dev/memcg > @@ -65,62 +65,53 @@ do_mount() > # $4 - How long does this test run ? in second > run_stress() > { > - do_mount; > + local i > + > + do_mount > for i in $(seq 0 $(($1-1))) > do Please keep do on the same line (in memcg_stress_test.sh on several times): for i in $(seq 0 $(($1-1))); do > mkdir /dev/memcg/$i 2> /dev/null > - ./memcg_process_stress $2 $3 & > - eval pid$i=$! Eval is evil. And eval with local is even more evil. Even if it works on dash, using eval in whole function is just bad. Could you please rewrite it into single variable? You can just separate pids with space. + maybe describe variables instead of using positional variables, e.g.: local cgroups="$1" local mem="$2" ... > + memcg_process_stress $2 $3 & > + eval local pid$i=$! > eval echo \$pid$i > /dev/memcg/$i/tasks > done > for i in $(seq 0 $(($1-1))) > do > - eval /bin/kill -s SIGUSR1 \$pid$i 2> /dev/null > + eval kill -USR1 \$pid$i 2> /dev/null > done > sleep $4 > for i in $(seq 0 $(($1-1))) > do > - eval /bin/kill -s SIGKILL \$pid$i 2> /dev/null > + eval kill -KILL \$pid$i 2> /dev/null > eval wait \$pid$i > rmdir /dev/memcg/$i 2> /dev/null > done > - cleanup; > + do_cleanup > } > testcase_1() I'd rename it to test1. > { > - run_stress 150 $(( ($mem-150) / 150 )) 5 $RUN_TIME > + tst_res TINFO "testcase 1 started...it will run for $RUN_TIME secs" Can you avoid dots, will? Maybe describe it a bit. I'd print test info in run_stress(), something like this: tst_res TINFO "Testing $cgroups cgroups, using $mem, run for $RUN_TIME secs" Kind regards, Petr ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-01-28 16:32 ` Petr Vorel @ 2019-01-29 17:50 ` Petr Vorel 2019-01-29 19:25 ` Cristian Marussi 0 siblings, 1 reply; 11+ messages in thread From: Petr Vorel @ 2019-01-29 17:50 UTC (permalink / raw) To: ltp Hi Cristian, I suggest following cleanup of your patch (see bellow). This change, with my other cleanup patch [1] + your patch [2] can be seen on [3]. Kind regards, Petr [1] https://patchwork.ozlabs.org/patch/1032923/ [2] https://patchwork.ozlabs.org/patch/1020887/ [3] https://github.com/pevik/ltp/commits/christian/memcg_stress_test.sh.v3.fixes diff --git testcases/kernel/controllers/cgroup_lib.sh testcases/kernel/controllers/cgroup_lib.sh index c164932fa..7918b5636 100644 --- testcases/kernel/controllers/cgroup_lib.sh +++ testcases/kernel/controllers/cgroup_lib.sh @@ -33,7 +33,7 @@ is_cgroup_subsystem_available_and_enabled() [ $# -eq 0 ] && tst_brk TBROK "is_cgroup_subsystem_available_and_enabled: subsystem not defined" val=$(grep -w $subsystem /proc/cgroups | awk '{ print $4 }') - [ "x$val" = "x1" ] && return 0 + [ "$val" = "1" ] && return 0 return 1 } diff --git testcases/kernel/controllers/memcg/stress/Makefile testcases/kernel/controllers/memcg/stress/Makefile index 773363cfc..a9678bf3b 100644 --- testcases/kernel/controllers/memcg/stress/Makefile +++ testcases/kernel/controllers/memcg/stress/Makefile @@ -1,9 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (C) 2009, Cisco Systems Inc. # Author: Ngie Cooper, September 2009 -# -# kernel/controllers/memcg/stress testcase suite Makefile. -# top_srcdir ?= ../../../../.. diff --git testcases/kernel/controllers/memcg/stress/memcg_process_stress.c testcases/kernel/controllers/memcg/stress/memcg_process_stress.c index 6af550012..422deaeee 100644 --- testcases/kernel/controllers/memcg/stress/memcg_process_stress.c +++ testcases/kernel/controllers/memcg/stress/memcg_process_stress.c @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) 2009 FUJITSU LIMITED - * * Author: Li Zefan <lizf@cn.fujitsu.com> */ diff --git testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh index 652d99e55..9972b6c45 100755 --- testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh +++ testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh @@ -7,25 +7,23 @@ # Restructure for LTP: Shi Weihua <shiwh@cn.fujitsu.com> # Added memcg enable/disable functionality: Rishikesh K Rajak <risrajak@linux.vnet.ibm.com -TST_TESTFUNC=testcase_ -TST_SETUP=do_setup -TST_CLEANUP=do_cleanup +TST_TESTFUNC=test +TST_SETUP=setup +TST_CLEANUP=cleanup TST_CNT=2 TST_NEEDS_ROOT=1 TST_NEEDS_CMDS="mount umount cat kill mkdir rmdir grep awk cut" -# Each test case runs for 900 secs when everything fine... -# ...so the default 5mins timeout is not enough. +# Each test case runs for 900 secs when everything fine +# therefore the default 5 mins timeout is not enough. LTP_TIMEOUT_MUL=7 -. tst_test.sh . cgroup_lib.sh -do_setup() +setup() { - if ! is_cgroup_subsystem_available_and_enabled "memory";then - tst_res TWARN "Either Kernel does not support MEMORY resource controller or feature not enabled" - tst_brk TCONF ignored "Skipping all memory cgroup testcases...." + if ! is_cgroup_subsystem_available_and_enabled "memory"; then + tst_brk TCONF "Either kernel does not support Memory Resource Controller or feature not enabled" fi echo 3 > /proc/sys/vm/drop_caches @@ -40,7 +38,7 @@ do_setup() tst_res TINFO "Calculated available memory $MEM MB" } -do_cleanup() +cleanup() { if [ -e /dev/memcg ]; then umount /dev/memcg 2> /dev/null @@ -50,15 +48,12 @@ do_cleanup() do_mount() { - do_cleanup + cleanup mkdir /dev/memcg 2> /dev/null mount -t cgroup -omemory memcg /dev/memcg } - -# Run the stress test -# # $1 - Number of cgroups # $2 - Allocated how much memory in one process? in MB # $3 - The interval to touch memory in a process @@ -69,34 +64,31 @@ run_stress() do_mount - for i in $(seq 0 $(($1-1))) - do + for i in $(seq 0 $(($1-1))); do mkdir /dev/memcg/$i 2> /dev/null memcg_process_stress $2 $3 & - eval local pid$i=$! + eval pid$i=$! eval echo \$pid$i > /dev/memcg/$i/tasks done - for i in $(seq 0 $(($1-1))) - do + for i in $(seq 0 $(($1-1))); do eval kill -USR1 \$pid$i 2> /dev/null done sleep $4 - for i in $(seq 0 $(($1-1))) - do + for i in $(seq 0 $(($1-1))); do eval kill -KILL \$pid$i 2> /dev/null eval wait \$pid$i rmdir /dev/memcg/$i 2> /dev/null done - do_cleanup + cleanup } -testcase_1() +test1() { tst_res TINFO "testcase 1 started...it will run for $RUN_TIME secs" @@ -105,7 +97,7 @@ testcase_1() tst_res TPASS "stress test 1 passed" } -testcase_2() +test2() { tst_res TINFO "testcase 2 started...it will run for $RUN_TIME secs" ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-01-29 17:50 ` Petr Vorel @ 2019-01-29 19:25 ` Cristian Marussi 2019-02-01 13:45 ` Petr Vorel 0 siblings, 1 reply; 11+ messages in thread From: Cristian Marussi @ 2019-01-29 19:25 UTC (permalink / raw) To: ltp Hi On 29/01/2019 17:50, Petr Vorel wrote:> Hi Cristian, > > I suggest following cleanup of your patch (see bellow). > > This change, with my other cleanup patch [1] + your patch [2] can be seen on [3]. > > Kind regards, > Petr Thanks for the review and the cleanup. (next I was going to rework following your today's advises, but you've been faster...) I'll test asap. Thanks Regards Cristian ________________________________ From: Petr Vorel <pvorel@suse.cz> Sent: 29 January 2019 17:50:19 To: Cristian Marussi Cc: ltp@lists.linux.it Subject: Re: [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib Hi Cristian, I suggest following cleanup of your patch (see bellow). This change, with my other cleanup patch [1] + your patch [2] can be seen on [3]. Kind regards, Petr [1] https://patchwork.ozlabs.org/patch/1032923/ [2] https://patchwork.ozlabs.org/patch/1020887/ [3] https://github.com/pevik/ltp/commits/christian/memcg_stress_test.sh.v3.fixes diff --git testcases/kernel/controllers/cgroup_lib.sh testcases/kernel/controllers/cgroup_lib.sh index c164932fa..7918b5636 100644 --- testcases/kernel/controllers/cgroup_lib.sh +++ testcases/kernel/controllers/cgroup_lib.sh @@ -33,7 +33,7 @@ is_cgroup_subsystem_available_and_enabled() [ $# -eq 0 ] && tst_brk TBROK "is_cgroup_subsystem_available_and_enabled: subsystem not defined" val=$(grep -w $subsystem /proc/cgroups | awk '{ print $4 }') - [ "x$val" = "x1" ] && return 0 + [ "$val" = "1" ] && return 0 return 1 } diff --git testcases/kernel/controllers/memcg/stress/Makefile testcases/kernel/controllers/memcg/stress/Makefile index 773363cfc..a9678bf3b 100644 --- testcases/kernel/controllers/memcg/stress/Makefile +++ testcases/kernel/controllers/memcg/stress/Makefile @@ -1,9 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (C) 2009, Cisco Systems Inc. # Author: Ngie Cooper, September 2009 -# -# kernel/controllers/memcg/stress testcase suite Makefile. -# top_srcdir ?= ../../../../.. diff --git testcases/kernel/controllers/memcg/stress/memcg_process_stress.c testcases/kernel/controllers/memcg/stress/memcg_process_stress.c index 6af550012..422deaeee 100644 --- testcases/kernel/controllers/memcg/stress/memcg_process_stress.c +++ testcases/kernel/controllers/memcg/stress/memcg_process_stress.c @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) 2009 FUJITSU LIMITED - * * Author: Li Zefan <lizf@cn.fujitsu.com> */ diff --git testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh index 652d99e55..9972b6c45 100755 --- testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh +++ testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh @@ -7,25 +7,23 @@ # Restructure for LTP: Shi Weihua <shiwh@cn.fujitsu.com> # Added memcg enable/disable functionality: Rishikesh K Rajak <risrajak@linux.vnet.ibm.com -TST_TESTFUNC=testcase_ -TST_SETUP=do_setup -TST_CLEANUP=do_cleanup +TST_TESTFUNC=test +TST_SETUP=setup +TST_CLEANUP=cleanup TST_CNT=2 TST_NEEDS_ROOT=1 TST_NEEDS_CMDS="mount umount cat kill mkdir rmdir grep awk cut" -# Each test case runs for 900 secs when everything fine... -# ...so the default 5mins timeout is not enough. +# Each test case runs for 900 secs when everything fine +# therefore the default 5 mins timeout is not enough. LTP_TIMEOUT_MUL=7 -. tst_test.sh . cgroup_lib.sh -do_setup() +setup() { - if ! is_cgroup_subsystem_available_and_enabled "memory";then - tst_res TWARN "Either Kernel does not support MEMORY resource controller or feature not enabled" - tst_brk TCONF ignored "Skipping all memory cgroup testcases...." + if ! is_cgroup_subsystem_available_and_enabled "memory"; then + tst_brk TCONF "Either kernel does not support Memory Resource Controller or feature not enabled" fi echo 3 > /proc/sys/vm/drop_caches @@ -40,7 +38,7 @@ do_setup() tst_res TINFO "Calculated available memory $MEM MB" } -do_cleanup() +cleanup() { if [ -e /dev/memcg ]; then umount /dev/memcg 2> /dev/null @@ -50,15 +48,12 @@ do_cleanup() do_mount() { - do_cleanup + cleanup mkdir /dev/memcg 2> /dev/null mount -t cgroup -omemory memcg /dev/memcg } - -# Run the stress test -# # $1 - Number of cgroups # $2 - Allocated how much memory in one process? in MB # $3 - The interval to touch memory in a process @@ -69,34 +64,31 @@ run_stress() do_mount - for i in $(seq 0 $(($1-1))) - do + for i in $(seq 0 $(($1-1))); do mkdir /dev/memcg/$i 2> /dev/null memcg_process_stress $2 $3 & - eval local pid$i=$! + eval pid$i=$! eval echo \$pid$i > /dev/memcg/$i/tasks done - for i in $(seq 0 $(($1-1))) - do + for i in $(seq 0 $(($1-1))); do eval kill -USR1 \$pid$i 2> /dev/null done sleep $4 - for i in $(seq 0 $(($1-1))) - do + for i in $(seq 0 $(($1-1))); do eval kill -KILL \$pid$i 2> /dev/null eval wait \$pid$i rmdir /dev/memcg/$i 2> /dev/null done - do_cleanup + cleanup } -testcase_1() +test1() { tst_res TINFO "testcase 1 started...it will run for $RUN_TIME secs" @@ -105,7 +97,7 @@ testcase_1() tst_res TPASS "stress test 1 passed" } -testcase_2() +test2() { tst_res TINFO "testcase 2 started...it will run for $RUN_TIME secs" IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20190129/95a19364/attachment.html> ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-01-29 19:25 ` Cristian Marussi @ 2019-02-01 13:45 ` Petr Vorel 2019-02-04 11:56 ` Cristian Marussi 0 siblings, 1 reply; 11+ messages in thread From: Petr Vorel @ 2019-02-01 13:45 UTC (permalink / raw) To: ltp Hi Cristian, > Thanks for the review and the cleanup. (next I was going to rework following your today's advises, but you've been faster...) I'm sorry, I'll be patient next time :). I was thinking to add these changes to your commit and merge patchset, but as I introduced 2 regressions in your patchset last time (I test it of course), I was careful. > I'll test asap. Thanks a lot! Kind regards, Petr ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-02-01 13:45 ` Petr Vorel @ 2019-02-04 11:56 ` Cristian Marussi 2019-02-04 23:34 ` Petr Vorel 0 siblings, 1 reply; 11+ messages in thread From: Cristian Marussi @ 2019-02-04 11:56 UTC (permalink / raw) To: ltp Hi On 01/02/2019 13:45, Petr Vorel wrote: > Hi Cristian, > >> Thanks for the review and the cleanup. (next I was going to rework following your today's advises, but you've been faster...) > I'm sorry, I'll be patient next time :). > I was thinking to add these changes to your commit and merge patchset, > but as I introduced 2 regressions in your patchset last time (I test it of > course), I was careful. > >> I'll test asap. > Thanks a lot! Tested from you branch with ToT as: 10df48aa7 (HEAD -> pevik_ltp/christian/memcg_stress_test.sh.v3.fixes) memcg_stress_test.sh: fix memory usage ebd6efaa6 memcg_stress_test.sh: Further cleanup 02c520928 memcg_stress_test.sh: ported to newlib fc544f842 controllers/mem_process.c: Fix comparison warning ... on 4k/64k pages defocnfig....looks fine to me. Thanks a lot for the cleanup. Regards Cristian > > > Kind regards, > Petr > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-02-04 11:56 ` Cristian Marussi @ 2019-02-04 23:34 ` Petr Vorel 2019-02-05 17:46 ` Cristian Marussi 2019-02-14 8:32 ` Xiao Yang 0 siblings, 2 replies; 11+ messages in thread From: Petr Vorel @ 2019-02-04 23:34 UTC (permalink / raw) To: ltp Hi Cristian, > Tested from you branch with ToT as: > 10df48aa7 (HEAD -> pevik_ltp/christian/memcg_stress_test.sh.v3.fixes) > memcg_stress_test.sh: fix memory usage > ebd6efaa6 memcg_stress_test.sh: Further cleanup > 02c520928 memcg_stress_test.sh: ported to newlib > fc544f842 controllers/mem_process.c: Fix comparison warning > ... > on 4k/64k pages defocnfig....looks fine to me. > Thanks a lot for the cleanup. Thanks a lot for testing! Pushed (with your Tested-by on my commit). I did few cleanup changes in my commit (TINFO messages, fix missing local pid, redirect stderr for wait to make it quiet, move TPASS message into run_stress()). I wonder how to suppress kill message for the first cgroup pid: /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 14750 User defined signal 1 memcg_process_stress $mem_size $interval This pid is the only one killed by -USR1, but line 65 is not in fist kill loop. Therefore it cannot be killed by -KILL (second kill loop). Kind regards, Petr ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-02-04 23:34 ` Petr Vorel @ 2019-02-05 17:46 ` Cristian Marussi 2019-02-14 8:32 ` Xiao Yang 1 sibling, 0 replies; 11+ messages in thread From: Cristian Marussi @ 2019-02-05 17:46 UTC (permalink / raw) To: ltp Hi Petr On 04/02/2019 23:34, Petr Vorel wrote: > Hi Cristian, > >> Tested from you branch with ToT as: > >> 10df48aa7 (HEAD -> pevik_ltp/christian/memcg_stress_test.sh.v3.fixes) >> memcg_stress_test.sh: fix memory usage >> ebd6efaa6 memcg_stress_test.sh: Further cleanup >> 02c520928 memcg_stress_test.sh: ported to newlib >> fc544f842 controllers/mem_process.c: Fix comparison warning >> ... > >> on 4k/64k pages defocnfig....looks fine to me. > >> Thanks a lot for the cleanup. > Thanks a lot for testing! > Pushed (with your Tested-by on my commit). > > I did few cleanup changes in my commit (TINFO messages, fix missing local pid, redirect > stderr for wait to make it quiet, move TPASS message into run_stress()). > > I wonder how to suppress kill message for the first cgroup pid: > /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 14750 User defined signal 1 memcg_process_stress $mem_size $interval > > This pid is the only one killed by -USR1, but line 65 is not in fist kill loop. > Therefore it cannot be killed by -KILL (second kill loop). > ? ... I saw no error messages in my testing :D Regards Cristian ^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-02-04 23:34 ` Petr Vorel 2019-02-05 17:46 ` Cristian Marussi @ 2019-02-14 8:32 ` Xiao Yang 2019-02-15 10:09 ` Cristian Marussi 1 sibling, 1 reply; 11+ messages in thread From: Xiao Yang @ 2019-02-14 8:32 UTC (permalink / raw) To: ltp On 2019/02/05 7:34, Petr Vorel wrote: > Hi Cristian, > >> Tested from you branch with ToT as: >> 10df48aa7 (HEAD -> pevik_ltp/christian/memcg_stress_test.sh.v3.fixes) >> memcg_stress_test.sh: fix memory usage >> ebd6efaa6 memcg_stress_test.sh: Further cleanup >> 02c520928 memcg_stress_test.sh: ported to newlib >> fc544f842 controllers/mem_process.c: Fix comparison warning >> ... >> on 4k/64k pages defocnfig....looks fine to me. >> Thanks a lot for the cleanup. > Thanks a lot for testing! > Pushed (with your Tested-by on my commit). > > I did few cleanup changes in my commit (TINFO messages, fix missing local pid, redirect > stderr for wait to make it quiet, move TPASS message into run_stress()). > > I wonder how to suppress kill message for the first cgroup pid: > /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 14750 User defined signal 1 memcg_process_stress $mem_size $interval Hi Petr, Cristian I also saw the same message occasionally when running memcg_stress_test, as below: ----------------------------------------------------------------------- ... /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 11262 User defined signal 1 memcg_process_stress $mem_size $interval /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 11264 User defined signal 1 memcg_process_stress $mem_size $interval /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 11267 User defined signal 1 memcg_process_stress $mem_size $interval ... ----------------------------------------------------------------------- It is possible for memcg_process_stress to be killed by SIGUSR1(default action), because it may have received SIGUSR1 before completing the specified initialization of SIGUSR1 signal action. Perpahs, we should sleep a few seconds to wait for the completion of initializing SIGUSR1 signal action. :-) Please see my patch for details: ----------------------------------------------------------------------- diff --git a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh index 5b19cc2..697a973 100755 --- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh +++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh @@ -81,6 +81,13 @@ run_stress() pids="$! $pids" done + # If memcg_process_stress has received SIGUSR1 before completing the + # specified initialization of SIGUSR1 signal action, memcg_process_stress + # will be killed by SIGUSR1(i.e. default action). So we should wait for + # the completion of initializing SIGUSR1 signal action by sleeping a few + # seconds. + sleep 2 + for pid in $pids; do kill -USR1 $pid 2> /dev/null done ----------------------------------------------------------------------- Best Regards, Xiao Yang > This pid is the only one killed by -USR1, but line 65 is not in fist kill loop. > Therefore it cannot be killed by -KILL (second kill loop). > > Kind regards, > Petr > > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib 2019-02-14 8:32 ` Xiao Yang @ 2019-02-15 10:09 ` Cristian Marussi 0 siblings, 0 replies; 11+ messages in thread From: Cristian Marussi @ 2019-02-15 10:09 UTC (permalink / raw) To: ltp Hi Xiao On 14/02/2019 08:32, Xiao Yang wrote: > On 2019/02/05 7:34, Petr Vorel wrote: >> Hi Cristian, > Hi Petr, Cristian > > I also saw the same message occasionally when running memcg_stress_test, > as below: > ----------------------------------------------------------------------- > ... > /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 11262 User defined > signal 1 memcg_process_stress $mem_size $interval > /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 11264 User defined > signal 1 memcg_process_stress $mem_size $interval > /opt/ltp/testcases/bin/memcg_stress_test.sh: line 65: 11267 User defined > signal 1 memcg_process_stress $mem_size $interval > ... > ----------------------------------------------------------------------- > > It is possible for memcg_process_stress to be killed by SIGUSR1(default > action), because it may > have received SIGUSR1 before completing the specified initialization of > SIGUSR1 signal action > > Perpahs, we should sleep a few seconds to wait for the completion of > initializing SIGUSR1 signal action. :-) Mmm...yes that's a possibility....but I was noticing that we spawn a lot of bg processes in a loop before starting signaling all of them in the following loop (i.e. after all the forks)...but I just realized than then we signal them starting from the most recently spawned...not the oldest (in reverse order)....so maybe it could be enough to start signalling from the oldest. Signaling in the natural-born order... diff --git a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh index 5b19cc292..aa91f77b2 100755 --- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh +++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh @@ -78,7 +78,7 @@ run_stress() mkdir /dev/memcg/$i 2> /dev/null memcg_process_stress $mem_size $interval & echo $! > /dev/memcg/$i/tasks - pids="$! $pids" + pids="$pids $!" done for pid in $pids; do I understand that this approach is NO more deterministic than your sleeping solution, anyway. IMHO the only real deterministic solution would be to somehow explicitly "sync" at the end of process-spawn loop, with the test script waiting for all the spawned child to be ready....but it is maybe a bit overkill for the issue we face. Maybe it is worth trying signaling in the correct order and then sleeping a bit and see if it solves...I haven't seen this issue at all on my local setup on ARM. Regards Thanks Cristian > > Please see my patch for details: > ----------------------------------------------------------------------- > diff --git > a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh > b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh > index 5b19cc2..697a973 100755 > --- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh > +++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh > @@ -81,6 +81,13 @@ run_stress() > pids="$! $pids" > done > > + # If memcg_process_stress has received SIGUSR1 before completing the > + # specified initialization of SIGUSR1 signal action, > memcg_process_stress > + # will be killed by SIGUSR1(i.e. default action). So we should > wait for > + # the completion of initializing SIGUSR1 signal action by > sleeping a few > + # seconds. > + sleep 2 > + > for pid in $pids; do > kill -USR1 $pid 2> /dev/null > done > ----------------------------------------------------------------------- > > Best Regards, > Xiao Yang > >> This pid is the only one killed by -USR1, but line 65 is not in fist kill loop. >> Therefore it cannot be killed by -KILL (second kill loop). >> >> Kind regards, >> Petr >> >> > > > ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-02-15 10:09 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-04 10:54 [LTP] [PATCH v3 0/1] memcg_stress newlib porting Cristian Marussi 2019-01-04 10:54 ` [LTP] [PATCH v3] memcg_stress_test.sh: ported to newlib Cristian Marussi 2019-01-28 16:32 ` Petr Vorel 2019-01-29 17:50 ` Petr Vorel 2019-01-29 19:25 ` Cristian Marussi 2019-02-01 13:45 ` Petr Vorel 2019-02-04 11:56 ` Cristian Marussi 2019-02-04 23:34 ` Petr Vorel 2019-02-05 17:46 ` Cristian Marussi 2019-02-14 8:32 ` Xiao Yang 2019-02-15 10:09 ` Cristian Marussi
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.