ACK: [PATCH Xenial SRU] xen/qspinlock: Don't kick CPU if IRQ is not initialized

Colin Ian King colin.king at canonical.com
Wed Dec 14 15:43:30 UTC 2016


On 14/12/16 15:25, Tim Gardner wrote:
> On 12/14/2016 08:13 AM, Colin Ian King wrote:
>> On 14/12/16 15:07, Tim Gardner wrote:
>>> From: Ross Lagerwall <ross.lagerwall at citrix.com>
>>>
>>> BugLink: http://bugs.launchpad.net/bugs/1649821
>>>
>>> The following commit:
>>>
>>>   1fb3a8b2cfb2 ("xen/spinlock: Fix locking path engaging too soon under PVHVM.")
>>>
>>> ... moved the initalization of the kicker interrupt until after
>>> native_cpu_up() is called.
>>>
>>> However, when using qspinlocks, a CPU may try to kick another CPU that is
>>> spinning (because it has not yet initialized its kicker interrupt), resulting
>>> in the following crash during boot:
>>>
>>>   kernel BUG at /build/linux-Ay7j_C/linux-4.4.0/drivers/xen/events/events_base.c:1210!
>>>   invalid opcode: 0000 [#1] SMP
>>>   ...
>>>   RIP: 0010:[<ffffffff814c97c9>]  [<ffffffff814c97c9>] xen_send_IPI_one+0x59/0x60
>>>   ...
>>>   Call Trace:
>>>    [<ffffffff8102be9e>] xen_qlock_kick+0xe/0x10
>>>    [<ffffffff810cabc2>] __pv_queued_spin_unlock+0xb2/0xf0
>>>    [<ffffffff810ca6d1>] ? __raw_callee_save___pv_queued_spin_unlock+0x11/0x20
>>>    [<ffffffff81052936>] ? check_tsc_warp+0x76/0x150
>>>    [<ffffffff81052aa6>] check_tsc_sync_source+0x96/0x160
>>>    [<ffffffff81051e28>] native_cpu_up+0x3d8/0x9f0
>>>    [<ffffffff8102b315>] xen_hvm_cpu_up+0x35/0x80
>>>    [<ffffffff8108198c>] _cpu_up+0x13c/0x180
>>>    [<ffffffff81081a4a>] cpu_up+0x7a/0xa0
>>>    [<ffffffff81f80dfc>] smp_init+0x7f/0x81
>>>    [<ffffffff81f5a121>] kernel_init_freeable+0xef/0x212
>>>    [<ffffffff81817f30>] ? rest_init+0x80/0x80
>>>    [<ffffffff81817f3e>] kernel_init+0xe/0xe0
>>>    [<ffffffff8182488f>] ret_from_fork+0x3f/0x70
>>>    [<ffffffff81817f30>] ? rest_init+0x80/0x80
>>>
>>> To fix this, only send the kick if the target CPU's interrupt has been
>>> initialized. This check isn't racy, because the target is waiting for
>>> the spinlock, so it won't have initialized the interrupt in the
>>> meantime.
>>>
>>> Signed-off-by: Ross Lagerwall <ross.lagerwall at citrix.com>
>>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky at oracle.com>
>>> Cc: David Vrabel <david.vrabel at citrix.com>
>>> Cc: Juergen Gross <jgross at suse.com>
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
>>> Cc: Linus Torvalds <torvalds at linux-foundation.org>
>>> Cc: Peter Zijlstra <peterz at infradead.org>
>>> Cc: Thomas Gleixner <tglx at linutronix.de>
>>> Cc: linux-kernel at vger.kernel.org
>>> Cc: xen-devel at lists.xenproject.org
>>> Signed-off-by: Ingo Molnar <mingo at kernel.org>
>>> (cherry picked from commit 707e59ba494372a90d245f18b0c78982caa88e48)
>>> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
>>> ---
>>>  arch/x86/xen/spinlock.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
>>> index 9e2ba5c..f42e78d 100644
>>> --- a/arch/x86/xen/spinlock.c
>>> +++ b/arch/x86/xen/spinlock.c
>>> @@ -27,6 +27,12 @@ static bool xen_pvspin = true;
>>>  
>>>  static void xen_qlock_kick(int cpu)
>>>  {
>>> +	int irq = per_cpu(lock_kicker_irq, cpu);
>>> +
>>> +	/* Don't kick if the target's kicker interrupt is not initialized. */
>>> +	if (irq == -1)
>>> +		return;
>>> +
>>>  	xen_send_IPI_one(cpu, XEN_SPIN_UNLOCK_VECTOR);
>>>  }
>>>  
>>>
>> Seems OK to me, however has this been tested for this specific bug?
>>
>> Colin
>>
> 
> It seems like an obvious fix. Both call traces look identical. I can
> certainly produce a test kernel and get the reporter to produce some
> results.
> 
> rtg
> 
>
Thanks Tim, that's good with me.

Acked-by: Colin Ian King <colin.king at canonical.com>





More information about the kernel-team mailing list