Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1736595 > unrolled thread

[patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-21 13:50 +0200
    Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2017-09-21 15:40 +0200
      Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-21 16:10 +0200
        Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-22 03:20 +0200
          Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-22 12:10 +0200
            Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-22 13:00 +0200
              Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-22 14:40 +0200
                Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-22 15:00 +0200
                  Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Paolo Bonzini <pbonzini@redhat.com> - 2017-09-23 13:00 +0200
                    Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-23 15:50 +0200
                      Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Paolo Bonzini <pbonzini@redhat.com> - 2017-09-24 15:10 +0200
                        Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-25 05:00 +0200
                          Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-25 11:20 +0200
                            Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Paolo Bonzini <pbonzini@redhat.com> - 2017-09-25 17:20 +0200
                          Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2017-09-25 18:30 +0200
            Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-22 14:20 +0200
              Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-22 14:40 +0200
                Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-22 14:40 +0200
                  Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-22 15:10 +0200
                    Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-25 04:30 +0200
                      Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall Peter Zijlstra <peterz@infradead.org> - 2017-09-25 10:40 +0200
                Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall\ Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-22 14:50 +0200
                  Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall\ Peter Zijlstra <peterz@infradead.org> - 2017-09-22 15:10 +0200
                    Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall\ Marcelo Tosatti <mtosatti@redhat.com> - 2017-09-25 04:30 +0200
                      Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall\ Peter Zijlstra <peterz@infradead.org> - 2017-09-25 11:00 +0200
                      Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall\ Thomas Gleixner <tglx@linutronix.de> - 2017-09-25 12:50 +0200
                        Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO  hypercall\ Jan Kiszka <jan.kiszka@siemens.com> - 2017-09-25 20:40 +0200

Page 1 of 2  [1] 2  Next page →

#1736595 — [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

Add hypercalls to spinlock/unlock to set/unset FIFO priority
for the vcpu, protected by a static branch to avoid performance
increase in the normal kernels.

Enable option by "kvmfifohc" kernel command line parameter (disabled
by default).

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

---
 arch/x86/kernel/kvm.c            |   31 +++++++++++++++++++++++++++++++
 include/linux/spinlock.h         |    2 +-
 include/linux/spinlock_api_smp.h |   17 +++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

Index: kvm.fifopriohc-submit/arch/x86/kernel/kvm.c
===================================================================
--- kvm.fifopriohc-submit.orig/arch/x86/kernel/kvm.c
+++ kvm.fifopriohc-submit/arch/x86/kernel/kvm.c
@@ -37,6 +37,7 @@
 #include <linux/debugfs.h>
 #include <linux/nmi.h>
 #include <linux/swait.h>
+#include <linux/static_key.h>
 #include <asm/timer.h>
 #include <asm/cpu.h>
 #include <asm/traps.h>
@@ -321,6 +322,36 @@ static notrace void kvm_guest_apic_eoi_w
 	apic->native_eoi_write(APIC_EOI, APIC_EOI_ACK);
 }
 
