kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
@ 2020-10-13  9:12 Po-Hsu Lin
  2020-10-15 15:59 ` Vitaly Kuznetsov
  0 siblings, 1 reply; 13+ messages in thread
From: Po-Hsu Lin @ 2020-10-13  9:12 UTC (permalink / raw)
  To: kvm, pbonzini; +Cc: po-hsu.lin

We found that on Azure cloud hyperv instance Standard_D48_v3, it will
take about 45 seconds to run this apic test.

It takes even longer (about 150 seconds) to run inside a KVM instance
VM.Standard2.1 on Oracle cloud.

Bump the timeout threshold to give it a chance to finish.

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 x86/unittests.cfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 872d679..c72a659 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -41,7 +41,7 @@ file = apic.flat
 smp = 2
 extra_params = -cpu qemu64,+x2apic,+tsc-deadline
 arch = x86_64
-timeout = 30
+timeout = 240
 
 [ioapic]
 file = ioapic.flat
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-13  9:12 [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test Po-Hsu Lin
@ 2020-10-15 15:59 ` Vitaly Kuznetsov
  2020-10-15 16:35   ` Sean Christopherson
  2020-10-30  8:15   ` Po-Hsu Lin
  0 siblings, 2 replies; 13+ messages in thread
From: Vitaly Kuznetsov @ 2020-10-15 15:59 UTC (permalink / raw)
  To: Po-Hsu Lin; +Cc: po-hsu.lin, kvm, pbonzini

Po-Hsu Lin <po-hsu.lin@canonical.com> writes:

> We found that on Azure cloud hyperv instance Standard_D48_v3, it will
> take about 45 seconds to run this apic test.
>
> It takes even longer (about 150 seconds) to run inside a KVM instance
> VM.Standard2.1 on Oracle cloud.
>
> Bump the timeout threshold to give it a chance to finish.
>
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  x86/unittests.cfg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 872d679..c72a659 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -41,7 +41,7 @@ file = apic.flat
>  smp = 2
>  extra_params = -cpu qemu64,+x2apic,+tsc-deadline
>  arch = x86_64
> -timeout = 30
> +timeout = 240
>  
>  [ioapic]
>  file = ioapic.flat

AFAIR the default timeout for tests where timeout it not set explicitly
is 90s so don't you need to also modify it for other tests like
'apic-split', 'ioapic', 'ioapic-split', ... ?

