Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using hard_irq_disable()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux