Re: [LTP] [PATCH 3/4] cpu_hotplug: use hotplug/present cpus functions

Jan Stancek
2015-04-29T14:41:35Z

----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
> To: "Jan Stancek" <jstancek@redhat.com>, ltp-list@lists.sourceforge.net
> Sent: Wednesday, 29 April, 2015 3:09:06 PM
> Subject: Re: [PATCH 3/4] cpu_hotplug: use hotplug/present cpus functions
>
> On 04/29/2015 02:18 PM, Jan Stancek wrote:
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh | 6
> > +++---
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug02.sh | 2 +-
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug03.sh | 4 ++--
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug04.sh | 4 ++--
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug05.sh | 2 +-
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug06.sh | 2 +-
> > testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug07.sh | 2 +-
> > 7 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
> > index 52598a9..8ae1379 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
> > @@ -111,7 +111,7 @@ LOOP_COUNT=1
> >
> > tst_check_cmds perl
> >
> > -get_cpus_num
> > +get_present_cpus_num
> > if [ $? -lt 2 ]; then
> > tst_brkm TCONF "system doesn't have required CPU hotplug support"
> > fi
> > @@ -148,7 +148,7 @@ do
> > IRQ_START=$(cat /proc/interrupts)
> >
> > # Attempt to offline all CPUs
> > - for cpu in $( get_all_cpus ); do
> > + for cpu in $( get_hotplug_cpus ); do
> > if [ "$cpu" = "cpu0" ]; then
> > continue
> > fi
> > @@ -163,7 +163,7 @@ do
> > done
> >
> > # Attempt to online all CPUs
> > - for cpu in $( get_all_cpus ); do
> > + for cpu in $( get_hotplug_cpus ); do
> > if [ "$cpu" = "cpu0" ]; then
> > continue
> > fi
>
> Given that now this loop runs over hotpluggable cpus, it seems there is
> no reason to treat cpu0 specially in cpuhotplug0{1, 3, 4}.sh.

I'm not sure we can remove it in 01, because then it would try to bring
all offline and if all are hotpluggable it's going to fail.
I'd ignore 01 for now, it's broken anyway and disabled in runtest file.

03 looks like it can be removed, 04 is fixed by your patch.

Also, I'd prefer if those testcases would find suitable cpu to test
as part of test, instead of relying on passed parameters - but
I don't want to inflate this patchset even more now.

>
> Patch 4 in this series has a fix for cpuhotplug04.sh, but other
> occurrences are unfixed.
>
>
>
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug02.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug02.sh
> > index 3b33720..c42cc1b 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug02.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug02.sh
> > @@ -54,7 +54,7 @@ done
> >
> > LOOP_COUNT=1
> >
> > -get_cpus_num
> > +get_present_cpus_num
> > if [ $? -lt 2 ]; then
> > tst_brkm TCONF "system doesn't have required CPU hotplug support"
> > fi
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug03.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug03.sh
> > index 817f066..5da4854 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug03.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug03.sh
> > @@ -63,7 +63,7 @@ done
> >
> > LOOP_COUNT=1
> >
> > -get_cpus_num
> > +get_present_cpus_num
> > if [ $? -lt 2 ]; then
> > tst_brkm TCONF "system doesn't have required CPU hotplug support"
> > fi
> > @@ -86,7 +86,7 @@ until [ $LOOP_COUNT -gt $HOTPLUG03_LOOPS ]; do
> > number_of_cpus=0
> >
> > # Turns on all CPUs
> > - for i in $( get_all_cpus ); do
> > + for i in $( get_hotplug_cpus ); do
> > if [ "$i" = "cpu0" ]; then
> > continue
> > fi
>
> In general, get_hotplug_cpus should produce only a subset from
> get_all_cpus, so number_of_cpus below may be less that the total number
> of CPUs in the system.
>
> But below in the code we have:
>
> # Start up a number of processes equal to twice the number of
> # CPUs we have. This is to help ensure we've got enough processes
> # that at least one will migrate to the new CPU. Store the PIDs
> # so we can kill them later.
> number_of_cpus=$((number_of_cpus*2))
> until [ $number_of_cpus -eq 0 ]; do
> cpuhotplug_do_spin_loop > /dev/null 2>&1 &
> echo $! >> /var/run/hotplug4_$$.pid
> number_of_cpus=$((number_of_cpus-1))
> done
>
> It seems that now we need to fork $(( $(get_present_cpus_num) * 2 ))
> processes.

Good catch, I'll fix that.

Regards,
Jan

>
>
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug04.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug04.sh
> > index ea2723b..3e025da 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug04.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug04.sh
> > @@ -62,7 +62,7 @@ until [ $LOOP_COUNT -gt $HOTPLUG04_LOOPS ]; do
> > cpustate=1
> >
> > # Online all the CPUs
> > - for i in $(get_all_cpus); do
> > + for i in $(get_hotplug_cpus); do
> > if [ "$i" != "cpu0" ]; then
> > if ! cpu_is_online $i; then
> > if ! online_cpu $i; then
> > @@ -79,7 +79,7 @@ until [ $LOOP_COUNT -gt $HOTPLUG04_LOOPS ]; do
> > done
> >
> > # Now offline all the CPUs
> > - for i in $(get_all_cpus); do
> > + for i in $(get_hotplug_cpus); do
> > if ! offline_cpu $i; then
> > if [ "x$i" != "xcpu0" ]; then
> > tst_resm TFAIL "Did not offline first CPU (offlined $i instead)"
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug05.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug05.sh
> > index bb0e896..79f7e90 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug05.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug05.sh
> > @@ -54,7 +54,7 @@ LOOP_COUNT=1
> >
> > tst_check_cmds sar
> >
> > -get_cpus_num
> > +get_present_cpus_num
> > if [ $? -lt 2 ]; then
> > tst_brkm TCONF "system doesn't have required CPU hotplug support"
> > fi
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug06.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug06.sh
> > index 2e48242..6710fad 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug06.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug06.sh
> > @@ -49,7 +49,7 @@ done
> >
> > LOOP_COUNT=1
> >
> > -get_cpus_num
> > +get_present_cpus_num
> > if [ $? -lt 2 ]; then
> > tst_brkm TCONF "system doesn't have required CPU hotplug support"
> > fi
> > diff --git
> > a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug07.sh
> > b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug07.sh
> > index 723f3de..7edb3a9 100755
> > --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug07.sh
> > +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug07.sh
> > @@ -55,7 +55,7 @@ done
> >
> > LOOP_COUNT=1
> >
> > -get_cpus_num
> > +get_present_cpus_num
> > if [ $? -lt 2 ]; then
> > tst_brkm TCONF "system doesn't have required CPU hotplug support"
> > fi
> >