I was thinking about introducing a 'timeout multiplier' or something to
run_tests.sh for running in slow (read: nested) environments, doing that
would allow us to keep reasonably small timeouts by default. This is
somewhat important as tests tend to hang and waiting for 4 minutes every
time is not great.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-15 15:59 ` Vitaly Kuznetsov
@ 2020-10-15 16:35   ` Sean Christopherson
  2020-10-16 17:02     ` Paolo Bonzini
  2020-10-19 11:32     ` Vitaly Kuznetsov
  2020-10-30  8:15   ` Po-Hsu Lin
  1 sibling, 2 replies; 13+ messages in thread
From: Sean Christopherson @ 2020-10-15 16:35 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: Po-Hsu Lin, kvm, pbonzini

On Thu, Oct 15, 2020 at 05:59:52PM +0200, Vitaly Kuznetsov wrote:
> Po-Hsu Lin <po-hsu.lin@canonical.com> writes:
> 
> > We found that on Azure cloud hyperv instance Standard_D48_v3, it will
> > take about 45 seconds to run this apic test.
> >
> > It takes even longer (about 150 seconds) to run inside a KVM instance
> > VM.Standard2.1 on Oracle cloud.
> >
> > Bump the timeout threshold to give it a chance to finish.
> >
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >  x86/unittests.cfg | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> > index 872d679..c72a659 100644
> > --- a/x86/unittests.cfg
> > +++ b/x86/unittests.cfg
> > @@ -41,7 +41,7 @@ file = apic.flat
> >  smp = 2
> >  extra_params = -cpu qemu64,+x2apic,+tsc-deadline
> >  arch = x86_64
> > -timeout = 30
> > +timeout = 240
> >  
> >  [ioapic]
> >  file = ioapic.flat
> 
> AFAIR the default timeout for tests where timeout it not set explicitly
> is 90s so don't you need to also modify it for other tests like
> 'apic-split', 'ioapic', 'ioapic-split', ... ?
> 
> I was thinking about introducing a 'timeout multiplier' or something to
> run_tests.sh for running in slow (read: nested) environments, doing that
> would allow us to keep reasonably small timeouts by default. This is
> somewhat important as tests tend to hang and waiting for 4 minutes every
> time is not great.

I would much prefer to go in the other direction and make tests like APIC not
do so many loops (in a nested environment?).  The port80 test in particular is
an absolute waste of time.

E.g. does running 1M loops in test_multiple_nmi() really add value versus
say 10k or 100k loops?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-15 16:35   ` Sean Christopherson
@ 2020-10-16 17:02     ` Paolo Bonzini
  2020-10-16 17:40       ` Andrew Jones
  2020-10-19 11:32     ` Vitaly Kuznetsov
  1 sibling, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2020-10-16 17:02 UTC (permalink / raw)
  To: Sean Christopherson, Vitaly Kuznetsov; +Cc: Po-Hsu Lin, kvm

On 15/10/20 18:35, Sean Christopherson wrote:
> The port80 test in particular is an absolute waste of time.
> 

True, OTOH it was meant as a benchmark.  I think we can just delete it
or move it to vmexit.

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-16 17:02     ` Paolo Bonzini
@ 2020-10-16 17:40       ` Andrew Jones
  2020-10-20  5:53         ` Thomas Huth
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Jones @ 2020-10-16 17:40 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Sean Christopherson, Vitaly Kuznetsov, Po-Hsu Lin, kvm

On Fri, Oct 16, 2020 at 07:02:57PM +0200, Paolo Bonzini wrote:
> On 15/10/20 18:35, Sean Christopherson wrote:
> > The port80 test in particular is an absolute waste of time.
> > 
> 
> True, OTOH it was meant as a benchmark.  I think we can just delete it
> or move it to vmexit.
>

If you want to keep the code, but only run it manually sometimes,
then you can mark the test as nodefault.

Thanks,
drew 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-15 16:35   ` Sean Christopherson
  2020-10-16 17:02     ` Paolo Bonzini
@ 2020-10-19 11:32     ` Vitaly Kuznetsov
  2020-10-19 16:37       ` Christopherson, Sean J
  1 sibling, 1 reply; 13+ messages in thread
From: Vitaly Kuznetsov @ 2020-10-19 11:32 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Po-Hsu Lin, kvm, pbonzini

Sean Christopherson <sean.j.christopherson@intel.com> writes:

> On Thu, Oct 15, 2020 at 05:59:52PM +0200, Vitaly Kuznetsov wrote:
>> Po-Hsu Lin <po-hsu.lin@canonical.com> writes:
>> 
>> > We found that on Azure cloud hyperv instance Standard_D48_v3, it will
>> > take about 45 seconds to run this apic test.
>> >
>> > It takes even longer (about 150 seconds) to run inside a KVM instance
>> > VM.Standard2.1 on Oracle cloud.
>> >
>> > Bump the timeout threshold to give it a chance to finish.
>> >
>> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
>> > ---
>> >  x86/unittests.cfg | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/x86/unittests.cfg b/x86/unittests.cfg
>> > index 872d679..c72a659 100644
>> > --- a/x86/unittests.cfg
>> > +++ b/x86/unittests.cfg
>> > @@ -41,7 +41,7 @@ file = apic.flat
>> >  smp = 2
>> >  extra_params = -cpu qemu64,+x2apic,+tsc-deadline
>> >  arch = x86_64
>> > -timeout = 30
>> > +timeout = 240
>> >  
>> >  [ioapic]
>> >  file = ioapic.flat
>> 
>> AFAIR the default timeout for tests where timeout it not set explicitly
>> is 90s so don't you need to also modify it for other tests like
>> 'apic-split', 'ioapic', 'ioapic-split', ... ?
>> 
>> I was thinking about introducing a 'timeout multiplier' or something to
>> run_tests.sh for running in slow (read: nested) environments, doing that
>> would allow us to keep reasonably small timeouts by default. This is
>> somewhat important as tests tend to hang and waiting for 4 minutes every
>> time is not great.
>
> I would much prefer to go in the other direction and make tests like APIC not
> do so many loops (in a nested environment?). The port80 test in particular is
> an absolute waste of time.

