[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <938757058.3991379.1476698922290.JavaMail.zimbra@redhat.com>
Date: Mon, 17 Oct 2016 06:08:42 -0400 (EDT)
From: Paolo Bonzini <pbonzini@...hat.com>
To: Wanpeng Li <kernellwp@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
linux-kernel@...r.kernel.org, Wanpeng Li <wanpeng.li@...mail.com>,
Ingo Molnar <mingo@...nel.org>, Mike Galbraith <efault@....de>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] x86/smp: Add irq_enter/exit() in
smp_reschedule_interrupt()
----- Original Message -----
> From: "Wanpeng Li" <kernellwp@...il.com>
> To: "Peter Zijlstra" <peterz@...radead.org>
> Cc: linux-kernel@...r.kernel.org, "Wanpeng Li" <wanpeng.li@...mail.com>, "Ingo Molnar" <mingo@...nel.org>, "Mike
> Galbraith" <efault@....de>, "Thomas Gleixner" <tglx@...utronix.de>, "Paolo Bonzini" <pbonzini@...hat.com>
> Sent: Monday, October 17, 2016 11:45:32 AM
> Subject: Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt()
>
> Cc Paolo,
> 2016-10-17 16:22 GMT+08:00 Peter Zijlstra <peterz@...radead.org>:
> > On Mon, Oct 17, 2016 at 12:19:43PM +0800, Wanpeng Li wrote:
> >> 2016-10-16 21:39 GMT+08:00 Peter Zijlstra <peterz@...radead.org>:
> >
> >> >> [<ffffffff9d492b95>] do_trace_write_msr+0x135/0x140
> >> >> [<ffffffff9d06f860>] native_write_msr+0x20/0x30
> >> >> [<ffffffff9d065fad>] native_apic_msr_eoi_write+0x1d/0x30
> >> >> [<ffffffff9d05bd1d>] smp_reschedule_interrupt+0x1d/0x30
> >> >> [<ffffffff9d8daec6>] reschedule_interrupt+0x96/0xa0
> >
> >> >> __visible void smp_reschedule_interrupt(struct pt_regs *regs)
> >> >> {
> >> >> + irq_enter();
> >> >> ack_APIC_irq();
> >> >> __smp_reschedule_interrupt();
> >> >> + irq_exit();
> >> >
> >> > Urgh, I really hate this...
> >> >
> >> > So now we're making a very frequent interrupt slower because of debug
> >> > code :/
> >>
> >> Do you have a better idea? :)
> >
> > Something like the below avoids all that. Paravirt will still need fixing.
>
> kvm_guest_apic_eoi_write
> -> native_apic_msr_write
kvm_guest_apic_eoi_write can use native_apic_msr_eoi_write too:
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index f5aaf6c83222..9769d76a62c4 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -174,7 +174,7 @@ static inline void disable_local_APIC(void) { }
static inline void lapic_update_tsc_freq(void) { }
#endif /* !CONFIG_X86_LOCAL_APIC */
-#ifdef CONFIG_X86_X2APIC
+#if defined CONFIG_X86_X2APIC || defined CONFIG_KVM_GUEST
/*
* Make previous memory operations globally visible before
* sending the IPI through x2apic wrmsr. We need a serializing instruction or
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index edbbfc854e39..61cc6a5e3f44 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -319,7 +319,7 @@ static void kvm_guest_apic_eoi_write(u32 reg, u32 val)
*/
if (__test_and_clear_bit(KVM_PV_EOI_BIT, this_cpu_ptr(&kvm_apic_eoi)))
return;
- apic_write(APIC_EOI, APIC_EOI_ACK);
+ native_apic_msr_eoi_write(APIC_EOI, APIC_EOI_ACK);
}
static void kvm_guest_cpu_init(void)
Thanks,
Paolo
> I think you can replace the wrmsr in native_apic_msr_write() by your
> wrmsr_notrace().
>
> Regards,
> Wanpeng Li
>
> >
> > The thing is, many many smp_reschedule_interrupt() invocations don't
> > actually execute anything much at all and are only send to tickle the
> > return to user path (which does the actual preemption).
> >
> > Having to do the whole irq_enter/irq_exit dance just for this unlikely
> > debug case totally blows.
> >
> > ---
> > arch/x86/include/asm/apic.h | 2 +-
> > arch/x86/include/asm/msr.h | 15 +++++++++++++++
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> > index f5aaf6c83222..b97bfeed6456 100644
> > --- a/arch/x86/include/asm/apic.h
> > +++ b/arch/x86/include/asm/apic.h
> > @@ -196,7 +196,7 @@ static inline void native_apic_msr_write(u32 reg, u32
> > v)
> >
> > static inline void native_apic_msr_eoi_write(u32 reg, u32 v)
> > {
> > - wrmsr(APIC_BASE_MSR + (APIC_EOI >> 4), APIC_EOI_ACK, 0);
> > + wrmsr_notrace(APIC_BASE_MSR + (APIC_EOI >> 4), APIC_EOI_ACK, 0);
> > }
> >
> > static inline u32 native_apic_msr_read(u32 reg)
> > diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
> > index b5fee97813cd..45c080449d5b 100644
> > --- a/arch/x86/include/asm/msr.h
> > +++ b/arch/x86/include/asm/msr.h
> > @@ -127,6 +127,16 @@ notrace static inline void native_write_msr(unsigned
> > int msr,
> > }
> >
> > /* Can be uninlined because referenced by paravirt */
> > +notrace static inline void native_write_msr_notrace(unsigned int msr,
> > + unsigned low, unsigned high)
> > +{
> > + asm volatile("1: wrmsr\n"
> > + "2:\n"
> > + _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > + : : "c" (msr), "a"(low), "d" (high) : "memory");
> > +}
> > +
> > +/* Can be uninlined because referenced by paravirt */
> > notrace static inline int native_write_msr_safe(unsigned int msr,
> > unsigned low, unsigned high)
> > {
> > @@ -228,6 +238,11 @@ static inline void wrmsr(unsigned msr, unsigned low,
> > unsigned high)
> > native_write_msr(msr, low, high);
> > }
> >
> > +static inline void wrmsr_notrace(unsigned msr, unsigned low, unsigned
> > high)
> > +{
> > + native_write_msr_notrace(msr, low, high);
> > +}
> > +
> > #define rdmsrl(msr, val) \
> > ((val) = native_read_msr((msr)))
> >
>
Powered by blists - more mailing lists