From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Thomas Huth" <thuth@redhat.com>
Cc: <kvm@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg
Date: Thu, 09 May 2024 15:44:27 +1000 [thread overview]
Message-ID: <D14VIP36ZLPW.334QUWLZMQ5T3@gmail.com> (raw)
In-Reply-To: <50e43047-b251-465b-b4b0-b5987ec9aa78@redhat.com>
On Wed May 8, 2024 at 11:36 PM AEST, Thomas Huth wrote:
> On 08/05/2024 14.58, Thomas Huth wrote:
> > On 08/05/2024 14.55, Thomas Huth wrote:
> >> On 08/05/2024 14.27, Nicholas Piggin wrote:
> >>> On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:
> >>>> On 04/05/2024 14.28, Nicholas Piggin wrote:
> >>>>> This allows different machines with different requirements to be
> >>>>> supported by run_tests.sh, similarly to how different accelerators
> >>>>> are handled.
> >>>>>
> >>>>> Acked-by: Thomas Huth <thuth@redhat.com>
> >>>>> Acked-by: Andrew Jones <andrew.jones@linux.dev>
> >>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>>>> ---
> >>>>> docs/unittests.txt | 7 +++++++
> >>>>> scripts/common.bash | 8 ++++++--
> >>>>> scripts/runtime.bash | 16 ++++++++++++----
> >>>>> 3 files changed, 25 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/docs/unittests.txt b/docs/unittests.txt
> >>>>> index 7cf2c55ad..6449efd78 100644
> >>>>> --- a/docs/unittests.txt
> >>>>> +++ b/docs/unittests.txt
> >>>>> @@ -42,6 +42,13 @@ For <arch>/ directories that support multiple
> >>>>> architectures, this restricts
> >>>>> the test to the specified arch. By default, the test will run on any
> >>>>> architecture.
> >>>>> +machine
> >>>>> +-------
> >>>>> +For those architectures that support multiple machine types, this
> >>>>> restricts
> >>>>> +the test to the specified machine. By default, the test will run on
> >>>>> +any machine type. (Note, the machine can be specified with the MACHINE=
> >>>>> +environment variable, and defaults to the architecture's default.)
> >>>>> +
> >>>>> smp
> >>>>> ---
> >>>>> smp = <number>
> >>>>> diff --git a/scripts/common.bash b/scripts/common.bash
> >>>>> index 5e9ad53e2..3aa557c8c 100644
> >>>>> --- a/scripts/common.bash
> >>>>> +++ b/scripts/common.bash
> >>>>> @@ -10,6 +10,7 @@ function for_each_unittest()
> >>>>> local opts
> >>>>> local groups
> >>>>> local arch
> >>>>> + local machine
> >>>>> local check
> >>>>> local accel
> >>>>> local timeout
> >>>>> @@ -21,7 +22,7 @@ function for_each_unittest()
> >>>>> if [[ "$line" =~ ^\[(.*)\]$ ]]; then
> >>>>> rematch=${BASH_REMATCH[1]}
> >>>>> if [ -n "${testname}" ]; then
> >>>>> - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp"
> >>>>> "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> >>>>> + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp"
> >>>>> "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> >>>>> fi
> >>>>> testname=$rematch
> >>>>> smp=1
> >>>>> @@ -29,6 +30,7 @@ function for_each_unittest()
> >>>>> opts=""
> >>>>> groups=""
> >>>>> arch=""
> >>>>> + machine=""
> >>>>> check=""
> >>>>> accel=""
> >>>>> timeout=""
> >>>>> @@ -58,6 +60,8 @@ function for_each_unittest()
> >>>>> groups=${BASH_REMATCH[1]}
> >>>>> elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> >>>>> arch=${BASH_REMATCH[1]}
> >>>>> + elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
> >>>>> + machine=${BASH_REMATCH[1]}
> >>>>> elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
> >>>>> check=${BASH_REMATCH[1]}
> >>>>> elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
> >>>>> @@ -67,7 +71,7 @@ function for_each_unittest()
> >>>>> fi
> >>>>> done
> >>>>> if [ -n "${testname}" ]; then
> >>>>> - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel"
> >>>>> "$opts" "$arch" "$check" "$accel" "$timeout"
> >>>>> + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel"
> >>>>> "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
> >>>>> fi
> >>>>> exec {fd}<&-
> >>>>> }
> >>>>> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> >>>>> index 177b62166..0c96d6ea2 100644
> >>>>> --- a/scripts/runtime.bash
> >>>>> +++ b/scripts/runtime.bash
> >>>>> @@ -32,7 +32,7 @@ premature_failure()
> >>>>> get_cmdline()
> >>>>> {
> >>>>> local kernel=$1
> >>>>> - echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel
> >>>>> $RUNTIME_arch_run $kernel -smp $smp $opts"
> >>>>> + echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine
> >>>>> ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
> >>>>> }
> >>>>> skip_nodefault()
> >>>>> @@ -80,9 +80,10 @@ function run()
> >>>>> local kernel="$4"
> >>>>> local opts="$5"
> >>>>> local arch="$6"
> >>>>> - local check="${CHECK:-$7}"
> >>>>> - local accel="$8"
> >>>>> - local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
> >>>>> + local machine="$7"
> >>>>> + local check="${CHECK:-$8}"
> >>>>> + local accel="$9"
> >>>>> + local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
> >>>>> if [ "${CONFIG_EFI}" == "y" ]; then
> >>>>> kernel=${kernel/%.flat/.efi}
> >>>>> @@ -116,6 +117,13 @@ function run()
> >>>>> return 2
> >>>>> fi
> >>>>> + if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" !=
> >>>>> "$MACHINE" ]; then
> >>>>> + print_result "SKIP" $testname "" "$machine only"
> >>>>> + return 2
> >>>>> + elif [ -n "$MACHINE" ]; then
> >>>>> + machine="$MACHINE"
> >>>>> + fi
> >>>>> +
> >>>>> if [ -n "$accel" ] && [ -n "$ACCEL" ] && [ "$accel" != "$ACCEL"
> >>>>> ]; then
> >>>>> print_result "SKIP" $testname "" "$accel only, but
> >>>>> ACCEL=$ACCEL"
> >>>>> return 2
> >>>>
> >>>> For some reasons that I don't quite understand yet, this patch causes the
> >>>> "sieve" test to always timeout on the s390x runner, see e.g.:
> >>>>
> >>>> https://gitlab.com/thuth/kvm-unit-tests/-/jobs/6798954987
> >>>
> >>> How do you use the s390x runner?
> >>>
> >>>>
> >>>> Everything is fine in the previous patches (I pushed now the previous 5
> >>>> patches to the repo):
> >>>>
> >>>> https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/pipelines/1281919104
> >>>>
> >>>> Could it be that he TIMEOUT gets messed up in certain cases?
> >>>
> >>> Hmm not sure yet. At least it got timeout right for the duration=90s
> >>> message.
> >>
> >> That seems to be wrong, the test is declared like this in
> >> s390x/unittests.cfg :
> >>
> >> [sieve]
> >> file = sieve.elf
> >> groups = selftest
> >> # can take fairly long when KVM is nested inside z/VM
> >> timeout = 600
> >>
> >> And indeed, it takes way longer than 90 seconds on that CI machine, so the
> >> timeout after 90 seconds should not occur here...
> >
> > I guess you need to adjust arch_cmd_s390x in scripts/s390x/func.bash to be
> > aware of the new parameter, too?
>
> This seems to fix the problem:
Thanks, that looks good.
> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
> index fa47d019..6b817727 100644
> --- a/scripts/s390x/func.bash
> +++ b/scripts/s390x/func.bash
> @@ -13,12 +13,13 @@ function arch_cmd_s390x()
> local kernel=$5
> local opts=$6
> local arch=$7
> - local check=$8
> - local accel=$9
> - local timeout=${10}
> + local machine=$8
> + local check=$9
> + local accel=${10}
> + local timeout=${11}
>
> # run the normal test case
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> + "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"
>
> # run PV test case
> if [ "$accel" = 'tcg' ] || grep -q "migration" <<< "$groups"; then
>
> If you don't like to respin, I can add it to the patch while picking it up?
Yeah I shouldn't resend the full series just for this. If you're happy
to squash it in that would be good.
Thanks,
Nick
next prev parent reply other threads:[~2024-05-09 5:44 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-04 12:28 [kvm-unit-tests PATCH v9 00/31] powerpc improvements Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 01/31] doc: update unittests doc Nicholas Piggin
2024-05-06 7:03 ` Thomas Huth
2024-05-07 3:57 ` Nicholas Piggin
2024-05-06 8:02 ` Andrew Jones
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 02/31] report: Add known failure reporting option Nicholas Piggin
2024-05-06 7:25 ` Thomas Huth
2024-05-06 8:01 ` Andrew Jones
2024-05-06 10:19 ` Thomas Huth
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 03/31] powerpc: Mark known failing tests as kfail Nicholas Piggin
2024-05-06 7:37 ` Thomas Huth
2024-05-07 4:07 ` Nicholas Piggin
2024-05-07 11:44 ` Thomas Huth
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 04/31] powerpc: Update unittests for latest QEMU version Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 05/31] powerpc/sprs: Specify SPRs with data rather than code Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 06/31] powerpc/sprs: Avoid taking PMU interrupts caused by register fuzzing Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg Nicholas Piggin
2024-05-07 15:08 ` Thomas Huth
2024-05-08 12:27 ` Nicholas Piggin
2024-05-08 12:55 ` Thomas Huth
2024-05-08 12:58 ` Thomas Huth
2024-05-08 13:36 ` Thomas Huth
2024-05-09 5:44 ` Nicholas Piggin [this message]
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 08/31] scripts: Accommodate powerpc powernv machine differences Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 09/31] powerpc: Support powernv machine with QEMU TCG Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 10/31] powerpc: Fix emulator illegal instruction test for powernv Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 11/31] powerpc/sprs: Test hypervisor registers on powernv machine Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 12/31] powerpc: general interrupt tests Nicholas Piggin
2024-05-07 12:12 ` Thomas Huth
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 13/31] powerpc: Add rtas stop-self support Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 14/31] powerpc: Remove broken SMP exception stack setup Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 15/31] powerpc: Enable page alloc operations Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 16/31] powerpc: add SMP and IPI support Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 17/31] powerpc: Add cpu_relax Nicholas Piggin
2024-05-07 13:44 ` Thomas Huth
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 18/31] powerpc: Permit ACCEL=tcg,thread=single Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 19/31] powerpc: Avoid using larx/stcx. in spinlocks when only one CPU is running Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 20/31] powerpc: Add atomics tests Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 21/31] powerpc: Add timebase tests Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 22/31] powerpc: Add MMU support Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 23/31] common/sieve: Use vmalloc.h for setup_mmu definition Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 24/31] common/sieve: Support machines without MMU Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 25/31] powerpc: Add sieve.c common test Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 26/31] powerpc: add usermode support Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 27/31] powerpc: add pmu tests Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 28/31] configure: Make arch_libdir a first-class entity Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 29/31] powerpc: Remove remnants of ppc64 directory and build structure Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 30/31] powerpc: Add facility to query TCG or KVM host Nicholas Piggin
2024-05-04 12:28 ` [kvm-unit-tests PATCH v9 31/31] powerpc: gitlab CI update Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D14VIP36ZLPW.334QUWLZMQ5T3@gmail.com \
--to=npiggin@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).