I don't think these two suggestions are opposite. Yes, making tests run fast
is good, however, some of the tests are doomed to be slow. E.g. running
VMX testsuite while nested (leaving aside the question about who needs
three level nesting) is always going to be much slower than on bare metal.

>
> E.g. does running 1M loops in test_multiple_nmi() really add value versus
> say 10k or 100k loops?

Oddly enough, I vaguely remember this particular test hanging
*sometimes* after a few thousand loops but I don't remember any
details.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-19 11:32     ` Vitaly Kuznetsov
@ 2020-10-19 16:37       ` Christopherson, Sean J
  2020-10-19 16:52         ` Nadav Amit
  0 siblings, 1 reply; 13+ messages in thread
From: Christopherson, Sean J @ 2020-10-19 16:37 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: Po-Hsu Lin, kvm, pbonzini

On Mon, Oct 19, 2020 at 01:32:00PM +0200, Vitaly Kuznetsov wrote:
> Sean Christopherson <sean.j.christopherson@intel.com> writes:
> >> > diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> >> > index 872d679..c72a659 100644
> >> > --- a/x86/unittests.cfg
> >> > +++ b/x86/unittests.cfg
> >> > @@ -41,7 +41,7 @@ file = apic.flat
> >> >  smp = 2
> >> >  extra_params = -cpu qemu64,+x2apic,+tsc-deadline
> >> >  arch = x86_64
> >> > -timeout = 30
> >> > +timeout = 240
> >> >
> >> >  [ioapic]
> >> >  file = ioapic.flat
> >>
> >> AFAIR the default timeout for tests where timeout it not set explicitly
> >> is 90s so don't you need to also modify it for other tests like
> >> 'apic-split', 'ioapic', 'ioapic-split', ... ?
> >>
> >> I was thinking about introducing a 'timeout multiplier' or something to
> >> run_tests.sh for running in slow (read: nested) environments, doing that
> >> would allow us to keep reasonably small timeouts by default. This is
> >> somewhat important as tests tend to hang and waiting for 4 minutes every
> >> time is not great.
> >
> > I would much prefer to go in the other direction and make tests like APIC not
> > do so many loops (in a nested environment?). The port80 test in particular is
> > an absolute waste of time.
>
> I don't think these two suggestions are opposite. Yes, making tests run fast
> is good, however, some of the tests are doomed to be slow. E.g. running
> VMX testsuite while nested (leaving aside the question about who needs
> three level nesting) is always going to be much slower than on bare metal.

Ya, I was specifically referring to tests that arbitrarily choose a high loop
count, without any real/documented justification for running millions of loops.

> > E.g. does running 1M loops in test_multiple_nmi() really add value versus
> > say 10k or 100k loops?
>
> Oddly enough, I vaguely remember this particular test hanging
> *sometimes* after a few thousand loops but I don't remember any
> details.

Thousands still ain't millions :-D.

IMO, the unit tests should sit between a smoke test and a long running,
intensive stress test, i.e. the default config shouldn't be trying to find
literal one-in-a-million bugs on every run.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-19 16:37       ` Christopherson, Sean J
@ 2020-10-19 16:52         ` Nadav Amit
  2020-10-20  8:48           ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Nadav Amit @ 2020-10-19 16:52 UTC (permalink / raw)
  To: Christopherson, Sean J; +Cc: Vitaly Kuznetsov, Po-Hsu Lin, kvm, pbonzini

> On Oct 19, 2020, at 9:37 AM, Christopherson, Sean J <sean.j.christopherson@intel.com> wrote:
> 
> On Mon, Oct 19, 2020 at 01:32:00PM +0200, Vitaly Kuznetsov wrote:
>> Sean Christopherson <sean.j.christopherson@intel.com> writes:
>>> E.g. does running 1M loops in test_multiple_nmi() really add value versus
>>> say 10k or 100k loops?
>> 
>> Oddly enough, I vaguely remember this particular test hanging
>> *sometimes* after a few thousand loops but I don't remember any
>> details.
> 
> Thousands still ain't millions :-D.
> 
> IMO, the unit tests should sit between a smoke test and a long running,
> intensive stress test, i.e. the default config shouldn't be trying to find
> literal one-in-a-million bugs on every run.

IIRC, this test failed on VMware, and according to our previous discussions,
does not follow the SDM as NMIs might be collapsed [1].

[1] https://marc.info/?l=kvm&m=145876994031502&w=2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-16 17:40       ` Andrew Jones
@ 2020-10-20  5:53         ` Thomas Huth
  2020-10-20  8:49           ` Andrew Jones
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Huth @ 2020-10-20  5:53 UTC (permalink / raw)
  To: Andrew Jones, Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Po-Hsu Lin, kvm

