* [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03
@ 2019-06-25 10:03 Po-Hsu Lin
2019-06-25 10:59 ` Xiao Yang
0 siblings, 1 reply; 6+ messages in thread
From: Po-Hsu Lin @ 2019-06-25 10:03 UTC (permalink / raw)
To: ltp
The arithmetic operation (( total_cpus-=1 )) does not work for dash,
which was symbolic linked to /bin/sh on some OS like Ubuntu.
In such case, people will see error like:
runpwtests03.sh: total_cpus-=1: not found
And this further leads to access for a non-existing file and cause
false-positive result in the end:
runpwtests03.sh: cannot create
/sys/devices/system/cpu/cpu8/cpufreq/scaling_governor: Directory nonexistent
runpwtests03.sh: FAIL: Unable to set governor -- powersave for cpu8
Power_Management03 2 TFAIL: Changing governors
Use expr instead for fix this issue.
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
testcases/kernel/power_management/runpwtests03.sh | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/testcases/kernel/power_management/runpwtests03.sh b/testcases/kernel/power_management/runpwtests03.sh
index 11197937f..3fb85d273 100755
--- a/testcases/kernel/power_management/runpwtests03.sh
+++ b/testcases/kernel/power_management/runpwtests03.sh
@@ -25,8 +25,7 @@ export TST_TOTAL=4
. pm_include.sh
check_cpufreq_sysfs_files() {
- total_cpus=$(tst_ncpus)
- (( total_cpus-=1 ))
+ total_cpus=`expr $(tst_ncpus) - 1`
RC=0
for cpu in $(seq 0 "${total_cpus}" )
@@ -51,8 +50,7 @@ check_cpufreq_sysfs_files() {
change_govr() {
available_govr=$(get_supporting_govr)
- total_cpus=$(tst_ncpus)
- (( total_cpus-=1 ))
+ total_cpus=`expr $(tst_ncpus) - 1`
RC=0
for cpu in $(seq 0 "${total_cpus}" )
@@ -79,8 +77,7 @@ change_freq() {
available_govr=$(get_supporting_govr)
RC=0
- total_cpus=$(tst_ncpus)
- (( total_cpus-=1 ))
+ total_cpus=`expr $(tst_ncpus) - 1`
if ( echo ${available_govr} | grep -i "userspace" \
>/dev/null 2>&1 ); then
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03
2019-06-25 10:03 [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03 Po-Hsu Lin
@ 2019-06-25 10:59 ` Xiao Yang
2019-06-25 16:13 ` Po-Hsu Lin
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Yang @ 2019-06-25 10:59 UTC (permalink / raw)
To: ltp
On 2019/06/25 18:03, Po-Hsu Lin wrote:
> The arithmetic operation (( total_cpus-=1 )) does not work for dash,
> which was symbolic linked to /bin/sh on some OS like Ubuntu.
>
> In such case, people will see error like:
> runpwtests03.sh: total_cpus-=1: not found
>
> And this further leads to access for a non-existing file and cause
> false-positive result in the end:
> runpwtests03.sh: cannot create
> /sys/devices/system/cpu/cpu8/cpufreq/scaling_governor: Directory nonexistent
> runpwtests03.sh: FAIL: Unable to set governor -- powersave for cpu8
> Power_Management03 2 TFAIL: Changing governors
>
> Use expr instead for fix this issue.
>
> Signed-off-by: Po-Hsu Lin<po-hsu.lin@canonical.com>
> ---
> testcases/kernel/power_management/runpwtests03.sh | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/testcases/kernel/power_management/runpwtests03.sh b/testcases/kernel/power_management/runpwtests03.sh
> index 11197937f..3fb85d273 100755
> --- a/testcases/kernel/power_management/runpwtests03.sh
> +++ b/testcases/kernel/power_management/runpwtests03.sh
> @@ -25,8 +25,7 @@ export TST_TOTAL=4
> . pm_include.sh
>
> check_cpufreq_sysfs_files() {
> - total_cpus=$(tst_ncpus)
> - (( total_cpus-=1 ))
> + total_cpus=`expr $(tst_ncpus) - 1`
Hi,
It's wrong for single variable to use $(), did you mean ${tst_ncpus}?
Best Regards,
Xiao Yang
> RC=0
>
> for cpu in $(seq 0 "${total_cpus}" )
> @@ -51,8 +50,7 @@ check_cpufreq_sysfs_files() {
> change_govr() {
> available_govr=$(get_supporting_govr)
>
> - total_cpus=$(tst_ncpus)
> - (( total_cpus-=1 ))
> + total_cpus=`expr $(tst_ncpus) - 1`
> RC=0
>
> for cpu in $(seq 0 "${total_cpus}" )
> @@ -79,8 +77,7 @@ change_freq() {
> available_govr=$(get_supporting_govr)
> RC=0
>
> - total_cpus=$(tst_ncpus)
> - (( total_cpus-=1 ))
> + total_cpus=`expr $(tst_ncpus) - 1`
>
> if ( echo ${available_govr} | grep -i "userspace" \
> >/dev/null 2>&1 ); then
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03
2019-06-25 10:59 ` Xiao Yang
@ 2019-06-25 16:13 ` Po-Hsu Lin
2019-06-26 2:54 ` Xiao Yang
0 siblings, 1 reply; 6+ messages in thread
From: Po-Hsu Lin @ 2019-06-25 16:13 UTC (permalink / raw)
To: ltp
Hello!
The $() here is for the tst_ncpus executable, which will return the
total cpu number.
Thanks
PHLin
On Tue, Jun 25, 2019 at 6:59 PM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> On 2019/06/25 18:03, Po-Hsu Lin wrote:
> > The arithmetic operation (( total_cpus-=1 )) does not work for dash,
> > which was symbolic linked to /bin/sh on some OS like Ubuntu.
> >
> > In such case, people will see error like:
> > runpwtests03.sh: total_cpus-=1: not found
> >
> > And this further leads to access for a non-existing file and cause
> > false-positive result in the end:
> > runpwtests03.sh: cannot create
> > /sys/devices/system/cpu/cpu8/cpufreq/scaling_governor: Directory nonexistent
> > runpwtests03.sh: FAIL: Unable to set governor -- powersave for cpu8
> > Power_Management03 2 TFAIL: Changing governors
> >
> > Use expr instead for fix this issue.
> >
> > Signed-off-by: Po-Hsu Lin<po-hsu.lin@canonical.com>
> > ---
> > testcases/kernel/power_management/runpwtests03.sh | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/testcases/kernel/power_management/runpwtests03.sh b/testcases/kernel/power_management/runpwtests03.sh
> > index 11197937f..3fb85d273 100755
> > --- a/testcases/kernel/power_management/runpwtests03.sh
> > +++ b/testcases/kernel/power_management/runpwtests03.sh
> > @@ -25,8 +25,7 @@ export TST_TOTAL=4
> > . pm_include.sh
> >
> > check_cpufreq_sysfs_files() {
> > - total_cpus=$(tst_ncpus)
> > - (( total_cpus-=1 ))
> > + total_cpus=`expr $(tst_ncpus) - 1`
>
> Hi,
>
> It's wrong for single variable to use $(), did you mean ${tst_ncpus}?
>
> Best Regards,
> Xiao Yang
> > RC=0
> >
> > for cpu in $(seq 0 "${total_cpus}" )
> > @@ -51,8 +50,7 @@ check_cpufreq_sysfs_files() {
> > change_govr() {
> > available_govr=$(get_supporting_govr)
> >
> > - total_cpus=$(tst_ncpus)
> > - (( total_cpus-=1 ))
> > + total_cpus=`expr $(tst_ncpus) - 1`
> > RC=0
> >
> > for cpu in $(seq 0 "${total_cpus}" )
> > @@ -79,8 +77,7 @@ change_freq() {
> > available_govr=$(get_supporting_govr)
> > RC=0
> >
> > - total_cpus=$(tst_ncpus)
> > - (( total_cpus-=1 ))
> > + total_cpus=`expr $(tst_ncpus) - 1`
> >
> > if ( echo ${available_govr} | grep -i "userspace" \
> > >/dev/null 2>&1 ); then
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03
2019-06-25 16:13 ` Po-Hsu Lin
@ 2019-06-26 2:54 ` Xiao Yang
2019-06-26 3:45 ` Po-Hsu Lin
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Yang @ 2019-06-26 2:54 UTC (permalink / raw)
To: ltp
Hi,
Sorry, I missed the fact that tst_ncpus is executable binary.
It's good to me.
Reviewed-by: Xiao Yang <ice_yangxiao@163.com>
BTW: if you don't want to depend expr command, we can fix it by using
total_cpus=$((total_cpus - 1)) instead.
Best Regards,
Xiao Yang
On 06/26/2019 12:13 AM, Po-Hsu Lin wrote:
> Hello!
>
> The $() here is for the tst_ncpus executable, which will return the
> total cpu number.
>
> Thanks
> PHLin
>
> On Tue, Jun 25, 2019 at 6:59 PM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>> On 2019/06/25 18:03, Po-Hsu Lin wrote:
>>> The arithmetic operation (( total_cpus-=1 )) does not work for dash,
>>> which was symbolic linked to /bin/sh on some OS like Ubuntu.
>>>
>>> In such case, people will see error like:
>>> runpwtests03.sh: total_cpus-=1: not found
>>>
>>> And this further leads to access for a non-existing file and cause
>>> false-positive result in the end:
>>> runpwtests03.sh: cannot create
>>> /sys/devices/system/cpu/cpu8/cpufreq/scaling_governor: Directory nonexistent
>>> runpwtests03.sh: FAIL: Unable to set governor -- powersave for cpu8
>>> Power_Management03 2 TFAIL: Changing governors
>>>
>>> Use expr instead for fix this issue.
>>>
>>> Signed-off-by: Po-Hsu Lin<po-hsu.lin@canonical.com>
>>> ---
>>> testcases/kernel/power_management/runpwtests03.sh | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/testcases/kernel/power_management/runpwtests03.sh b/testcases/kernel/power_management/runpwtests03.sh
>>> index 11197937f..3fb85d273 100755
>>> --- a/testcases/kernel/power_management/runpwtests03.sh
>>> +++ b/testcases/kernel/power_management/runpwtests03.sh
>>> @@ -25,8 +25,7 @@ export TST_TOTAL=4
>>> . pm_include.sh
>>>
>>> check_cpufreq_sysfs_files() {
>>> - total_cpus=$(tst_ncpus)
>>> - (( total_cpus-=1 ))
>>> + total_cpus=`expr $(tst_ncpus) - 1`
>> Hi,
>>
>> It's wrong for single variable to use $(), did you mean ${tst_ncpus}?
>>
>> Best Regards,
>> Xiao Yang
>>> RC=0
>>>
>>> for cpu in $(seq 0 "${total_cpus}" )
>>> @@ -51,8 +50,7 @@ check_cpufreq_sysfs_files() {
>>> change_govr() {
>>> available_govr=$(get_supporting_govr)
>>>
>>> - total_cpus=$(tst_ncpus)
>>> - (( total_cpus-=1 ))
>>> + total_cpus=`expr $(tst_ncpus) - 1`
>>> RC=0
>>>
>>> for cpu in $(seq 0 "${total_cpus}" )
>>> @@ -79,8 +77,7 @@ change_freq() {
>>> available_govr=$(get_supporting_govr)
>>> RC=0
>>>
>>> - total_cpus=$(tst_ncpus)
>>> - (( total_cpus-=1 ))
>>> + total_cpus=`expr $(tst_ncpus) - 1`
>>>
>>> if ( echo ${available_govr} | grep -i "userspace" \
>>> >/dev/null 2>&1 ); then
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03
2019-06-26 2:54 ` Xiao Yang
@ 2019-06-26 3:45 ` Po-Hsu Lin
2019-06-27 8:04 ` Xiao Yang
0 siblings, 1 reply; 6+ messages in thread
From: Po-Hsu Lin @ 2019-06-26 3:45 UTC (permalink / raw)
To: ltp
Thanks for the review,
They are both fine with me, if you want to see this to be changed into
the double parentheses form, I can send a V2 for that.
Cheers
PHLin
On Wed, Jun 26, 2019 at 10:55 AM Xiao Yang <ice_yangxiao@163.com> wrote:
>
> Hi,
>
> Sorry, I missed the fact that tst_ncpus is executable binary.
>
> It's good to me.
>
> Reviewed-by: Xiao Yang <ice_yangxiao@163.com>
>
> BTW: if you don't want to depend expr command, we can fix it by using
> total_cpus=$((total_cpus - 1)) instead.
>
> Best Regards,
> Xiao Yang
>
> On 06/26/2019 12:13 AM, Po-Hsu Lin wrote:
> > Hello!
> >
> > The $() here is for the tst_ncpus executable, which will return the
> > total cpu number.
> >
> > Thanks
> > PHLin
> >
> > On Tue, Jun 25, 2019 at 6:59 PM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
> >> On 2019/06/25 18:03, Po-Hsu Lin wrote:
> >>> The arithmetic operation (( total_cpus-=1 )) does not work for dash,
> >>> which was symbolic linked to /bin/sh on some OS like Ubuntu.
> >>>
> >>> In such case, people will see error like:
> >>> runpwtests03.sh: total_cpus-=1: not found
> >>>
> >>> And this further leads to access for a non-existing file and cause
> >>> false-positive result in the end:
> >>> runpwtests03.sh: cannot create
> >>> /sys/devices/system/cpu/cpu8/cpufreq/scaling_governor: Directory nonexistent
> >>> runpwtests03.sh: FAIL: Unable to set governor -- powersave for cpu8
> >>> Power_Management03 2 TFAIL: Changing governors
> >>>
> >>> Use expr instead for fix this issue.
> >>>
> >>> Signed-off-by: Po-Hsu Lin<po-hsu.lin@canonical.com>
> >>> ---
> >>> testcases/kernel/power_management/runpwtests03.sh | 9 +++------
> >>> 1 file changed, 3 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/testcases/kernel/power_management/runpwtests03.sh b/testcases/kernel/power_management/runpwtests03.sh
> >>> index 11197937f..3fb85d273 100755
> >>> --- a/testcases/kernel/power_management/runpwtests03.sh
> >>> +++ b/testcases/kernel/power_management/runpwtests03.sh
> >>> @@ -25,8 +25,7 @@ export TST_TOTAL=4
> >>> . pm_include.sh
> >>>
> >>> check_cpufreq_sysfs_files() {
> >>> - total_cpus=$(tst_ncpus)
> >>> - (( total_cpus-=1 ))
> >>> + total_cpus=`expr $(tst_ncpus) - 1`
> >> Hi,
> >>
> >> It's wrong for single variable to use $(), did you mean ${tst_ncpus}?
> >>
> >> Best Regards,
> >> Xiao Yang
> >>> RC=0
> >>>
> >>> for cpu in $(seq 0 "${total_cpus}" )
> >>> @@ -51,8 +50,7 @@ check_cpufreq_sysfs_files() {
> >>> change_govr() {
> >>> available_govr=$(get_supporting_govr)
> >>>
> >>> - total_cpus=$(tst_ncpus)
> >>> - (( total_cpus-=1 ))
> >>> + total_cpus=`expr $(tst_ncpus) - 1`
> >>> RC=0
> >>>
> >>> for cpu in $(seq 0 "${total_cpus}" )
> >>> @@ -79,8 +77,7 @@ change_freq() {
> >>> available_govr=$(get_supporting_govr)
> >>> RC=0
> >>>
> >>> - total_cpus=$(tst_ncpus)
> >>> - (( total_cpus-=1 ))
> >>> + total_cpus=`expr $(tst_ncpus) - 1`
> >>>
> >>> if ( echo ${available_govr} | grep -i "userspace" \
> >>> >/dev/null 2>&1 ); then
> >>
> >>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03
2019-06-26 3:45 ` Po-Hsu Lin
@ 2019-06-27 8:04 ` Xiao Yang
0 siblings, 0 replies; 6+ messages in thread
From: Xiao Yang @ 2019-06-27 8:04 UTC (permalink / raw)
To: ltp
Hi,
Don't need.? It's fine to me as well.
Pushed, thanks for your patch.
Best Reagrds,
Xiao Yang
On 06/26/2019 11:45 AM, Po-Hsu Lin wrote:
> Thanks for the review,
>
> They are both fine with me, if you want to see this to be changed into
> the double parentheses form, I can send a V2 for that.
>
> Cheers
> PHLin
>
> On Wed, Jun 26, 2019 at 10:55 AM Xiao Yang <ice_yangxiao@163.com> wrote:
>> Hi,
>>
>> Sorry, I missed the fact that tst_ncpus is executable binary.
>>
>> It's good to me.
>>
>> Reviewed-by: Xiao Yang <ice_yangxiao@163.com>
>>
>> BTW: if you don't want to depend expr command, we can fix it by using
>> total_cpus=$((total_cpus - 1)) instead.
>>
>> Best Regards,
>> Xiao Yang
>>
>> On 06/26/2019 12:13 AM, Po-Hsu Lin wrote:
>>> Hello!
>>>
>>> The $() here is for the tst_ncpus executable, which will return the
>>> total cpu number.
>>>
>>> Thanks
>>> PHLin
>>>
>>> On Tue, Jun 25, 2019 at 6:59 PM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>>>> On 2019/06/25 18:03, Po-Hsu Lin wrote:
>>>>> The arithmetic operation (( total_cpus-=1 )) does not work for dash,
>>>>> which was symbolic linked to /bin/sh on some OS like Ubuntu.
>>>>>
>>>>> In such case, people will see error like:
>>>>> runpwtests03.sh: total_cpus-=1: not found
>>>>>
>>>>> And this further leads to access for a non-existing file and cause
>>>>> false-positive result in the end:
>>>>> runpwtests03.sh: cannot create
>>>>> /sys/devices/system/cpu/cpu8/cpufreq/scaling_governor: Directory nonexistent
>>>>> runpwtests03.sh: FAIL: Unable to set governor -- powersave for cpu8
>>>>> Power_Management03 2 TFAIL: Changing governors
>>>>>
>>>>> Use expr instead for fix this issue.
>>>>>
>>>>> Signed-off-by: Po-Hsu Lin<po-hsu.lin@canonical.com>
>>>>> ---
>>>>> testcases/kernel/power_management/runpwtests03.sh | 9 +++------
>>>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/testcases/kernel/power_management/runpwtests03.sh b/testcases/kernel/power_management/runpwtests03.sh
>>>>> index 11197937f..3fb85d273 100755
>>>>> --- a/testcases/kernel/power_management/runpwtests03.sh
>>>>> +++ b/testcases/kernel/power_management/runpwtests03.sh
>>>>> @@ -25,8 +25,7 @@ export TST_TOTAL=4
>>>>> . pm_include.sh
>>>>>
>>>>> check_cpufreq_sysfs_files() {
>>>>> - total_cpus=$(tst_ncpus)
>>>>> - (( total_cpus-=1 ))
>>>>> + total_cpus=`expr $(tst_ncpus) - 1`
>>>> Hi,
>>>>
>>>> It's wrong for single variable to use $(), did you mean ${tst_ncpus}?
>>>>
>>>> Best Regards,
>>>> Xiao Yang
>>>>> RC=0
>>>>>
>>>>> for cpu in $(seq 0 "${total_cpus}" )
>>>>> @@ -51,8 +50,7 @@ check_cpufreq_sysfs_files() {
>>>>> change_govr() {
>>>>> available_govr=$(get_supporting_govr)
>>>>>
>>>>> - total_cpus=$(tst_ncpus)
>>>>> - (( total_cpus-=1 ))
>>>>> + total_cpus=`expr $(tst_ncpus) - 1`
>>>>> RC=0
>>>>>
>>>>> for cpu in $(seq 0 "${total_cpus}" )
>>>>> @@ -79,8 +77,7 @@ change_freq() {
>>>>> available_govr=$(get_supporting_govr)
>>>>> RC=0
>>>>>
>>>>> - total_cpus=$(tst_ncpus)
>>>>> - (( total_cpus-=1 ))
>>>>> + total_cpus=`expr $(tst_ncpus) - 1`
>>>>>
>>>>> if ( echo ${available_govr} | grep -i "userspace" \
>>>>> >/dev/null 2>&1 ); then
>>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-27 8:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25 10:03 [LTP] [PATCH] runpwtests03: use expr to calculate total_cpus for runpwtests03 Po-Hsu Lin
2019-06-25 10:59 ` Xiao Yang
2019-06-25 16:13 ` Po-Hsu Lin
2019-06-26 2:54 ` Xiao Yang
2019-06-26 3:45 ` Po-Hsu Lin
2019-06-27 8:04 ` Xiao Yang
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.