+static int kvmfifohc;
+
+static int parse_kvmfifohc(char *arg)
+{
+	kvmfifohc = 1;
+	return 0;
+}
+
+early_param("kvmfifohc", parse_kvmfifohc);
+
+DEFINE_STATIC_KEY_FALSE(kvm_fifo_hc_key);
+
+static void kvm_init_fifo_hc(void)
+{
+	long ret;
+
+	ret = kvm_hypercall1(KVM_HC_RT_PRIO, 0);
+
+	if (ret == 0 && kvmfifohc == 1)
+		static_branch_enable(&kvm_fifo_hc_key);
+}
+
+static __init int kvmguest_late_init(void)
+{
+	kvm_init_fifo_hc();
+	return 0;
+}
+
+late_initcall(kvmguest_late_init);
+
 static void kvm_guest_cpu_init(void)
 {
 	if (!kvm_para_available())
Index: kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
===================================================================
--- kvm.fifopriohc-submit.orig/include/linux/spinlock_api_smp.h
+++ kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
@@ -136,11 +136,28 @@ static inline void __raw_spin_lock_bh(ra
 	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
 }
 
+#ifdef CONFIG_KVM_GUEST
+DECLARE_STATIC_KEY_FALSE(kvm_fifo_hc_key);
+#endif
+
 static inline void __raw_spin_lock(raw_spinlock_t *lock)
 {
 	preempt_disable();
+
+#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
+	/* enable FIFO priority */
+	if (static_branch_unlikely(&kvm_fifo_hc_key))
+		kvm_hypercall1(KVM_HC_RT_PRIO, 0x1);
+#endif
+
 	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
 	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
+
+#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
+	/* disable FIFO priority */
+	if (static_branch_unlikely(&kvm_fifo_hc_key))
+		kvm_hypercall1(KVM_HC_RT_PRIO, 0);
+#endif
 }
 
 #endif /* !CONFIG_GENERIC_LOCKBREAK || CONFIG_DEBUG_LOCK_ALLOC */
Index: kvm.fifopriohc-submit/include/linux/spinlock.h
===================================================================
--- kvm.fifopriohc-submit.orig/include/linux/spinlock.h
+++ kvm.fifopriohc-submit/include/linux/spinlock.h
@@ -56,7 +56,7 @@
 #include <linux/stringify.h>
 #include <linux/bottom_half.h>
 #include <asm/barrier.h>
-
+#include <uapi/linux/kvm_para.h>
 
 /*
  * Must define these before including other files, inline functions need them

[toc] | [next] | [standalone]

#1736659 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Thu, Sep 21, 2017 at 08:38:38AM -0300, Marcelo Tosatti wrote:
> Add hypercalls to spinlock/unlock to set/unset FIFO priority
> for the vcpu, protected by a static branch to avoid performance
> increase in the normal kernels.
> 
> Enable option by "kvmfifohc" kernel command line parameter (disabled
> by default).

Wouldn't be better if there was a global 'kvm=' which could have the
various overrides?

> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> ---
>  arch/x86/kernel/kvm.c            |   31 +++++++++++++++++++++++++++++++
>  include/linux/spinlock.h         |    2 +-
>  include/linux/spinlock_api_smp.h |   17 +++++++++++++++++

Hm. It looks like you forgot to CC the maintainers:

$ scripts/get_maintainer.pl -f include/linux/spinlock.h
Peter Zijlstra <peterz@infradead.org> (maintainer:LOCKING PRIMITIVES)
Ingo Molnar <mingo@redhat.com> (maintainer:LOCKING PRIMITIVES)
linux-kernel@vger.kernel.org (open list:LOCKING PRIMITIVES)

Doing that for you.

>  3 files changed, 49 insertions(+), 1 deletion(-)
> 
> Index: kvm.fifopriohc-submit/arch/x86/kernel/kvm.c
> ===================================================================
> --- kvm.fifopriohc-submit.orig/arch/x86/kernel/kvm.c
> +++ kvm.fifopriohc-submit/arch/x86/kernel/kvm.c
> @@ -37,6 +37,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/nmi.h>
>  #include <linux/swait.h>
> +#include <linux/static_key.h>
>  #include <asm/timer.h>
>  #include <asm/cpu.h>
>  #include <asm/traps.h>
> @@ -321,6 +322,36 @@ static notrace void kvm_guest_apic_eoi_w
>  	apic->native_eoi_write(APIC_EOI, APIC_EOI_ACK);
>  }
>  
> +static int kvmfifohc;
> +
> +static int parse_kvmfifohc(char *arg)
> +{
> +	kvmfifohc = 1;
> +	return 0;
> +}
> +
> +early_param("kvmfifohc", parse_kvmfifohc);
> +
> +DEFINE_STATIC_KEY_FALSE(kvm_fifo_hc_key);
> +
> +static void kvm_init_fifo_hc(void)
> +{
> +	long ret;
> +
> +	ret = kvm_hypercall1(KVM_HC_RT_PRIO, 0);
> +
> +	if (ret == 0 && kvmfifohc == 1)
> +		static_branch_enable(&kvm_fifo_hc_key);
> +}
> +
> +static __init int kvmguest_late_init(void)
> +{
> +	kvm_init_fifo_hc();
> +	return 0;
> +}
> +
> +late_initcall(kvmguest_late_init);
> +
>  static void kvm_guest_cpu_init(void)
>  {
>  	if (!kvm_para_available())
> Index: kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
> ===================================================================
> --- kvm.fifopriohc-submit.orig/include/linux/spinlock_api_smp.h
> +++ kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
> @@ -136,11 +136,28 @@ static inline void __raw_spin_lock_bh(ra
>  	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
>  }
>  
> +#ifdef CONFIG_KVM_GUEST
> +DECLARE_STATIC_KEY_FALSE(kvm_fifo_hc_key);
> +#endif
> +
>  static inline void __raw_spin_lock(raw_spinlock_t *lock)
>  {
>  	preempt_disable();
> +
> +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
> +	/* enable FIFO priority */
> +	if (static_branch_unlikely(&kvm_fifo_hc_key))
> +		kvm_hypercall1(KVM_HC_RT_PRIO, 0x1);
> +#endif

I am assuming the reason you choose not to wrap this in a pvops
or any other structure that is more of hypervisor agnostic is
that only KVM exposes this. But what if other hypervisors expose
something similar? Or some other mechanism similar to this?

> +
>  	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
>  	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> +
> +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
> +	/* disable FIFO priority */
> +	if (static_branch_unlikely(&kvm_fifo_hc_key))
> +		kvm_hypercall1(KVM_HC_RT_PRIO, 0);
> +#endif
>  }
>  
>  #endif /* !CONFIG_GENERIC_LOCKBREAK || CONFIG_DEBUG_LOCK_ALLOC */
> Index: kvm.fifopriohc-submit/include/linux/spinlock.h
> ===================================================================
> --- kvm.fifopriohc-submit.orig/include/linux/spinlock.h
> +++ kvm.fifopriohc-submit/include/linux/spinlock.h
> @@ -56,7 +56,7 @@
>  #include <linux/stringify.h>
>  #include <linux/bottom_half.h>
>  #include <asm/barrier.h>
> -
> +#include <uapi/linux/kvm_para.h>
>  
>  /*
>   * Must define these before including other files, inline functions need them
> 
> 

[toc] | [prev] | [next] | [standalone]

#1736695 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Thu, Sep 21, 2017 at 09:36:53AM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 21, 2017 at 08:38:38AM -0300, Marcelo Tosatti wrote:
> > Add hypercalls to spinlock/unlock to set/unset FIFO priority
> > for the vcpu, protected by a static branch to avoid performance
> > increase in the normal kernels.
> > 
> > Enable option by "kvmfifohc" kernel command line parameter (disabled
> > by default).

WTF kind of fudge is this? Changelog completely fails to explain the
problem this would solve. Why are you doing insane things like this?


NAK!

> > Index: kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
> > ===================================================================
> > --- kvm.fifopriohc-submit.orig/include/linux/spinlock_api_smp.h
> > +++ kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
> > @@ -136,11 +136,28 @@ static inline void __raw_spin_lock_bh(ra
> >  	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> >  }
> >  
> > +#ifdef CONFIG_KVM_GUEST
> > +DECLARE_STATIC_KEY_FALSE(kvm_fifo_hc_key);
> > +#endif
> > +
> >  static inline void __raw_spin_lock(raw_spinlock_t *lock)
> >  {
> >  	preempt_disable();
> > +
> > +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
> > +	/* enable FIFO priority */
> > +	if (static_branch_unlikely(&kvm_fifo_hc_key))
> > +		kvm_hypercall1(KVM_HC_RT_PRIO, 0x1);
> > +#endif
> > +
> >  	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
> >  	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> > +
> > +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
> > +	/* disable FIFO priority */
> > +	if (static_branch_unlikely(&kvm_fifo_hc_key))
> > +		kvm_hypercall1(KVM_HC_RT_PRIO, 0);
> > +#endif
> >  }
> >  
> >  #endif /* !CONFIG_GENERIC_LOCKBREAK || CONFIG_DEBUG_LOCK_ALLOC */

[toc] | [prev] | [next] | [standalone]

#1737122 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Thu, Sep 21, 2017 at 04:06:28PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 21, 2017 at 09:36:53AM -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Sep 21, 2017 at 08:38:38AM -0300, Marcelo Tosatti wrote:
> > > Add hypercalls to spinlock/unlock to set/unset FIFO priority
> > > for the vcpu, protected by a static branch to avoid performance
> > > increase in the normal kernels.
> > > 
> > > Enable option by "kvmfifohc" kernel command line parameter (disabled
> > > by default).
> 
> WTF kind of fudge is this? Changelog completely fails to explain the
> problem this would solve. Why are you doing insane things like this?
> 
> 
> NAK!

Copy&pasting from the initial message, please point out whether this
explanation makes sense (better solutions to this problem are welcome):


When executing guest vcpu-0 with FIFO:1 priority, which is necessary
to
deal with the following situation:

VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)

raw_spin_lock(A)
interrupted, schedule task T-1          raw_spin_lock(A) (spin)

raw_spin_unlock(A)

Certain operations must interrupt guest vcpu-0 (see trace below).

To fix this issue, only change guest vcpu-0 to FIFO priority
on spinlock critical sections (see patch).

Hang trace
==========

Without FIFO priority:

qemu-kvm-6705  [002] ....1.. 767785.648964: kvm_exit: reason
IO_INSTRUCTION rip 0xe8fe info 1f00039 0
qemu-kvm-6705  [002] ....1.. 767785.648965: kvm_exit: reason
IO_INSTRUCTION rip 0xe911 info 3f60008 0
qemu-kvm-6705  [002] ....1.. 767785.648968: kvm_exit: reason
IO_INSTRUCTION rip 0x8984 info 608000b 0
qemu-kvm-6705  [002] ....1.. 767785.648971: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
qemu-kvm-6705  [002] ....1.. 767785.648974: kvm_exit: reason
IO_INSTRUCTION rip 0xb514 info 3f60000 0
qemu-kvm-6705  [002] ....1.. 767785.648977: kvm_exit: reason
PENDING_INTERRUPT rip 0x8052 info 0 0
qemu-kvm-6705  [002] ....1.. 767785.648980: kvm_exit: reason
IO_INSTRUCTION rip 0xeee6 info 200040 0
qemu-kvm-6705  [002] ....1.. 767785.648999: kvm_exit: reason
EPT_MISCONFIG rip 0x2120 info 0 0

With FIFO priority:

qemu-kvm-7636  [002] ....1.. 768218.205065: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
qemu-kvm-7636  [002] ....1.. 768218.205068: kvm_exit: reason
IO_INSTRUCTION rip 0x8984 info 608000b 0
qemu-kvm-7636  [002] ....1.. 768218.205071: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
qemu-kvm-7636  [002] ....1.. 768218.205074: kvm_exit: reason
IO_INSTRUCTION rip 0x8984 info 608000b 0
qemu-kvm-7636  [002] ....1.. 768218.205077: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
..

Performance numbers (kernel compilation with make -j2)
======================================================

With hypercall: 4:40.  (make -j2)
Without hypercall: 3:38.  (make -j2)

Note for NFV workloads spinlock performance is not relevant
since DPDK should not enter the kernel (and housekeeping vcpu
performance is far from a key factor).


> 
> > > Index: kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
> > > ===================================================================
> > > --- kvm.fifopriohc-submit.orig/include/linux/spinlock_api_smp.h
> > > +++ kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h
> > > @@ -136,11 +136,28 @@ static inline void __raw_spin_lock_bh(ra
> > >  	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> > >  }
> > >  
> > > +#ifdef CONFIG_KVM_GUEST
> > > +DECLARE_STATIC_KEY_FALSE(kvm_fifo_hc_key);
> > > +#endif
> > > +
> > >  static inline void __raw_spin_lock(raw_spinlock_t *lock)
> > >  {
> > >  	preempt_disable();
> > > +
> > > +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
> > > +	/* enable FIFO priority */
> > > +	if (static_branch_unlikely(&kvm_fifo_hc_key))
> > > +		kvm_hypercall1(KVM_HC_RT_PRIO, 0x1);
> > > +#endif
> > > +
> > >  	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
> > >  	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> > > +
> > > +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP)
> > > +	/* disable FIFO priority */
> > > +	if (static_branch_unlikely(&kvm_fifo_hc_key))
> > > +		kvm_hypercall1(KVM_HC_RT_PRIO, 0);
> > > +#endif
> > >  }
> > >  
> > >  #endif /* !CONFIG_GENERIC_LOCKBREAK || CONFIG_DEBUG_LOCK_ALLOC */

[toc] | [prev] | [next] | [standalone]

#1737373 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> to
> deal with the following situation:
> 
> VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> 
> raw_spin_lock(A)
> interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> 
> raw_spin_unlock(A)
> 
> Certain operations must interrupt guest vcpu-0 (see trace below).

Those traces don't make any sense. All they include is kvm_exit and you
can't tell anything from that.

> To fix this issue, only change guest vcpu-0 to FIFO priority
> on spinlock critical sections (see patch).

This doesn't make sense. So you're saying that if you run all VCPUs as
FIFO things come apart? Why?

And why can't they still come apart when the guest holds a spinlock?

[toc] | [prev] | [next] | [standalone]

#1737407 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 12:00:04PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > to
> > deal with the following situation:
> > 
> > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > 
> > raw_spin_lock(A)
> > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > 
> > raw_spin_unlock(A)
> > 
> > Certain operations must interrupt guest vcpu-0 (see trace below).
> 
> Those traces don't make any sense. All they include is kvm_exit and you
> can't tell anything from that.
> 
> > To fix this issue, only change guest vcpu-0 to FIFO priority
> > on spinlock critical sections (see patch).
> 
> This doesn't make sense. So you're saying that if you run all VCPUs as
> FIFO things come apart? Why?
> 
> And why can't they still come apart when the guest holds a spinlock?

That is, running a RT guest and not having _all_ VCPUs being RT tasks on
the host is absolutely and completely insane and broken.

Fix whatever needs fixing to allow your VCPU0 to be RT, don't do insane
things like this.

[toc] | [prev] | [next] | [standalone]

#1737453 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 12:56:09PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 22, 2017 at 12:00:04PM +0200, Peter Zijlstra wrote:
> > On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > > to
> > > deal with the following situation:
> > > 
> > > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > > 
> > > raw_spin_lock(A)
> > > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > > 
> > > raw_spin_unlock(A)
> > > 
> > > Certain operations must interrupt guest vcpu-0 (see trace below).
> > 
> > Those traces don't make any sense. All they include is kvm_exit and you
> > can't tell anything from that.
> > 
> > > To fix this issue, only change guest vcpu-0 to FIFO priority
> > > on spinlock critical sections (see patch).
> > 
> > This doesn't make sense. So you're saying that if you run all VCPUs as
> > FIFO things come apart? Why?
> > 
> > And why can't they still come apart when the guest holds a spinlock?
> 
> That is, running a RT guest and not having _all_ VCPUs being RT tasks on
> the host is absolutely and completely insane and broken.

Can you explain why, please?

> Fix whatever needs fixing to allow your VCPU0 to be RT, don't do insane
> things like this.

VCPU0 can be RT, but you'll get the following hang, if the emulator
thread is sharing a pCPU with VCPU0:

	1. submit IO.
	2. busy spin.

As executed by the guest vcpu (its a natural problem).

Do you have a better suggestion as how to fix the problem?

We can fix the BIOS, but userspace will still be allowed to
generate the code pattern above.

And increasing the priority of the emulator thread, at random times 
(so it can inject interrupts to vcpu-0), can cause it to interrupt 
vcpu-0 in a spinlock protected section.

The only other option is for customers to live with the decreased 
packing (that is require one pcpu for each vcpu, and an additional pcpu
for emulator threads). Is that what you are suggesting?

[toc] | [prev] | [next] | [standalone]

#1737483 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 09:33:05AM -0300, Marcelo Tosatti wrote:
> > That is, running a RT guest and not having _all_ VCPUs being RT tasks on
> > the host is absolutely and completely insane and broken.
> 
> Can you explain why, please?

You just explained it yourself. If the thread that needs to complete
what you're waiting on has lower priority, it will _never_ get to run if
you're busy waiting on it.

This is _trivial_.

And even for !RT it can be quite costly, because you can end up having
to burn your entire slot of CPU time before you run the other task.

Userspace spinning is _bad_, do not do this.

(the one exception where it works is where you have a single thread per
cpu, because then there's effectively no scheduling).

> > Fix whatever needs fixing to allow your VCPU0 to be RT, don't do insane
> > things like this.
> 
> VCPU0 can be RT, but you'll get the following hang, if the emulator
> thread is sharing a pCPU with VCPU0:
> 
> 	1. submit IO.
> 	2. busy spin.
> 
> As executed by the guest vcpu (its a natural problem).
> 
> Do you have a better suggestion as how to fix the problem?

Yes, not busy wait. Go to sleep and make sure you're woken up once the
IO completes.

> We can fix the BIOS, but userspace will still be allowed to
> generate the code pattern above.

What does the BIOS have to do with anything?

> And increasing the priority of the emulator thread, at random times 
> (so it can inject interrupts to vcpu-0), can cause it to interrupt 
> vcpu-0 in a spinlock protected section.

You can equally boost the emulator thread while you're spin-waiting, but
that's ugly as heck too.

The normal, sane solution is to not spin-wait but block.

[toc] | [prev] | [next] | [standalone]

#1737990 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On 22/09/2017 14:55, Peter Zijlstra wrote:
> You just explained it yourself. If the thread that needs to complete
> what you're waiting on has lower priority, it will _never_ get to run if
> you're busy waiting on it.
> 
> This is _trivial_.
> 
> And even for !RT it can be quite costly, because you can end up having
> to burn your entire slot of CPU time before you run the other task.
> 
> Userspace spinning is _bad_, do not do this.

This is not userspace spinning, it is guest spinning---which has
effectively the same effect but you cannot quite avoid.

But I agree that the solution is properly prioritizing threads that can
interrupt the VCPU, and using PI mutexes.

I'm not a priori opposed to paravirt scheduling primitives, but I am not
at all sure that it's required.

Paolo

> (the one exception where it works is where you have a single thread per
> cpu, because then there's effectively no scheduling).

[toc] | [prev] | [next] | [standalone]

#1738038 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Sat, Sep 23, 2017 at 12:56:12PM +0200, Paolo Bonzini wrote:
> On 22/09/2017 14:55, Peter Zijlstra wrote:
> > You just explained it yourself. If the thread that needs to complete
> > what you're waiting on has lower priority, it will _never_ get to run if
> > you're busy waiting on it.
> > 
> > This is _trivial_.
> > 
> > And even for !RT it can be quite costly, because you can end up having
> > to burn your entire slot of CPU time before you run the other task.
> > 
> > Userspace spinning is _bad_, do not do this.
> 
> This is not userspace spinning, it is guest spinning---which has
> effectively the same effect but you cannot quite avoid.

So I'm virt illiterate and have no clue on how all this works; but
wasn't this a vmexit ? (that's what marcelo traced). And once you've
done a vmexit you're a regular task again, not a vcpu.

> But I agree that the solution is properly prioritizing threads that can
> interrupt the VCPU, and using PI mutexes.

Right, if you want to run RT VCPUs the whole emulator/vcpu interaction
needs to be designed for RT.

> I'm not a priori opposed to paravirt scheduling primitives, but I am not
> at all sure that it's required.

Problem is that the proposed thing doesn't solve anything. There is
nothing that prohibits the guest from triggering a vmexit while holding
a spinlock and landing in the self-same problems.

[toc] | [prev] | [next] | [standalone]

#1738180 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

----- Original Message -----
> From: "Peter Zijlstra" <peterz@infradead.org>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Marcelo Tosatti" <mtosatti@redhat.com>, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>, mingo@redhat.com,
> kvm@vger.kernel.org, linux-kernel@vger.kernel.org, "Thomas Gleixner" <tglx@linutronix.de>
> Sent: Saturday, September 23, 2017 3:41:14 PM
> Subject: Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall
> 
> On Sat, Sep 23, 2017 at 12:56:12PM +0200, Paolo Bonzini wrote:
> > On 22/09/2017 14:55, Peter Zijlstra wrote:
> > > You just explained it yourself. If the thread that needs to complete
> > > what you're waiting on has lower priority, it will _never_ get to run if
> > > you're busy waiting on it.
> > > 
> > > This is _trivial_.
> > > 
> > > And even for !RT it can be quite costly, because you can end up having
> > > to burn your entire slot of CPU time before you run the other task.
> > > 
> > > Userspace spinning is _bad_, do not do this.
> > 
> > This is not userspace spinning, it is guest spinning---which has
> > effectively the same effect but you cannot quite avoid.
> 
> So I'm virt illiterate and have no clue on how all this works; but
> wasn't this a vmexit ? (that's what marcelo traced). And once you've
> done a vmexit you're a regular task again, not a vcpu.

His trace simply shows that the timer tick happened and the SCHED_NORMAL
thread was preempted.  Bumping the vCPU thread to SCHED_FIFO drops
the scheduler tick (the system is NOHZ_FULL) and thus 1) the frequency
of EXTERNAL_INTERRUPT vmexits drops to 1 second 2) the thread is not
preempted anymore.

> > But I agree that the solution is properly prioritizing threads that can
> > interrupt the VCPU, and using PI mutexes.
> 
> Right, if you want to run RT VCPUs the whole emulator/vcpu interaction
> needs to be designed for RT.
> 
> > I'm not a priori opposed to paravirt scheduling primitives, but I am not
> > at all sure that it's required.
> 
> Problem is that the proposed thing doesn't solve anything. There is
> nothing that prohibits the guest from triggering a vmexit while holding
> a spinlock and landing in the self-same problems.

Well, part of configuring virt for RT is (at all levels: host hypervisor+QEMU
and guest kernel+userspace) is that vmexits while holding a spinlock are either
confined to one vCPU or are handled in the host hypervisor very quickly, like
less than 2000 clock cycles.

So I'm not denying that Marcelo's approach solves the problem, but it's very
heavyweight and it masks an important misconfiguration (as you write above,
everything needs to be RT and the priorities must be designed carefully).

_However_, even if you do this, you may want to put the less important vCPUs
and the emulator threads on the same physical CPU.  In that case, the vCPU
can be placed at SCHED_RR to avoid starvation (while the emulator thread needs
to stay at SCHED_FIFO and higher priority).  Some kind of trick that bumps
spinlock critical sections in that vCPU to SCHED_FIFO, for a limited time only,
might still be useful.

Paolo

[toc] | [prev] | [next] | [standalone]

#1738717 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Sun, Sep 24, 2017 at 09:05:44AM -0400, Paolo Bonzini wrote:
> 
> 
> ----- Original Message -----
> > From: "Peter Zijlstra" <peterz@infradead.org>
> > To: "Paolo Bonzini" <pbonzini@redhat.com>
> > Cc: "Marcelo Tosatti" <mtosatti@redhat.com>, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>, mingo@redhat.com,
> > kvm@vger.kernel.org, linux-kernel@vger.kernel.org, "Thomas Gleixner" <tglx@linutronix.de>
> > Sent: Saturday, September 23, 2017 3:41:14 PM
> > Subject: Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall
> > 
> > On Sat, Sep 23, 2017 at 12:56:12PM +0200, Paolo Bonzini wrote:
> > > On 22/09/2017 14:55, Peter Zijlstra wrote:
> > > > You just explained it yourself. If the thread that needs to complete
> > > > what you're waiting on has lower priority, it will _never_ get to run if
> > > > you're busy waiting on it.
> > > > 
> > > > This is _trivial_.
> > > > 
> > > > And even for !RT it can be quite costly, because you can end up having
> > > > to burn your entire slot of CPU time before you run the other task.
> > > > 
> > > > Userspace spinning is _bad_, do not do this.
> > > 
> > > This is not userspace spinning, it is guest spinning---which has
> > > effectively the same effect but you cannot quite avoid.
> > 
> > So I'm virt illiterate and have no clue on how all this works; but
> > wasn't this a vmexit ? (that's what marcelo traced). And once you've
> > done a vmexit you're a regular task again, not a vcpu.
> 
> His trace simply shows that the timer tick happened and the SCHED_NORMAL
> thread was preempted.  Bumping the vCPU thread to SCHED_FIFO drops
> the scheduler tick (the system is NOHZ_FULL) and thus 1) the frequency
> of EXTERNAL_INTERRUPT vmexits drops to 1 second 2) the thread is not
> preempted anymore.
> 
> > > But I agree that the solution is properly prioritizing threads that can
> > > interrupt the VCPU, and using PI mutexes.

Thats exactly what the patch does, the prioritization is not fixed in
time, and depends on whether or not vcpu-0 is in spinlock protected
section.

Are you suggesting a different prioritization? Can you describe it
please, even if incomplete?

