* [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() @ 2016-10-14 1:48 Wanpeng Li 2016-10-14 12:19 ` [tip:x86/urgent] " tip-bot for Wanpeng Li 2016-10-16 13:39 ` [PATCH] " Peter Zijlstra 0 siblings, 2 replies; 15+ messages in thread From: Wanpeng Li @ 2016-10-14 1:48 UTC (permalink / raw) To: linux-kernel Cc: Wanpeng Li, Ingo Molnar, Mike Galbraith, Peter Zijlstra, Thomas Gleixner =============================== [ INFO: suspicious RCU usage. ] 4.8.0+ #24 Not tainted ------------------------------- ./arch/x86/include/asm/msr-trace.h:47 suspicious rcu_dereference_check() usage! other info that might help us debug this: RCU used illegally from idle CPU! rcu_scheduler_active = 1, debug_locks = 0 RCU used illegally from extended quiescent state! no locks held by swapper/1/0. stack backtrace: CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #24 Hardware name: Dell Inc. OptiPlex 7020/0F5C5X, BIOS A03 01/08/2015 ffff90285de03f58 ffffffff9d44a0c9 ffff90285ca5d100 0000000000000001 ffff90285de03f88 ffffffff9d0ebd67 ffff902845165410 000000000000080b 0000000000000000 0000000000000000 ffff90285de03fb8 ffffffff9d492b95 Call Trace: <IRQ> [<ffffffff9d44a0c9>] dump_stack+0x99/0xd0 [<ffffffff9d0ebd67>] lockdep_rcu_suspicious+0xe7/0x120 [<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 <EOI> [<ffffffff9d732634>] ? cpuidle_enter_state+0xe4/0x360 [<ffffffff9d73261f>] ? cpuidle_enter_state+0xcf/0x360 [<ffffffff9d7328e7>] cpuidle_enter+0x17/0x20 [<ffffffff9d0e1a73>] call_cpuidle+0x23/0x50 [<ffffffff9d0e1d0c>] cpu_startup_entry+0x15c/0x280 [<ffffffff9d05ce64>] start_secondary+0x154/0x180 Reschedule interrupt may be called in cpu idle state. This causes lockdep check warning above. Add irq_enter/exit() in smp_reschedule_interrupt(), irq_enter() tells the RCU subsystems to end the extended quiescent state, so the following trace call in ack_APIC_irq() works correctly. Cc: Ingo Molnar <mingo@kernel.org> Cc: Mike Galbraith <efault@gmx.de> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> --- arch/x86/kernel/smp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c index 658777c..ac2ee87 100644 --- a/arch/x86/kernel/smp.c +++ b/arch/x86/kernel/smp.c @@ -259,8 +259,10 @@ static inline void __smp_reschedule_interrupt(void) __visible void smp_reschedule_interrupt(struct pt_regs *regs) { + irq_enter(); ack_APIC_irq(); __smp_reschedule_interrupt(); + irq_exit(); /* * KVM uses this interrupt to force a cpu out of guest mode */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [tip:x86/urgent] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-14 1:48 [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() Wanpeng Li @ 2016-10-14 12:19 ` tip-bot for Wanpeng Li 2016-10-16 13:39 ` [PATCH] " Peter Zijlstra 1 sibling, 0 replies; 15+ messages in thread From: tip-bot for Wanpeng Li @ 2016-10-14 12:19 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, kernellwp, tglx, peterz, wanpeng.li, efault Commit-ID: 1ec6ec14a2943f6f611fc1d5fb2d4eaa85bd9d72 Gitweb: http://git.kernel.org/tip/1ec6ec14a2943f6f611fc1d5fb2d4eaa85bd9d72 Author: Wanpeng Li <kernellwp@gmail.com> AuthorDate: Fri, 14 Oct 2016 09:48:53 +0800 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Fri, 14 Oct 2016 14:14:20 +0200 x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() =============================== [ INFO: suspicious RCU usage. ] 4.8.0+ #24 Not tainted ------------------------------- ./arch/x86/include/asm/msr-trace.h:47 suspicious rcu_dereference_check() usage! other info that might help us debug this: RCU used illegally from idle CPU! rcu_scheduler_active = 1, debug_locks = 0 RCU used illegally from extended quiescent state! no locks held by swapper/1/0. [<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 Reschedule interrupt may be called in cpu idle state. This causes lockdep check warning above. Add irq_enter/exit() in smp_reschedule_interrupt(), irq_enter() tells the RCU subsystems to end the extended quiescent state, so the following trace call in ack_APIC_irq() works correctly. Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Mike Galbraith <efault@gmx.de> Link: http://lkml.kernel.org/r/1476409733-5133-1-git-send-email-wanpeng.li@hotmail.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/smp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c index 658777c..ac2ee87 100644 --- a/arch/x86/kernel/smp.c +++ b/arch/x86/kernel/smp.c @@ -259,8 +259,10 @@ static inline void __smp_reschedule_interrupt(void) __visible void smp_reschedule_interrupt(struct pt_regs *regs) { + irq_enter(); ack_APIC_irq(); __smp_reschedule_interrupt(); + irq_exit(); /* * KVM uses this interrupt to force a cpu out of guest mode */ ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-14 1:48 [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() Wanpeng Li 2016-10-14 12:19 ` [tip:x86/urgent] " tip-bot for Wanpeng Li @ 2016-10-16 13:39 ` Peter Zijlstra 2016-10-17 4:19 ` Wanpeng Li 1 sibling, 1 reply; 15+ messages in thread From: Peter Zijlstra @ 2016-10-16 13:39 UTC (permalink / raw) To: Wanpeng Li Cc: linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner On Fri, Oct 14, 2016 at 09:48:53AM +0800, Wanpeng Li wrote: > =============================== > [ INFO: suspicious RCU usage. ] > 4.8.0+ #24 Not tainted > ------------------------------- > ./arch/x86/include/asm/msr-trace.h:47 suspicious rcu_dereference_check() usage! > > other info that might help us debug this: > > > RCU used illegally from idle CPU! > rcu_scheduler_active = 1, debug_locks = 0 > RCU used illegally from extended quiescent state! > no locks held by swapper/1/0. > > stack backtrace: > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #24 > Hardware name: Dell Inc. OptiPlex 7020/0F5C5X, BIOS A03 01/08/2015 > ffff90285de03f58 ffffffff9d44a0c9 ffff90285ca5d100 0000000000000001 > ffff90285de03f88 ffffffff9d0ebd67 ffff902845165410 000000000000080b > 0000000000000000 0000000000000000 ffff90285de03fb8 ffffffff9d492b95 > Call Trace: > <IRQ> [<ffffffff9d44a0c9>] dump_stack+0x99/0xd0 > [<ffffffff9d0ebd67>] lockdep_rcu_suspicious+0xe7/0x120 > [<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 > <EOI> [<ffffffff9d732634>] ? cpuidle_enter_state+0xe4/0x360 > [<ffffffff9d73261f>] ? cpuidle_enter_state+0xcf/0x360 > [<ffffffff9d7328e7>] cpuidle_enter+0x17/0x20 > [<ffffffff9d0e1a73>] call_cpuidle+0x23/0x50 > [<ffffffff9d0e1d0c>] cpu_startup_entry+0x15c/0x280 > [<ffffffff9d05ce64>] start_secondary+0x154/0x180 > > Reschedule interrupt may be called in cpu idle state. This causes lockdep > check warning above. > > Add irq_enter/exit() in smp_reschedule_interrupt(), irq_enter() tells the RCU > subsystems to end the extended quiescent state, so the following trace call in > ack_APIC_irq() works correctly. > > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Mike Galbraith <efault@gmx.de> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> > --- > arch/x86/kernel/smp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c > index 658777c..ac2ee87 100644 > --- a/arch/x86/kernel/smp.c > +++ b/arch/x86/kernel/smp.c > @@ -259,8 +259,10 @@ static inline void __smp_reschedule_interrupt(void) > > __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 :/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-16 13:39 ` [PATCH] " Peter Zijlstra @ 2016-10-17 4:19 ` Wanpeng Li 2016-10-17 8:22 ` Peter Zijlstra 0 siblings, 1 reply; 15+ messages in thread From: Wanpeng Li @ 2016-10-17 4:19 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner 2016-10-16 21:39 GMT+08:00 Peter Zijlstra <peterz@infradead.org>: > On Fri, Oct 14, 2016 at 09:48:53AM +0800, Wanpeng Li wrote: >> =============================== >> [ INFO: suspicious RCU usage. ] >> 4.8.0+ #24 Not tainted >> ------------------------------- >> ./arch/x86/include/asm/msr-trace.h:47 suspicious rcu_dereference_check() usage! >> >> other info that might help us debug this: >> >> >> RCU used illegally from idle CPU! >> rcu_scheduler_active = 1, debug_locks = 0 >> RCU used illegally from extended quiescent state! >> no locks held by swapper/1/0. >> >> stack backtrace: >> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #24 >> Hardware name: Dell Inc. OptiPlex 7020/0F5C5X, BIOS A03 01/08/2015 >> ffff90285de03f58 ffffffff9d44a0c9 ffff90285ca5d100 0000000000000001 >> ffff90285de03f88 ffffffff9d0ebd67 ffff902845165410 000000000000080b >> 0000000000000000 0000000000000000 ffff90285de03fb8 ffffffff9d492b95 >> Call Trace: >> <IRQ> [<ffffffff9d44a0c9>] dump_stack+0x99/0xd0 >> [<ffffffff9d0ebd67>] lockdep_rcu_suspicious+0xe7/0x120 >> [<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 >> <EOI> [<ffffffff9d732634>] ? cpuidle_enter_state+0xe4/0x360 >> [<ffffffff9d73261f>] ? cpuidle_enter_state+0xcf/0x360 >> [<ffffffff9d7328e7>] cpuidle_enter+0x17/0x20 >> [<ffffffff9d0e1a73>] call_cpuidle+0x23/0x50 >> [<ffffffff9d0e1d0c>] cpu_startup_entry+0x15c/0x280 >> [<ffffffff9d05ce64>] start_secondary+0x154/0x180 >> >> Reschedule interrupt may be called in cpu idle state. This causes lockdep >> check warning above. >> >> Add irq_enter/exit() in smp_reschedule_interrupt(), irq_enter() tells the RCU >> subsystems to end the extended quiescent state, so the following trace call in >> ack_APIC_irq() works correctly. >> >> Cc: Ingo Molnar <mingo@kernel.org> >> Cc: Mike Galbraith <efault@gmx.de> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> >> --- >> arch/x86/kernel/smp.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c >> index 658777c..ac2ee87 100644 >> --- a/arch/x86/kernel/smp.c >> +++ b/arch/x86/kernel/smp.c >> @@ -259,8 +259,10 @@ static inline void __smp_reschedule_interrupt(void) >> >> __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? :) Regards, Wanpeng Li ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-17 4:19 ` Wanpeng Li @ 2016-10-17 8:22 ` Peter Zijlstra 2016-10-17 9:45 ` Wanpeng Li 2016-10-17 12:19 ` Wanpeng Li 0 siblings, 2 replies; 15+ messages in thread From: Peter Zijlstra @ 2016-10-17 8:22 UTC (permalink / raw) To: Wanpeng Li Cc: linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner On Mon, Oct 17, 2016 at 12:19:43PM +0800, Wanpeng Li wrote: > 2016-10-16 21:39 GMT+08:00 Peter Zijlstra <peterz@infradead.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. 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))) ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-17 8:22 ` Peter Zijlstra @ 2016-10-17 9:45 ` Wanpeng Li 2016-10-17 10:08 ` Paolo Bonzini 2016-10-17 12:19 ` Wanpeng Li 1 sibling, 1 reply; 15+ messages in thread From: Wanpeng Li @ 2016-10-17 9:45 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner, Paolo Bonzini Cc Paolo, 2016-10-17 16:22 GMT+08:00 Peter Zijlstra <peterz@infradead.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@infradead.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 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))) > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-17 9:45 ` Wanpeng Li @ 2016-10-17 10:08 ` Paolo Bonzini 2016-10-17 10:23 ` Wanpeng Li 2016-10-18 0:01 ` Wanpeng Li 0 siblings, 2 replies; 15+ messages in thread From: Paolo Bonzini @ 2016-10-17 10:08 UTC (permalink / raw) To: Wanpeng Li Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner ----- Original Message ----- > From: "Wanpeng Li" <kernellwp@gmail.com> > To: "Peter Zijlstra" <peterz@infradead.org> > Cc: linux-kernel@vger.kernel.org, "Wanpeng Li" <wanpeng.li@hotmail.com>, "Ingo Molnar" <mingo@kernel.org>, "Mike > Galbraith" <efault@gmx.de>, "Thomas Gleixner" <tglx@linutronix.de>, "Paolo Bonzini" <pbonzini@redhat.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@infradead.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@infradead.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))) > > > ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-17 10:08 ` Paolo Bonzini @ 2016-10-17 10:23 ` Wanpeng Li 2016-10-18 0:01 ` Wanpeng Li 1 sibling, 0 replies; 15+ messages in thread From: Wanpeng Li @ 2016-10-17 10:23 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: > > > ----- Original Message ----- >> From: "Wanpeng Li" <kernellwp@gmail.com> >> To: "Peter Zijlstra" <peterz@infradead.org> >> Cc: linux-kernel@vger.kernel.org, "Wanpeng Li" <wanpeng.li@hotmail.com>, "Ingo Molnar" <mingo@kernel.org>, "Mike >> Galbraith" <efault@gmx.de>, "Thomas Gleixner" <tglx@linutronix.de>, "Paolo Bonzini" <pbonzini@redhat.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@infradead.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@infradead.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) I see, thanks Paolo and Peterz. :) Regards, Wanpeng Li > > > 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))) >> > >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-17 10:08 ` Paolo Bonzini 2016-10-17 10:23 ` Wanpeng Li @ 2016-10-18 0:01 ` Wanpeng Li 2016-10-19 14:01 ` Wanpeng Li 1 sibling, 1 reply; 15+ messages in thread From: Wanpeng Li @ 2016-10-18 0:01 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: [...] >> > 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 If CONFIG_X86_X2APIC is undefined and CONFIG_KVM_GUEST is defined, there are undefined reference warning for x2apic_mode and check_x2apic etc since they are extern variables and just define under CONFIG_X86_X2APIC. > /* > * 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); > } If -cpu host,-x2apic we should go through mmio when writes xapic EOI. Regards, Wanpeng Li ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-18 0:01 ` Wanpeng Li @ 2016-10-19 14:01 ` Wanpeng Li 2016-10-19 14:10 ` Paolo Bonzini 0 siblings, 1 reply; 15+ messages in thread From: Wanpeng Li @ 2016-10-19 14:01 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner 2016-10-18 8:01 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>: > 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: > [...] >>> > 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 > > If CONFIG_X86_X2APIC is undefined and CONFIG_KVM_GUEST is defined, > there are undefined reference warning for x2apic_mode and check_x2apic > etc since they are extern variables and just define under > CONFIG_X86_X2APIC. [...] > > If -cpu host,-x2apic we should go through mmio when writes xapic EOI. Any ideas? :) Regards, Wanpeng Li ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-19 14:01 ` Wanpeng Li @ 2016-10-19 14:10 ` Paolo Bonzini 2016-10-19 14:30 ` Wanpeng Li 0 siblings, 1 reply; 15+ messages in thread From: Paolo Bonzini @ 2016-10-19 14:10 UTC (permalink / raw) To: Wanpeng Li Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner On 19/10/2016 16:01, Wanpeng Li wrote: > 2016-10-18 8:01 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>: >> 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: >> [...] >>>>> 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 >> >> If CONFIG_X86_X2APIC is undefined and CONFIG_KVM_GUEST is defined, >> there are undefined reference warning for x2apic_mode and check_x2apic >> etc since they are extern variables and just define under >> CONFIG_X86_X2APIC. > > [...] > >> >> If -cpu host,-x2apic we should go through mmio when writes xapic EOI. > > Any ideas? :) Make apic_set_eoi_write return the old pointer? Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-19 14:10 ` Paolo Bonzini @ 2016-10-19 14:30 ` Wanpeng Li 2016-10-24 14:32 ` Paolo Bonzini 0 siblings, 1 reply; 15+ messages in thread From: Wanpeng Li @ 2016-10-19 14:30 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner, Radim Krcmar Cc Radim, 2016-10-19 22:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: > > > On 19/10/2016 16:01, Wanpeng Li wrote: >> 2016-10-18 8:01 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>: >>> 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: >>> [...] >>>>>> 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 >>> >>> If CONFIG_X86_X2APIC is undefined and CONFIG_KVM_GUEST is defined, >>> there are undefined reference warning for x2apic_mode and check_x2apic >>> etc since they are extern variables and just define under >>> CONFIG_X86_X2APIC. >> This results in undefined reference warning. >> >>> >>> If -cpu host,-x2apic we should go through mmio when writes xapic EOI. >> >> Any ideas? :) > > Make apic_set_eoi_write return the old pointer? Could you elaborate? :) Regards, Wanpeng Li ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-19 14:30 ` Wanpeng Li @ 2016-10-24 14:32 ` Paolo Bonzini 2016-10-25 2:52 ` Wanpeng Li 0 siblings, 1 reply; 15+ messages in thread From: Paolo Bonzini @ 2016-10-24 14:32 UTC (permalink / raw) To: Wanpeng Li Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner, Radim Krcmar On 19/10/2016 16:30, Wanpeng Li wrote: > Cc Radim, > 2016-10-19 22:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: >> >> >> On 19/10/2016 16:01, Wanpeng Li wrote: >>> 2016-10-18 8:01 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>: >>>> 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: >>>> [...] >>>>>>> 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 >>>> >>>> If CONFIG_X86_X2APIC is undefined and CONFIG_KVM_GUEST is defined, >>>> there are undefined reference warning for x2apic_mode and check_x2apic >>>> etc since they are extern variables and just define under >>>> CONFIG_X86_X2APIC. >>> > > This results in undefined reference warning. > >>> >>>> >>>> If -cpu host,-x2apic we should go through mmio when writes xapic EOI. >>> >>> Any ideas? :) >> >> Make apic_set_eoi_write return the old pointer? > > Could you elaborate? :) No, that won't work due to how struct apic works and how apic_set_eoi_write patches it. One possibility then is to add a void (*native_eoi_write)(u32 reg, u32 v); field to struct apic, and make apic_set_eoi_write set (*drv)->native_eoi_write = (*drv)->eoi_write; (*drv)->eoi_write = eoi_write; Then kvm.c can call apic->native_eoi_write(reg, v); Thanks, Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-24 14:32 ` Paolo Bonzini @ 2016-10-25 2:52 ` Wanpeng Li 0 siblings, 0 replies; 15+ messages in thread From: Wanpeng Li @ 2016-10-25 2:52 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Zijlstra, linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner, Radim Krcmar 2016-10-24 22:32 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: > > > On 19/10/2016 16:30, Wanpeng Li wrote: >> Cc Radim, >> 2016-10-19 22:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: >>> >>> >>> On 19/10/2016 16:01, Wanpeng Li wrote: >>>> 2016-10-18 8:01 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>: >>>>> 2016-10-17 18:08 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>: >>>>> [...] >>>>>>>> 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 >>>>> >>>>> If CONFIG_X86_X2APIC is undefined and CONFIG_KVM_GUEST is defined, >>>>> there are undefined reference warning for x2apic_mode and check_x2apic >>>>> etc since they are extern variables and just define under >>>>> CONFIG_X86_X2APIC. >>>> >> >> This results in undefined reference warning. >> >>>> >>>>> >>>>> If -cpu host,-x2apic we should go through mmio when writes xapic EOI. >>>> >>>> Any ideas? :) >>> >>> Make apic_set_eoi_write return the old pointer? >> >> Could you elaborate? :) > > No, that won't work due to how struct apic works and how > apic_set_eoi_write patches it. One possibility then is to add a > > void (*native_eoi_write)(u32 reg, u32 v); > > field to struct apic, and make apic_set_eoi_write set > > (*drv)->native_eoi_write = (*drv)->eoi_write; > (*drv)->eoi_write = eoi_write; > > Then kvm.c can call > > apic->native_eoi_write(reg, v); It works. Thanks Paolo. :) Regards, Wanpeng Li ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() 2016-10-17 8:22 ` Peter Zijlstra 2016-10-17 9:45 ` Wanpeng Li @ 2016-10-17 12:19 ` Wanpeng Li 1 sibling, 0 replies; 15+ messages in thread From: Wanpeng Li @ 2016-10-17 12:19 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, Wanpeng Li, Ingo Molnar, Mike Galbraith, Thomas Gleixner 2016-10-17 16:22 GMT+08:00 Peter Zijlstra <peterz@infradead.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@infradead.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. > > 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); > +} I move this out of !PARAVIRT since PARAVIRT also will use it. Regards, Wanpeng Li > + > #define rdmsrl(msr, val) \ > ((val) = native_read_msr((msr))) > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-10-25 2:52 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-14 1:48 [PATCH] x86/smp: Add irq_enter/exit() in smp_reschedule_interrupt() Wanpeng Li 2016-10-14 12:19 ` [tip:x86/urgent] " tip-bot for Wanpeng Li 2016-10-16 13:39 ` [PATCH] " Peter Zijlstra 2016-10-17 4:19 ` Wanpeng Li 2016-10-17 8:22 ` Peter Zijlstra 2016-10-17 9:45 ` Wanpeng Li 2016-10-17 10:08 ` Paolo Bonzini 2016-10-17 10:23 ` Wanpeng Li 2016-10-18 0:01 ` Wanpeng Li 2016-10-19 14:01 ` Wanpeng Li 2016-10-19 14:10 ` Paolo Bonzini 2016-10-19 14:30 ` Wanpeng Li 2016-10-24 14:32 ` Paolo Bonzini 2016-10-25 2:52 ` Wanpeng Li 2016-10-17 12:19 ` Wanpeng Li
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).