Some nits below: On Tue, Jul 1, 2014 at 5:35 AM, Lisa Nguyen <lisa.ngu...@linaro.org> wrote: > Run the cpuhotplug test suite on all cpus including cpu0. If we wish > to skip cpu0, pass the parameter, hotplug_allow_cpu0=1, to make check > like in these examples:
Umm, this description is the opposite of what the default code does. cpuhotplug testsuite does *not* run on cpu0 by default. If we wish to *allow* cpu0, we set hotplug_allow_cpu0 to 1. > sudo make -C cpuhotplug hotplug_allow_cpu0=1 check > sudo make hotplug_allow_cpu0=1 check > > Changes from v1 to v2: > - Rename the parameter from hotplug_cpu_start to hotplug_allow_cpu0 > - Add is_cpu0_allowed() function to check if we want to start running > test scripts from cpu0..cpuN > > Signed-off-by: Lisa Nguyen <lisa.ngu...@linaro.org> > --- > Makefile | 3 ++- > cpuhotplug/Makefile | 2 +- > cpuhotplug/cpuhotplug_02.sh | 3 +-- > cpuhotplug/cpuhotplug_03.sh | 2 +- > cpuhotplug/cpuhotplug_04.sh | 2 +- > cpuhotplug/cpuhotplug_05.sh | 4 ++-- > cpuhotplug/cpuhotplug_06.sh | 4 ++-- > cpuhotplug/cpuhotplug_07.sh | 2 +- > cpuhotplug/cpuhotplug_08.sh | 7 ++++++- > include/functions.sh | 12 ++++++++++++ > 10 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/Makefile b/Makefile > index 731619d..9f26fd3 100644 > --- a/Makefile > +++ b/Makefile > @@ -21,6 +21,7 @@ > # Torez Smith <torez.sm...@linaro.org> (IBM Corporation) > # - initial API and implementation > # > +hotplug_allow_cpu0?=0 > > all: > @(cd utils; $(MAKE)) > @@ -28,7 +29,7 @@ all: > check: > @(cd utils; $(MAKE) check) > @(cd cpufreq; $(MAKE) check) > - @(cd cpuhotplug; $(MAKE) check) > + @(cd cpuhotplug; $(MAKE) hotplug_allow_cpu0=${hotplug_allow_cpu0} > check) > @(cd cpuidle; $(MAKE) check) > # @(cd suspend; $(MAKE) check) > @(cd thermal; $(MAKE) check) > diff --git a/cpuhotplug/Makefile b/cpuhotplug/Makefile > index df0b8f4..6ee600d 100644 > --- a/cpuhotplug/Makefile > +++ b/cpuhotplug/Makefile > @@ -21,5 +21,5 @@ > # Daniel Lezcano <daniel.lezc...@linaro.org> (IBM Corporation) > # - initial API and implementation > # > - > +export hotplug_allow_cpu0?=0 > include ../Test.mk > diff --git a/cpuhotplug/cpuhotplug_02.sh b/cpuhotplug/cpuhotplug_02.sh > index 3157307..58e1f02 100755 > --- a/cpuhotplug/cpuhotplug_02.sh > +++ b/cpuhotplug/cpuhotplug_02.sh > @@ -24,7 +24,6 @@ > # > > # URL : > https://wiki.linaro.org/WorkingGroups/PowerManagement/Resources/TestSuite/PmQaSpecification#cpuhotplug_02 > - > source ../include/functions.sh > > check_state() { > @@ -34,7 +33,7 @@ check_state() { > shift 1 > > if [ "$cpu" == "cpu0" ]; then > - return 0 > + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0 Since is_cpu0_allowed has cpu0 in its name, why pass it $cpu as a parameter? You are already checking for cpu0 in the if statement. This can be simplified as if [ "$cpu" == "cpu0" ]; then is_cpu0_allowed $hotplug_allow_cpu0 || return 0 > fi > > set_offline $cpu > diff --git a/cpuhotplug/cpuhotplug_03.sh b/cpuhotplug/cpuhotplug_03.sh > index 13a0ce9..b29ff8e 100755 > --- a/cpuhotplug/cpuhotplug_03.sh > +++ b/cpuhotplug/cpuhotplug_03.sh > @@ -34,7 +34,7 @@ check_affinity_fails() { > local ret= > > if [ "$cpu" == "cpu0" ]; then > - return 0 > + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0 > fi > > set_offline $cpu > diff --git a/cpuhotplug/cpuhotplug_04.sh b/cpuhotplug/cpuhotplug_04.sh > index 394a512..543853f 100755 > --- a/cpuhotplug/cpuhotplug_04.sh > +++ b/cpuhotplug/cpuhotplug_04.sh > @@ -37,7 +37,7 @@ check_task_migrate() { > local ret= > > if [ "$cpu" == "cpu0" ]; then > - return 0 > + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0 > fi > > taskset 0x$cpumask $CPUBURN $cpu & > diff --git a/cpuhotplug/cpuhotplug_05.sh b/cpuhotplug/cpuhotplug_05.sh > index a8eb312..2f33a0c 100755 > --- a/cpuhotplug/cpuhotplug_05.sh > +++ b/cpuhotplug/cpuhotplug_05.sh > @@ -33,9 +33,9 @@ check_procinfo() { > local ret= > > if [ "$cpu" == "cpu0" ]; then > - return 0 > + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0 > fi > - > + > set_offline $cpu > > TAB=$(printf "\t"); grep "processor$TAB: $cpuid" /proc/cpuinfo > diff --git a/cpuhotplug/cpuhotplug_06.sh b/cpuhotplug/cpuhotplug_06.sh > index 347906d..528aeca 100755 > --- a/cpuhotplug/cpuhotplug_06.sh > +++ b/cpuhotplug/cpuhotplug_06.sh > @@ -33,9 +33,9 @@ check_procinfo() { > local ret= > > if [ "$cpu" == "cpu0" ]; then > - return 0 > + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0 > fi > - > + > set_offline $cpu > > grep "CPU$cpuid" /proc/interrupts > diff --git a/cpuhotplug/cpuhotplug_07.sh b/cpuhotplug/cpuhotplug_07.sh > index eaeba77..00fd009 100755 > --- a/cpuhotplug/cpuhotplug_07.sh > +++ b/cpuhotplug/cpuhotplug_07.sh > @@ -35,7 +35,7 @@ check_notification() { > local ret= > > if [ "$cpu" == "cpu0" ]; then > - return 0 > + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0 > fi > > # damn ! udevadm is buffering the output, we have to use a temp file > diff --git a/cpuhotplug/cpuhotplug_08.sh b/cpuhotplug/cpuhotplug_08.sh > index 9e2c355..0b1b9b6 100755 > --- a/cpuhotplug/cpuhotplug_08.sh > +++ b/cpuhotplug/cpuhotplug_08.sh > @@ -28,7 +28,12 @@ > source ../include/functions.sh > > function randomize() { > - echo $[ ( $RANDOM % $1 ) + 1 ] > + #if we are skipping cpu0 > + if [ $hotplug_allow_cpu0 -ne 0 ]; then > + echo $[ ( $RANDOM % $1 ) + 1 ] > + else > + echo $[ ( $RANDOM % $1 ) ] > + fi > } > > random_stress() { > diff --git a/include/functions.sh b/include/functions.sh > index 6d75e34..3ec2270 100644 > --- a/include/functions.sh > +++ b/include/functions.sh > @@ -372,3 +372,15 @@ is_root() { > fi > return $ret > } > + > +is_cpu0_allowed() { I suggest renaming this to is_cpu0_hotplug_allowed() to make functions.sh easy to read later > + local cpu=$1 remove cpu variable > + local status=$2 > + > + # Allow cpu0 to be tested > + if [[ "$cpu" == "cpu0" && $status -eq 0 ]]; then cpu == cpu0 check can be removed. > + return 0 > + else > + return 1 > + fi > +} > -- > 1.7.9.5 > _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev