On 11.07.2013, at 04:48, tiejun.chen wrote: > On 07/10/2013 05:49 PM, Alexander Graf wrote: >> >> On 10.07.2013, at 08:02, Tiejun Chen wrote: >> >>> We should ensure the preemption cannot occur while calling get_paca() >>> insdide hard_irq_disable(), otherwise the paca_struct may be the >>> wrong one just after. And btw, we may update timing stats in this case. >>> >>> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxxxxxx> >>> --- >>> arch/powerpc/kvm/booke.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >>> index dcc94f0..9dae25d 100644 >>> --- a/arch/powerpc/kvm/booke.c >>> +++ b/arch/powerpc/kvm/booke.c >>> @@ -839,6 +839,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, >>> WARN_ON(local_paca->irq_happened != 0); >>> #endif >>> >>> + preempt_disable(); >>> /* >>> * We enter with interrupts disabled in hardware, but >>> * we need to call hard_irq_disable anyway to ensure that >>> @@ -848,6 +849,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, >>> >>> /* update before a new last_exit_type is rewritten */ >>> kvmppc_update_timing_stats(vcpu); >>> + preempt_enable(); >> >> All of the code here is already called with interrupts disabled. I don't see how we could preempt then? > > But the kernel still check this in preempt case, > > #define get_paca() ((void) debug_smp_processor_id(), local_paca) > > then trigger that known call trace as I observed :) > > BUG: using smp_processor_id() in preemptible [00000000] code: qemu-system-ppc/2065 # define preemptible() (preempt_count() == 0 && !irqs_disabled()) So we are only hitting this BUG() when either preempt_count is 0 (which your patch is trying to fix) and at the same time interrupts are enabled. But wait - interrupts are disabled, aren't they? Let's check. #define irqs_disabled() \ ({ \ unsigned long _flags; \ raw_local_save_flags(_flags); \ raw_irqs_disabled_flags(_flags); \ }) #define raw_irqs_disabled_flags(flags) \ ({ \ typecheck(unsigned long, flags); \ arch_irqs_disabled_flags(flags); \ }) static inline unsigned long arch_local_save_flags(void) { unsigned long flags; asm volatile( "lbz %0,%1(13)" : "=r" (flags) : "i" (offsetof(struct paca_struct, soft_enabled))); return flags; } static inline bool arch_irqs_disabled_flags(unsigned long flags) { return flags == 0; } So we're running with soft_enabled == 0 here which means that irqs_disabled() also returns 0. Ben, is soft_enabled == 0; hard_enabled == 1 a valid combination that may ever occur? Alex > caller is .kvmppc_handle_exit+0x48/0x810 > CPU: 0 PID: 2065 Comm: qemu-system-ppc Not tainted 3.10.0-172036-ge2daa28-dirty #116 > Call Trace: > [c0000001fc637570] [c00000000000835c] .show_stack+0x7c/0x1f0 (unreliable) > [c0000001fc637640] [c000000000673a0c] .dump_stack+0x28/0x3c > [c0000001fc6376b0] [c0000000002f04d8] .debug_smp_processor_id+0x108/0x120 > [c0000001fc637740] [c0000000000444e8] .kvmppc_handle_exit+0x48/0x810 > [c0000001fc6377f0] [c000000000047c80] .kvmppc_resume_host+0xa4/0xf8 > > Tiejun > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html