> > 
> > Right, if you want to run RT VCPUs the whole emulator/vcpu interaction
> > needs to be designed for RT.
> > 
> > > I'm not a priori opposed to paravirt scheduling primitives, but I am not
> > > at all sure that it's required.
> > 
> > Problem is that the proposed thing doesn't solve anything. There is
> > nothing that prohibits the guest from triggering a vmexit while holding
> > a spinlock and landing in the self-same problems.
> 
> Well, part of configuring virt for RT is (at all levels: host hypervisor+QEMU
> and guest kernel+userspace) is that vmexits while holding a spinlock are either
> confined to one vCPU or are handled in the host hypervisor very quickly, like
> less than 2000 clock cycles.
> 
> So I'm not denying that Marcelo's approach solves the problem, but it's very
> heavyweight and it masks an important misconfiguration (as you write above,
> everything needs to be RT and the priorities must be designed carefully).

I think you are missing the following point:

"vcpu0 can be interrupted when its not in a spinlock protected section, 
otherwise it can't."

So you _have_ to communicate to the host when the guest enters/leaves a
critical section.

So this point of "everything needs to be RT and the priorities must be
designed carefully", is this: 

	WHEN in spinlock protected section (more specifically, when 
	spinlock protected section _shared with realtime vcpus_),

	priority of vcpu0 > priority of emulator thread

	OTHERWISE

	priority of vcpu0 < priority of emulator thread.

(*)

So emulator thread can interrupt and inject interrupts to vcpu0.

> 
> _However_, even if you do this, you may want to put the less important vCPUs
> and the emulator threads on the same physical CPU.  In that case, the vCPU
> can be placed at SCHED_RR to avoid starvation (while the emulator thread needs
> to stay at SCHED_FIFO and higher priority).  Some kind of trick that bumps
> spinlock critical sections in that vCPU to SCHED_FIFO, for a limited time only,
> might still be useful.

Anything that violates (*) above is going to cause excessive latencies
in realtime vcpus, via:

PCPU-0:
	* vcpu-0 grabs spinlock A.
	* event wakes up emulator thread, vcpu-0 sched out, vcpu-0 sched
	in.
PCPU-1:
	* realtime vcpu grabs spinlock-A, busy spins on emulator threads
	completion.

So its more than useful, its necessary.

I'm open to suggestions as better ways to solve this problem 
while sharing emulator thread with vcpu-0 (which is something users
are interested in, for obvious economical reasons), but:

	1) Don't get the point of Peters rejection.

	2) Don't get how SCHED_RR can help the situation.

[toc] | [prev] | [next] | [standalone]

#1738853 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Sun, Sep 24, 2017 at 11:57:53PM -0300, Marcelo Tosatti wrote:
> I think you are missing the following point:
> 
> "vcpu0 can be interrupted when its not in a spinlock protected section, 
> otherwise it can't."
> 
> So you _have_ to communicate to the host when the guest enters/leaves a
> critical section.
> 
> So this point of "everything needs to be RT and the priorities must be
> designed carefully", is this: 
> 
> 	WHEN in spinlock protected section (more specifically, when 
> 	spinlock protected section _shared with realtime vcpus_),
> 
> 	priority of vcpu0 > priority of emulator thread
> 
> 	OTHERWISE
> 
> 	priority of vcpu0 < priority of emulator thread.
> 
> (*)
> 
> So emulator thread can interrupt and inject interrupts to vcpu0.

spinlock protected regions are not everything. What about lock-free
constructs where CPU's spin-wait on one another (there's plenty).

And I'm clearly ignorant of how this emulation thread works, but why
would it run for a long time? Either it is needed for forward progress
of the VCPU or its not. If its not, it shouldn't run.

[toc] | [prev] | [next] | [standalone]

#1739113 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On 25/09/2017 11:13, Peter Zijlstra wrote:
> On Sun, Sep 24, 2017 at 11:57:53PM -0300, Marcelo Tosatti wrote:
>> I think you are missing the following point:
>>
>> "vcpu0 can be interrupted when its not in a spinlock protected section, 
>> otherwise it can't."

Who says that?  Certainly a driver can dedicate a single VCPU to
periodic polling of the device, in such a way that the polling does not
require a spinlock.

>> So you _have_ to communicate to the host when the guest enters/leaves a
>> critical section.
>>
>> So this point of "everything needs to be RT and the priorities must be
>> designed carefully", is this: 
>>
>> 	WHEN in spinlock protected section (more specifically, when 
>> 	spinlock protected section _shared with realtime vcpus_),
>>
>> 	priority of vcpu0 > priority of emulator thread
>>
>> 	OTHERWISE
>>
>> 	priority of vcpu0 < priority of emulator thread.

This is _not_ designed carefully, this is messy.

The emulator thread can interrupt the VCPU thread, so it has to be at
higher RT priority (+ priority inheritance of mutexes).  Once you have
done that we can decide on other approaches that e.g. let you get more
sharing by placing housekeeping VCPUs at SCHED_NORMAL or SCHED_RR.

>> So emulator thread can interrupt and inject interrupts to vcpu0.
> 
> spinlock protected regions are not everything. What about lock-free
> constructs where CPU's spin-wait on one another (there's plenty).
> 
> And I'm clearly ignorant of how this emulation thread works, but why
> would it run for a long time? Either it is needed for forward progress
> of the VCPU or its not. If its not, it shouldn't run.

The emulator thread 1) should not run for long period of times indeed,
and 2) it is needed for forward progress of the VCPU.  So it has to be
at higher RT priority.  I agree with Peter, sorry.  Spinlocks are a red
herring here.

Paolo

[toc] | [prev] | [next] | [standalone]

#1739159 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

> I think you are missing the following point:
> 
> "vcpu0 can be interrupted when its not in a spinlock protected section, 
> otherwise it can't."
> 
> So you _have_ to communicate to the host when the guest enters/leaves a
> critical section.


How would this work for Windows or FreeBSD?