On 16/10/2020 19.40, Andrew Jones wrote:
> On Fri, Oct 16, 2020 at 07:02:57PM +0200, Paolo Bonzini wrote:
>> On 15/10/20 18:35, Sean Christopherson wrote:
>>> The port80 test in particular is an absolute waste of time.
>>>
>>
>> True, OTOH it was meant as a benchmark.  I think we can just delete it
>> or move it to vmexit.
>>
> 
> If you want to keep the code, but only run it manually sometimes,
> then you can mark the test as nodefault.

Please let's avoid that. Code that does not get run by default tends to
bitrot. I suggest to decrease the amount of loops by default, and if
somebody still wants to run this as a kind of benchmark, maybe the amount of
loops could be made configurable? (i.e. so that you could control it via an
argv[] parameter?)

 Thomas


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-19 16:52         ` Nadav Amit
@ 2020-10-20  8:48           ` Paolo Bonzini
  2020-10-20 15:05             ` Christopherson, Sean J
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2020-10-20  8:48 UTC (permalink / raw)
  To: Nadav Amit, Christopherson, Sean J; +Cc: Vitaly Kuznetsov, Po-Hsu Lin, kvm

On 19/10/20 18:52, Nadav Amit wrote:
> IIRC, this test failed on VMware, and according to our previous discussions,
> does not follow the SDM as NMIs might be collapsed [1].
> 
> [1] https://marc.info/?l=kvm&m=145876994031502&w=2

So should KVM be changed to always collapse NMIs, like this?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 105261402921..4032336ecba3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -668,7 +668,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_emulated_page_fault);

 void kvm_inject_nmi(struct kvm_vcpu *vcpu)
 {
-	atomic_inc(&vcpu->arch.nmi_queued);
+	atomic_set(&vcpu->arch.nmi_queued, 1);
 	kvm_make_request(KVM_REQ_NMI, vcpu);
 }
 EXPORT_SYMBOL_GPL(kvm_inject_nmi);
@@ -8304,18 +8304,7 @@ static void inject_pending_event(struct kvm_vcpu
*vcpu, bool *req_immediate_exit

 static void process_nmi(struct kvm_vcpu *vcpu)
 {
-	unsigned limit = 2;
-
-	/*
-	 * x86 is limited to one NMI running, and one NMI pending after it.
-	 * If an NMI is already in progress, limit further NMIs to just one.
-	 * Otherwise, allow two (and we'll inject the first one immediately).
-	 */
-	if (kvm_x86_ops.get_nmi_mask(vcpu) || vcpu->arch.nmi_injected)
-		limit = 1;
-
-	vcpu->arch.nmi_pending += atomic_xchg(&vcpu->arch.nmi_queued, 0);
-	vcpu->arch.nmi_pending = min(vcpu->arch.nmi_pending, limit);
+	vcpu->arch.nmi_pending |= atomic_xchg(&vcpu->arch.nmi_queued, 0);
 	kvm_make_request(KVM_REQ_EVENT, vcpu);
 }


Paolo


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-20  5:53         ` Thomas Huth
@ 2020-10-20  8:49           ` Andrew Jones
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Jones @ 2020-10-20  8:49 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Po-Hsu Lin, kvm

On Tue, Oct 20, 2020 at 07:53:49AM +0200, Thomas Huth wrote:
> On 16/10/2020 19.40, Andrew Jones wrote:
> > On Fri, Oct 16, 2020 at 07:02:57PM +0200, Paolo Bonzini wrote:
> >> On 15/10/20 18:35, Sean Christopherson wrote:
> >>> The port80 test in particular is an absolute waste of time.
> >>>
> >>
> >> True, OTOH it was meant as a benchmark.  I think we can just delete it
> >> or move it to vmexit.
> >>
> > 
> > If you want to keep the code, but only run it manually sometimes,
> > then you can mark the test as nodefault.
> 
> Please let's avoid that. Code that does not get run by default tends to
> bitrot. I suggest to decrease the amount of loops by default, and if
> somebody still wants to run this as a kind of benchmark, maybe the amount of
> loops could be made configurable? (i.e. so that you could control it via an
> argv[] parameter?)
>

I think both make sense. Making the number of loops variable is a good
idea in order to keep the test running in CI in a reasonable amount of
time (the timeout can also be adjusted by the CI runner, of course). Also,
marking the test as nodefault makes sense if nobody really cares about
the output unless they're specifically doing some benchmarking. Nothing
stops travis or other CI from running nodefault tests, they just have to
be explicitly requested.

Thanks,
drew 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-20  8:48           ` Paolo Bonzini
@ 2020-10-20 15:05             ` Christopherson, Sean J
  0 siblings, 0 replies; 13+ messages in thread
From: Christopherson, Sean J @ 2020-10-20 15:05 UTC (permalink / raw)
  To: Paolo Bonzini, Nadav Amit; +Cc: Vitaly Kuznetsov, Po-Hsu Lin, kvm

On Tue, Oct 20, 2020 at 10:48:24AM +0200, Paolo Bonzini wrote:
> On 19/10/20 18:52, Nadav Amit wrote:
> > IIRC, this test failed on VMware, and according to our previous discussions,
> > does not follow the SDM as NMIs might be collapsed [1].
> >
> > [1] https://marc.info/?l=kvm&m=145876994031502&w=2
>
> So should KVM be changed to always collapse NMIs, like this?

No, Nadav's failure is not on bare metal.  The test passes on bare metal.

Quoting myself:

  Architecturally I don't think there are any guarantees regarding
  simultaneous NMIs, but practically speaking the probability of NMIs
  being collapsed (on hardware) when NMIs aren't blocked is nil.  So while
  it may be architecturally legal for a VMM to drop an NMI in this case,
  it's reasonable for software to expect two NMIs to be received.


[*] https://lkml.kernel.org/r/A7453828-BD8E-43F8-B140-6D660535B7F2@gmail.com

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 105261402921..4032336ecba3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -668,7 +668,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_emulated_page_fault);
>
>  void kvm_inject_nmi(struct kvm_vcpu *vcpu)
>  {
> -     atomic_inc(&vcpu->arch.nmi_queued);
> +     atomic_set(&vcpu->arch.nmi_queued, 1);
>       kvm_make_request(KVM_REQ_NMI, vcpu);
>  }
>  EXPORT_SYMBOL_GPL(kvm_inject_nmi);
> @@ -8304,18 +8304,7 @@ static void inject_pending_event(struct kvm_vcpu
> *vcpu, bool *req_immediate_exit
>
>  static void process_nmi(struct kvm_vcpu *vcpu)
>  {
> -     unsigned limit = 2;
> -
> -     /*
> -      * x86 is limited to one NMI running, and one NMI pending after it.
> -      * If an NMI is already in progress, limit further NMIs to just one.
> -      * Otherwise, allow two (and we'll inject the first one immediately).
> -      */
> -     if (kvm_x86_ops.get_nmi_mask(vcpu) || vcpu->arch.nmi_injected)
> -             limit = 1;
> -
> -     vcpu->arch.nmi_pending += atomic_xchg(&vcpu->arch.nmi_queued, 0);
> -     vcpu->arch.nmi_pending = min(vcpu->arch.nmi_pending, limit);
> +     vcpu->arch.nmi_pending |= atomic_xchg(&vcpu->arch.nmi_queued, 0);
>       kvm_make_request(KVM_REQ_EVENT, vcpu);
>  }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test
  2020-10-15 15:59 ` Vitaly Kuznetsov
  2020-10-15 16:35   ` Sean Christopherson
@ 2020-10-30  8:15   ` Po-Hsu Lin
  1 sibling, 0 replies; 13+ messages in thread
