[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] x86 spinlock: Fix memory corruption on completing completions
- To: Oleg Nesterov <oleg@xxxxxxxxxx>
- From: Raghavendra K T <raghavendra.kt@xxxxxxxxxxxxxxxxxx>
- Date: Wed, 11 Feb 2015 16:38:58 +0530
- Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, KVM list <kvm@xxxxxxxxxxxxxxx>, Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Jason Wang <jasowang@xxxxxxxxxx>, virtualization <virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx>, Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>, Peter Anvin <hpa@xxxxxxxxx>, Davidlohr Bueso <dave@xxxxxxxxxxxx>, Andrey Ryabinin <a.ryabinin@xxxxxxxxxxx>, the arch/x86 maintainers <x86@xxxxxxxxxx>, Christian Borntraeger <borntraeger@xxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>, Sasha Levin <sasha.levin@xxxxxxxxxx>, Paul McKenney <paulmck@xxxxxxxxxxxxxxxxxx>, Andi Kleen <ak@xxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Dave Jones <davej@xxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Waiman Long <waiman.long@xxxxxx>, Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>, Paolo Bonzini <pbonzini@xxxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Wed, 11 Feb 2015 11:08:16 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On 02/10/2015 06:56 PM, Oleg Nesterov wrote:
On 02/10, Raghavendra K T wrote:
On 02/10/2015 06:23 AM, Linus Torvalds wrote:
add_smp(&lock->tickets.head, TICKET_LOCK_INC);
if (READ_ONCE(lock->tickets.tail) & TICKET_SLOWPATH_FLAG) ..
into something like
val = xadd((&lock->ticket.head_tail, TICKET_LOCK_INC << TICKET_SHIFT);
if (unlikely(val & TICKET_SLOWPATH_FLAG)) ...
would be the right thing to do. Somebody should just check that I got
that shift right, and that the tail is in the high bytes (head really
needs to be high to work, if it's in the low byte(s) the xadd would
overflow from head into tail which would be wrong).
Unfortunately xadd could result in head overflow as tail is high.
The other option was repeated cmpxchg which is bad I believe.
Any suggestions?
Stupid question... what if we simply move SLOWPATH from .tail to .head?
In this case arch_spin_unlock() could do xadd(tickets.head) and check
the result
It is a good idea. Trying this now.
In this case __ticket_check_and_clear_slowpath() really needs to cmpxchg
the whole .head_tail. Plus obviously more boring changes. This needs a
separate patch even _if_ this can work.
Correct, but apart from this, before doing xadd in unlock,
we would have to make sure lsb bit is cleared so that we can live with 1
bit overflow to tail which is unused. now either or both of head,tail
lsb bit may be set after unlock.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