[toc] | [prev] | [next] | [standalone]

#1737442 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 12:00:05PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > to
> > deal with the following situation:
> > 
> > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > 
> > raw_spin_lock(A)
> > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > 
> > raw_spin_unlock(A)
> > 
> > Certain operations must interrupt guest vcpu-0 (see trace below).
> 
> Those traces don't make any sense. All they include is kvm_exit and you
> can't tell anything from that.

Hi Peter,

OK lets describe whats happening:

With QEMU emulator thread and vcpu-0 sharing a physical CPU
(which is a request from several NFV customers, to improve
guest packing), the following occurs when the guest generates 
the following pattern:

		1. submit IO.
		2. busy spin.

Hang trace
==========

Without FIFO priority:

qemu-kvm-6705  [002] ....1.. 767785.648964: kvm_exit: reason
IO_INSTRUCTION rip 0xe8fe info 1f00039 0
qemu-kvm-6705  [002] ....1.. 767785.648965: kvm_exit: reason
IO_INSTRUCTION rip 0xe911 info 3f60008 0
qemu-kvm-6705  [002] ....1.. 767785.648968: kvm_exit: reason
IO_INSTRUCTION rip 0x8984 info 608000b 0
qemu-kvm-6705  [002] ....1.. 767785.648971: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
qemu-kvm-6705  [002] ....1.. 767785.648974: kvm_exit: reason
IO_INSTRUCTION rip 0xb514 info 3f60000 0
qemu-kvm-6705  [002] ....1.. 767785.648977: kvm_exit: reason
PENDING_INTERRUPT rip 0x8052 info 0 0
qemu-kvm-6705  [002] ....1.. 767785.648980: kvm_exit: reason
IO_INSTRUCTION rip 0xeee6 info 200040 0
qemu-kvm-6705  [002] ....1.. 767785.648999: kvm_exit: reason
EPT_MISCONFIG rip 0x2120 info 0 0

The emulator thread is able to interrupt qemu vcpu0 at SCHED_NORMAL
priority.

With FIFO priority:

Now, with qemu vcpu0 at SCHED_FIFO priority, which is necessary to
avoid the following scenario:

(*)
VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
 
raw_spin_lock(A)
interrupted, schedule task T-1          raw_spin_lock(A) (spin)
 
raw_spin_unlock(A)

And the following code pattern by vcpu0:

		1. submit IO.
		2. busy spin.

The emulator thread is unable to interrupt vcpu0 thread
(vcpu0 busy spinning at SCHED_FIFO, emulator thread at SCHED_NORMAL), 
and you get a hang at boot as follows:

qemu-kvm-7636  [002] ....1.. 768218.205065: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
qemu-kvm-7636  [002] ....1.. 768218.205068: kvm_exit: reason
IO_INSTRUCTION rip 0x8984 info 608000b 0
qemu-kvm-7636  [002] ....1.. 768218.205071: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0
qemu-kvm-7636  [002] ....1.. 768218.205074: kvm_exit: reason
IO_INSTRUCTION rip 0x8984 info 608000b 0
qemu-kvm-7636  [002] ....1.. 768218.205077: kvm_exit: reason
IO_INSTRUCTION rip 0xb313 info 1f70008 0

So to fix this problem, the patchset changes the priority
of the VCPU thread (to fix (*)), only when taking spinlocks.

Does that make sense now?

> 
> > To fix this issue, only change guest vcpu-0 to FIFO priority
> > on spinlock critical sections (see patch).
> 
> This doesn't make sense. So you're saying that if you run all VCPUs as
> FIFO things come apart? Why?

Please see above.

> And why can't they still come apart when the guest holds a spinlock?

Hopefully the above makes sense.

[toc] | [prev] | [next] | [standalone]

#1737454 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 09:16:40AM -0300, Marcelo Tosatti wrote:
> On Fri, Sep 22, 2017 at 12:00:05PM +0200, Peter Zijlstra wrote:
> > On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > > to
> > > deal with the following situation:
> > > 
> > > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > > 
> > > raw_spin_lock(A)
> > > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > > 
> > > raw_spin_unlock(A)
> > > 
> > > Certain operations must interrupt guest vcpu-0 (see trace below).
> > 
> > Those traces don't make any sense. All they include is kvm_exit and you
> > can't tell anything from that.
> 
> Hi Peter,
> 
> OK lets describe whats happening:
> 
> With QEMU emulator thread and vcpu-0 sharing a physical CPU
> (which is a request from several NFV customers, to improve
> guest packing), the following occurs when the guest generates 
> the following pattern:
> 
> 		1. submit IO.
> 		2. busy spin.

User-space spinning is a bad idea in general and terminally broken in
a RT setup. Sounds like you need to go fix qemu to not suck.

[toc] | [prev] | [next] | [standalone]

#1737455 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 02:31:07PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 22, 2017 at 09:16:40AM -0300, Marcelo Tosatti wrote:
> > On Fri, Sep 22, 2017 at 12:00:05PM +0200, Peter Zijlstra wrote:
> > > On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > > > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > > > to
> > > > deal with the following situation:
> > > > 
> > > > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > > > 
> > > > raw_spin_lock(A)
> > > > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > > > 
> > > > raw_spin_unlock(A)
> > > > 
> > > > Certain operations must interrupt guest vcpu-0 (see trace below).
> > > 
> > > Those traces don't make any sense. All they include is kvm_exit and you
> > > can't tell anything from that.
> > 
> > Hi Peter,
> > 
> > OK lets describe whats happening:
> > 
> > With QEMU emulator thread and vcpu-0 sharing a physical CPU
> > (which is a request from several NFV customers, to improve
> > guest packing), the following occurs when the guest generates 
> > the following pattern:
> > 
> > 		1. submit IO.
> > 		2. busy spin.
> 
> User-space spinning is a bad idea in general and terminally broken in
> a RT setup. Sounds like you need to go fix qemu to not suck.

One can run whatever application they want on the housekeeping
vcpus. This is why rteval exists.

This is not the realtime vcpu we are talking about.

We can fix the BIOS, which is hanging now, but userspace can 
do whatever it wants, on non realtime vcpus (again, this is why
rteval test exists and is used by the -RT community as 
a testcase).

I haven't understood what is the wrong with the patch? Are you trying
to avoid pollution of the spinlock codepath to keep it simple?

[toc] | [prev] | [next] | [standalone]

#1737488 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 09:36:39AM -0300, Marcelo Tosatti wrote:
> On Fri, Sep 22, 2017 at 02:31:07PM +0200, Peter Zijlstra wrote:
> > On Fri, Sep 22, 2017 at 09:16:40AM -0300, Marcelo Tosatti wrote:
> > > On Fri, Sep 22, 2017 at 12:00:05PM +0200, Peter Zijlstra wrote:
> > > > On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > > > > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > > > > to
> > > > > deal with the following situation:
> > > > > 
> > > > > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > > > > 
> > > > > raw_spin_lock(A)
> > > > > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > > > > 
> > > > > raw_spin_unlock(A)
> > > > > 
> > > > > Certain operations must interrupt guest vcpu-0 (see trace below).
> > > > 
> > > > Those traces don't make any sense. All they include is kvm_exit and you
> > > > can't tell anything from that.
> > > 
> > > Hi Peter,
> > > 
> > > OK lets describe whats happening:
> > > 
> > > With QEMU emulator thread and vcpu-0 sharing a physical CPU
> > > (which is a request from several NFV customers, to improve
> > > guest packing), the following occurs when the guest generates 
> > > the following pattern:
> > > 
> > > 		1. submit IO.
> > > 		2. busy spin.
> > 
> > User-space spinning is a bad idea in general and terminally broken in
> > a RT setup. Sounds like you need to go fix qemu to not suck.
> 
> One can run whatever application they want on the housekeeping
> vcpus. This is why rteval exists.

Nobody cares about other tasks. The problem is between the VCPU and
emulator thread. They get a priority inversion and live-lock because of
spin-waiting.

> This is not the realtime vcpu we are talking about.

You're being confused, its a RT _guest_, all VCPUs _must_ be RT.
Because, as you ran into, the guest functions as a whole, not as a bunch
of individual CPUs.

> We can fix the BIOS, which is hanging now, but userspace can 
> do whatever it wants, on non realtime vcpus (again, this is why
> rteval test exists and is used by the -RT community as 
> a testcase).

But nobody cares what other tasks on the system do, all you care about
is that the VCPUs make deterministic forward progress.

> I haven't understood what is the wrong with the patch? Are you trying
> to avoid pollution of the spinlock codepath to keep it simple?

Your patch is voodoo programming. You don't solve the actual problem,
you try and paper over it.

[toc] | [prev] | [next] | [standalone]

#1738712 — Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall

On Fri, Sep 22, 2017 at 02:59:51PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 22, 2017 at 09:36:39AM -0300, Marcelo Tosatti wrote:
> > On Fri, Sep 22, 2017 at 02:31:07PM +0200, Peter Zijlstra wrote:
> > > On Fri, Sep 22, 2017 at 09:16:40AM -0300, Marcelo Tosatti wrote:
> > > > On Fri, Sep 22, 2017 at 12:00:05PM +0200, Peter Zijlstra wrote:
> > > > > On Thu, Sep 21, 2017 at 10:10:41PM -0300, Marcelo Tosatti wrote:
> > > > > > When executing guest vcpu-0 with FIFO:1 priority, which is necessary
> > > > > > to
> > > > > > deal with the following situation:
> > > > > > 
> > > > > > VCPU-0 (housekeeping VCPU)              VCPU-1 (realtime VCPU)
> > > > > > 
> > > > > > raw_spin_lock(A)
> > > > > > interrupted, schedule task T-1          raw_spin_lock(A) (spin)
> > > > > > 
> > > > > > raw_spin_unlock(A)
> > > > > > 
> > > > > > Certain operations must interrupt guest vcpu-0 (see trace below).
> > > > > 
> > > > > Those traces don't make any sense. All they include is kvm_exit and you
> > > > > can't tell anything from that.
> > > > 
> > > > Hi Peter,
> > > > 
> > > > OK lets describe whats happening:
> > > > 
> > > > With QEMU emulator thread and vcpu-0 sharing a physical CPU
> > > > (which is a request from several NFV customers, to improve
> > > > guest packing), the following occurs when the guest generates 
> > > > the following pattern:
> > > > 
> > > > 		1. submit IO.
> > > > 		2. busy spin.
> > > 
> > > User-space spinning is a bad idea in general and terminally broken in
> > > a RT setup. Sounds like you need to go fix qemu to not suck.
> > 
> > One can run whatever application they want on the housekeeping
> > vcpus. This is why rteval exists.
> 
> Nobody cares about other tasks. The problem is between the VCPU and
> emulator thread. They get a priority inversion and live-lock because of
> spin-waiting.
> 
> > This is not the realtime vcpu we are talking about.
> 
> You're being confused, its a RT _guest_, all VCPUs _must_ be RT.
> Because, as you ran into, the guest functions as a whole, not as a bunch
> of individual CPUs.
> 
> > We can fix the BIOS, which is hanging now, but userspace can 
> > do whatever it wants, on non realtime vcpus (again, this is why
> > rteval test exists and is used by the -RT community as 
> > a testcase).
> 
> But nobody cares what other tasks on the system do, all you care about
> is that the VCPUs make deterministic forward progress.
> 
> > I haven't understood what is the wrong with the patch? Are you trying
> > to avoid pollution of the spinlock codepath to keep it simple?
> 
> Your patch is voodoo programming. You don't solve the actual problem,
> you try and paper over it.

Priority boosting on a particular section of code is voodoo programming? 

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web