From: Po-Hsu Lin @ 2020-10-30  8:15 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm, Paolo Bonzini

On Thu, Oct 15, 2020 at 11:59 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Po-Hsu Lin <po-hsu.lin@canonical.com> writes:
>
> > We found that on Azure cloud hyperv instance Standard_D48_v3, it will
> > take about 45 seconds to run this apic test.
> >
> > It takes even longer (about 150 seconds) to run inside a KVM instance
> > VM.Standard2.1 on Oracle cloud.
> >
> > Bump the timeout threshold to give it a chance to finish.
> >
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >  x86/unittests.cfg | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> > index 872d679..c72a659 100644
> > --- a/x86/unittests.cfg
> > +++ b/x86/unittests.cfg
> > @@ -41,7 +41,7 @@ file = apic.flat
> >  smp = 2
> >  extra_params = -cpu qemu64,+x2apic,+tsc-deadline
> >  arch = x86_64
> > -timeout = 30
> > +timeout = 240
> >
> >  [ioapic]
> >  file = ioapic.flat
>
> AFAIR the default timeout for tests where timeout it not set explicitly
> is 90s so don't you need to also modify it for other tests like
> 'apic-split', 'ioapic', 'ioapic-split', ... ?
>
Hello,
interesting thing is that this apic test is the only one that will timeout.
For Azure cloud hyperv instance Standard_D48_v3 (kernel 5.4 on Bionic):
apic-split - 31.892s
ioapic - 1.187s
ioapic-split - 1.212s
vmx_apic_passthrough_thread - 2.099s
vmx_apic_passthrough_tpr_threshold_test - Failed (KVM: entry failed,
hardware error 0x0)

So I think bumping the timeout just for this test should be enough.
Thanks

> I was thinking about introducing a 'timeout multiplier' or something to
> run_tests.sh for running in slow (read: nested) environments, doing that
> would allow us to keep reasonably small timeouts by default. This is
> somewhat important as tests tend to hang and waiting for 4 minutes every
> time is not great.
>
> --
> Vitaly
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-30  8:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13  9:12 [kvm-unit-tests PATCHv2] unittests.cfg: Increase timeout for apic test Po-Hsu Lin
2020-10-15 15:59 ` Vitaly Kuznetsov
2020-10-15 16:35   ` Sean Christopherson
2020-10-16 17:02     ` Paolo Bonzini
2020-10-16 17:40       ` Andrew Jones
2020-10-20  5:53         ` Thomas Huth
2020-10-20  8:49           ` Andrew Jones
2020-10-19 11:32     ` Vitaly Kuznetsov
2020-10-19 16:37       ` Christopherson, Sean J
2020-10-19 16:52         ` Nadav Amit
2020-10-20  8:48           ` Paolo Bonzini
2020-10-20 15:05             ` Christopherson, Sean J
2020-10-30  8:15   ` Po-Hsu Lin

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 NNTP newsgroup(s).