From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933921AbcJQKJ6 (ORCPT ); Mon, 17 Oct 2016 06:09:58 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:49888 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757733AbcJQKJv (ORCPT ); Mon, 17 Oct 2016 06:09:51 -0400 Date: Mon, 17 Oct 2016 06:08:42 -0400 (EDT) From: Paolo Bonzini To: Wanpeng Li Cc: Peter Zijlstra , linux-kernel@vger.kernel.org, Wanpeng Li , Ingo Molnar , Mike Galbraith , Thomas Gleixner Message-ID: <938757058.3991379.1476698922290.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1476409733-5133-1-git-send-email-wanpeng.li@hotmail.com> <20161016133911.GI3142@twins.programming.kicks-ass.net> <20161017082250.GX3568@worktop.programming.kicks-ass.net> Subject: Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.4.164.1, 10.5.100.50] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF49 (Linux)/8.0.6_GA_5922) Thread-Topic: x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() Thread-Index: zQw4AjvnqYzMgPG0jxUi6xW8i7cgFA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- Original Message ----- > From: "Wanpeng Li" > To: "Peter Zijlstra" > Cc: linux-kernel@vger.kernel.org, "Wanpeng Li" , "Ingo Molnar" , "Mike > Galbraith" , "Thomas Gleixner" , "Paolo Bonzini" > 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 : > > On Mon, Oct 17, 2016 at 12:19:43PM +0800, Wanpeng Li wrote: > >> 2016-10-16 21:39 GMT+08:00 Peter Zijlstra : > > > >> >> [] do_trace_write_msr+0x135/0x140 > >> >> [] native_write_msr+0x20/0x30 > >> >> [] native_apic_msr_eoi_write+0x1d/0x30 > >> >> [] smp_reschedule_interrupt+0x1d/0x30 > >> >> [] 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))) > > >