By default, cpuhotplug does not run on cpu0. However, if we want to allow cpuhotplug to run on cpu0, then we must pass the parameter, hotplug_allow_cpu0=1, to make check.
Signed-off-by: Lisa Nguyen lisa.nguyen@linaro.org --- Changes from v2 to v3: - Renamed is_cpu0_allowed() function to is_cpu0_hotplug_allowed() - Removed $cpu parameter from is_cpu0_allowed function() - Modified test condition in check_cpuhotplug_files() function
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 --- Makefile | 4 ++-- cpuhotplug/Makefile | 2 +- cpuhotplug/cpuhotplug_02.sh | 2 +- cpuhotplug/cpuhotplug_03.sh | 2 +- cpuhotplug/cpuhotplug_04.sh | 2 +- cpuhotplug/cpuhotplug_05.sh | 2 +- cpuhotplug/cpuhotplug_06.sh | 2 +- cpuhotplug/cpuhotplug_07.sh | 2 +- cpuhotplug/cpuhotplug_08.sh | 6 +++++- include/functions.sh | 18 ++++++++++++++---- 10 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile index 731619d..d76deb7 100644 --- a/Makefile +++ b/Makefile @@ -21,14 +21,14 @@ # Torez Smith torez.smith@linaro.org (IBM Corporation) # - initial API and implementation # - +hotplug_allow_cpu0?=0 all: @(cd utils; $(MAKE))
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.lezcano@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..d2bb5b4 100755 --- a/cpuhotplug/cpuhotplug_02.sh +++ b/cpuhotplug/cpuhotplug_02.sh @@ -34,7 +34,7 @@ check_state() { shift 1
if [ "$cpu" == "cpu0" ]; then - return 0 + is_cpu0_hotplug_allowed $hotplug_allow_cpu0 || return 0 fi
set_offline $cpu diff --git a/cpuhotplug/cpuhotplug_03.sh b/cpuhotplug/cpuhotplug_03.sh index 13a0ce9..2a5ae48 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_hotplug_allowed $hotplug_allow_cpu0 || return 0 fi
set_offline $cpu diff --git a/cpuhotplug/cpuhotplug_04.sh b/cpuhotplug/cpuhotplug_04.sh index 394a512..7938e7d 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_hotplug_allowed $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..4042f2c 100755 --- a/cpuhotplug/cpuhotplug_05.sh +++ b/cpuhotplug/cpuhotplug_05.sh @@ -33,7 +33,7 @@ check_procinfo() { local ret=
if [ "$cpu" == "cpu0" ]; then - return 0 + is_cpu0_hotplug_allowed $hotplug_allow_cpu0 || return 0 fi
set_offline $cpu diff --git a/cpuhotplug/cpuhotplug_06.sh b/cpuhotplug/cpuhotplug_06.sh index 347906d..0461e37 100755 --- a/cpuhotplug/cpuhotplug_06.sh +++ b/cpuhotplug/cpuhotplug_06.sh @@ -33,7 +33,7 @@ check_procinfo() { local ret=
if [ "$cpu" == "cpu0" ]; then - return 0 + is_cpu0_hotplug_allowed $hotplug_allow_cpu0 || return 0 fi
set_offline $cpu diff --git a/cpuhotplug/cpuhotplug_07.sh b/cpuhotplug/cpuhotplug_07.sh index eaeba77..a3526be 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_hotplug_allowed $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..00315a4 100755 --- a/cpuhotplug/cpuhotplug_08.sh +++ b/cpuhotplug/cpuhotplug_08.sh @@ -28,7 +28,11 @@ source ../include/functions.sh
function randomize() { - echo $[ ( $RANDOM % $1 ) + 1 ] + if [ $hotplug_allow_cpu0 -eq 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..d44706f 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -285,10 +285,10 @@ check_cpuhotplug_files() { shift 1
for i in $@; do - # skip check for cpu0 - if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; then - continue - fi + if [[ $dirpath =~ cpu0$ && $hotplug_allow_cpu0 -eq 0 ]]; then + continue + fi + check_file $i $dirpath || return 1 done
@@ -372,3 +372,13 @@ is_root() { fi return $ret } + +is_cpu0_hotplug_allowed() { + local status=$1 + + if [ $status -eq 1 ]; then + return 0 + else + return 1 + fi +}
On Thu, Jul 3, 2014 at 7:37 AM, Lisa Nguyen lisa.nguyen@linaro.org wrote:
By default, cpuhotplug does not run on cpu0. However, if we want to allow cpuhotplug to run on cpu0, then we must pass the parameter, hotplug_allow_cpu0=1, to make check.
Signed-off-by: Lisa Nguyen lisa.nguyen@linaro.org
Changes from v2 to v3:
- Renamed is_cpu0_allowed() function to is_cpu0_hotplug_allowed()
- Removed $cpu parameter from is_cpu0_allowed function()
- Modified test condition in check_cpuhotplug_files() function
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
Makefile | 4 ++-- cpuhotplug/Makefile | 2 +- cpuhotplug/cpuhotplug_02.sh | 2 +- cpuhotplug/cpuhotplug_03.sh | 2 +- cpuhotplug/cpuhotplug_04.sh | 2 +- cpuhotplug/cpuhotplug_05.sh | 2 +- cpuhotplug/cpuhotplug_06.sh | 2 +- cpuhotplug/cpuhotplug_07.sh | 2 +- cpuhotplug/cpuhotplug_08.sh | 6 +++++- include/functions.sh | 18 ++++++++++++++---- 10 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile index 731619d..d76deb7 100644 --- a/Makefile +++ b/Makefile @@ -21,14 +21,14 @@ # Torez Smith torez.smith@linaro.org (IBM Corporation) # - initial API and implementation #
+hotplug_allow_cpu0?=0 all: @(cd utils; $(MAKE))
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.lezcano@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..d2bb5b4 100755 --- a/cpuhotplug/cpuhotplug_02.sh +++ b/cpuhotplug/cpuhotplug_02.sh @@ -34,7 +34,7 @@ check_state() { shift 1
if [ "$cpu" == "cpu0" ]; then
return 0
is_cpu0_hotplug_allowed $hotplug_allow_cpu0 || return 0fi
set_offline $cpu
diff --git a/cpuhotplug/cpuhotplug_03.sh b/cpuhotplug/cpuhotplug_03.sh index 13a0ce9..2a5ae48 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_hotplug_allowed $hotplug_allow_cpu0 || return 0fi
set_offline $cpu
diff --git a/cpuhotplug/cpuhotplug_04.sh b/cpuhotplug/cpuhotplug_04.sh index 394a512..7938e7d 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_hotplug_allowed $hotplug_allow_cpu0 || return 0fi
taskset 0x$cpumask $CPUBURN $cpu &
diff --git a/cpuhotplug/cpuhotplug_05.sh b/cpuhotplug/cpuhotplug_05.sh index a8eb312..4042f2c 100755 --- a/cpuhotplug/cpuhotplug_05.sh +++ b/cpuhotplug/cpuhotplug_05.sh @@ -33,7 +33,7 @@ check_procinfo() { local ret=
if [ "$cpu" == "cpu0" ]; then
return 0
is_cpu0_hotplug_allowed $hotplug_allow_cpu0 || return 0fi
set_offline $cpu
diff --git a/cpuhotplug/cpuhotplug_06.sh b/cpuhotplug/cpuhotplug_06.sh index 347906d..0461e37 100755 --- a/cpuhotplug/cpuhotplug_06.sh +++ b/cpuhotplug/cpuhotplug_06.sh @@ -33,7 +33,7 @@ check_procinfo() { local ret=
if [ "$cpu" == "cpu0" ]; then
return 0
is_cpu0_hotplug_allowed $hotplug_allow_cpu0 || return 0fi
set_offline $cpu
diff --git a/cpuhotplug/cpuhotplug_07.sh b/cpuhotplug/cpuhotplug_07.sh index eaeba77..a3526be 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_hotplug_allowed $hotplug_allow_cpu0 || return 0fi
# 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..00315a4 100755 --- a/cpuhotplug/cpuhotplug_08.sh +++ b/cpuhotplug/cpuhotplug_08.sh @@ -28,7 +28,11 @@ source ../include/functions.sh
function randomize() {
- echo $[ ( $RANDOM % $1 ) + 1 ]
- if [ $hotplug_allow_cpu0 -eq 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..d44706f 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -285,10 +285,10 @@ check_cpuhotplug_files() { shift 1
for i in $@; do
# skip check for cpu0if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; thencontinuefi
if [[ $dirpath =~ cpu0$ && $hotplug_allow_cpu0 -eq 0 ]]; then
I just checked 'man bash' for the meaning of =~ and found this:
"The return value is 0 if the string matches the pattern, and 1 otherwise."
So if you find cpu0 in $dirpath, it'll return a zero, right? So your above expression becomes zero and you don't end up skipping the test for cpu0.
(or is it too early in the morning and I shouldn't be trying to understand truth tables?) :)
continueficheck_file $i $dirpath || return 1@@ -372,3 +372,13 @@ is_root() { fi return $ret }
+is_cpu0_hotplug_allowed() {
- local status=$1
- if [ $status -eq 1 ]; then
return 0- else
return 1- fi
+}
1.7.9.5
On Thu, 2014-07-03 at 10:24 +0530, Amit Kucheria wrote:
On Thu, Jul 3, 2014 at 7:37 AM, Lisa Nguyen lisa.nguyen@linaro.org wrote:
[...]
diff --git a/include/functions.sh b/include/functions.sh index 6d75e34..d44706f 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -285,10 +285,10 @@ check_cpuhotplug_files() { shift 1
for i in $@; do
# skip check for cpu0if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; thencontinuefi
if [[ $dirpath =~ cpu0$ && $hotplug_allow_cpu0 -eq 0 ]]; thenI just checked 'man bash' for the meaning of =~ and found this:
"The return value is 0 if the string matches the pattern, and 1 otherwise."
So if you find cpu0 in $dirpath, it'll return a zero, right? So your above expression becomes zero and you don't end up skipping the test for cpu0.
But in shells, isn't zero true and non-zero false, and && behaves accordingly?
I have another comment though, will the scripts only be used on systems with bash? If we can't guarantee this perhaps bash specific features should be avoided. (Is the android shell bash compatible?)
On Thu, Jul 3, 2014 at 2:22 PM, Jon Medhurst (Tixy) tixy@linaro.org wrote:
On Thu, 2014-07-03 at 10:24 +0530, Amit Kucheria wrote:
On Thu, Jul 3, 2014 at 7:37 AM, Lisa Nguyen lisa.nguyen@linaro.org wrote:
[...]
diff --git a/include/functions.sh b/include/functions.sh index 6d75e34..d44706f 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -285,10 +285,10 @@ check_cpuhotplug_files() { shift 1
for i in $@; do
# skip check for cpu0if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; thencontinuefi
if [[ $dirpath =~ cpu0$ && $hotplug_allow_cpu0 -eq 0 ]]; thenI just checked 'man bash' for the meaning of =~ and found this:
"The return value is 0 if the string matches the pattern, and 1 otherwise."
So if you find cpu0 in $dirpath, it'll return a zero, right? So your above expression becomes zero and you don't end up skipping the test for cpu0.
But in shells, isn't zero true and non-zero false, and && behaves accordingly?
I knew I was missing something. Thanks for the correction. No more morning reviews.
I have another comment though, will the scripts only be used on systems with bash? If we can't guarantee this perhaps bash specific features should be avoided. (Is the android shell bash compatible?)
At the moment pm-qa depends on /bin/bash. While restricting ourselves to POSIX shell constructs would be ideal, I can't commit to doing a wholesale conversion anytime soon.
Having said that, I just confirmed with Vishal and Android doesn't care about #!/bin/bash. It just executes the shell script using /system/bin/sh. And we've been fixing any error reports, so it looks like the shell is Bash-compatible.
On Thu, 2014-07-03 at 14:59 +0530, Amit Kucheria wrote:
On Thu, Jul 3, 2014 at 2:22 PM, Jon Medhurst (Tixy) tixy@linaro.org wrote:
On Thu, 2014-07-03 at 10:24 +0530, Amit Kucheria wrote:
On Thu, Jul 3, 2014 at 7:37 AM, Lisa Nguyen lisa.nguyen@linaro.org wrote:
[...]
diff --git a/include/functions.sh b/include/functions.sh index 6d75e34..d44706f 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -285,10 +285,10 @@ check_cpuhotplug_files() { shift 1
for i in $@; do
# skip check for cpu0if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; thencontinuefi
if [[ $dirpath =~ cpu0$ && $hotplug_allow_cpu0 -eq 0 ]]; thenI just checked 'man bash' for the meaning of =~ and found this:
"The return value is 0 if the string matches the pattern, and 1 otherwise."
So if you find cpu0 in $dirpath, it'll return a zero, right? So your above expression becomes zero and you don't end up skipping the test for cpu0.
But in shells, isn't zero true and non-zero false, and && behaves accordingly?
I knew I was missing something. Thanks for the correction. No more morning reviews.
I have another comment though, will the scripts only be used on systems with bash? If we can't guarantee this perhaps bash specific features should be avoided. (Is the android shell bash compatible?)
At the moment pm-qa depends on /bin/bash. While restricting ourselves to POSIX shell constructs would be ideal, I can't commit to doing a wholesale conversion anytime soon.
Having said that, I just confirmed with Vishal and Android doesn't care about #!/bin/bash. It just executes the shell script using /system/bin/sh. And we've been fixing any error reports, so it looks like the shell is Bash-compatible.
Well, for me on Android I get
syntax error: '=~' unexpected operator/operand
when running a script containing
#!/bin/bash foo=bar; if [[ $foo =~ $1 ]]; then echo yes; else echo no; fi
The same script runs OK on my PC, giving yes/no as appropriate to what I pass as the argument.
So for the $subject patch, best to keep the existing check which uses grep, and just make the 'continue' conditional, e.g.
if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; then if [ $hotplug_allow_cpu0 -eq 0 ]; then continue; fi fi
On Thu, Jul 3, 2014 at 3:38 PM, Jon Medhurst (Tixy) tixy@linaro.org wrote:
On Thu, 2014-07-03 at 14:59 +0530, Amit Kucheria wrote:
On Thu, Jul 3, 2014 at 2:22 PM, Jon Medhurst (Tixy) tixy@linaro.org wrote:
On Thu, 2014-07-03 at 10:24 +0530, Amit Kucheria wrote:
On Thu, Jul 3, 2014 at 7:37 AM, Lisa Nguyen lisa.nguyen@linaro.org wrote:
[...]
diff --git a/include/functions.sh b/include/functions.sh index 6d75e34..d44706f 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -285,10 +285,10 @@ check_cpuhotplug_files() { shift 1
for i in $@; do
# skip check for cpu0if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; thencontinuefi
if [[ $dirpath =~ cpu0$ && $hotplug_allow_cpu0 -eq 0 ]]; thenI just checked 'man bash' for the meaning of =~ and found this:
"The return value is 0 if the string matches the pattern, and 1 otherwise."
So if you find cpu0 in $dirpath, it'll return a zero, right? So your above expression becomes zero and you don't end up skipping the test for cpu0.
But in shells, isn't zero true and non-zero false, and && behaves accordingly?
I knew I was missing something. Thanks for the correction. No more morning reviews.
I have another comment though, will the scripts only be used on systems with bash? If we can't guarantee this perhaps bash specific features should be avoided. (Is the android shell bash compatible?)
At the moment pm-qa depends on /bin/bash. While restricting ourselves to POSIX shell constructs would be ideal, I can't commit to doing a wholesale conversion anytime soon.
Having said that, I just confirmed with Vishal and Android doesn't care about #!/bin/bash. It just executes the shell script using /system/bin/sh. And we've been fixing any error reports, so it looks like the shell is Bash-compatible.
Well, for me on Android I get
syntax error: '=~' unexpected operator/operand
when running a script containing
#!/bin/bash foo=bar; if [[ $foo =~ $1 ]]; then echo yes; else echo no; fi
The same script runs OK on my PC, giving yes/no as appropriate to what I pass as the argument.
So for the $subject patch, best to keep the existing check which uses grep, and just make the 'continue' conditional, e.g.
if [ `echo $dirpath | grep -c "cpu0"` -eq 1 ]; then if [ $hotplug_allow_cpu0 -eq 0 ]; then continue; fi fi
Makes sense.
-
Amit Kucheria -
Jon Medhurst (Tixy) -
Lisa Nguyen