search for: xadd

Hi, I'm trying to add the xadd instruction to the X86 back end. xadd r/m32, r32 exchanges r/m32 and r32, and loads the sum into r/m32. I'm interested in the case where the destination operand is a memory location. I've added the following entry to X86InstrInfo.td: def XADD32mr : I<0x87, MRMDestMem,...

On Thu, 2 Dec 2004, Brent Monroe wrote: > I'm trying to add the xadd instruction to the X86 back end. > xadd r/m32, r32 > exchanges r/m32 and r32, and loads the sum into r/m32. I'm > interested in the case where the destination operand is a > memory location. > > I've added the following entry to X86InstrInfo.td: > def XADD32mr : I<...

On Mon, Feb 9, 2015 at 4:02 AM, Peter Zijlstra <peterz at infradead.org> wrote: > On Mon, Feb 09, 2015 at 03:04:22PM +0530, Raghavendra K T wrote: >> So we have 3 choices, >> 1. xadd >> 2. continue with current approach. >> 3. a read before unlock and also after that. > > For the truly paranoid we have probe_kernel_address(), suppose the lock > was in module space and the module just got unloaded under us. That's much too expensive. The xadd shouldn...

On Mon, Feb 9, 2015 at 4:02 AM, Peter Zijlstra <peterz at infradead.org> wrote: > On Mon, Feb 09, 2015 at 03:04:22PM +0530, Raghavendra K T wrote: >> So we have 3 choices, >> 1. xadd >> 2. continue with current approach. >> 3. a read before unlock and also after that. > > For the truly paranoid we have probe_kernel_address(), suppose the lock > was in module space and the module just got unloaded under us. That's much too expensive. The xadd shouldn...

Chris Lattner wrote: > On Thu, 2 Dec 2004, Brent Monroe wrote: > >>I'm trying to add the xadd instruction to the X86 back end. >>xadd r/m32, r32 >>exchanges r/m32 and r32, and loads the sum into r/m32. I'm >>interested in the case where the destination operand is a >>memory location. >> >>I've added the following entry to X86InstrInfo.td: >&gt...

...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 (hea...

...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 (hea...

...OCKS */ >> >> static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >> @@ -84,7 +105,7 @@ static __always_inline void arch_spin_lock(arch_spinlock_t *lock) >> register struct __raw_tickets inc = { .tail = TICKET_LOCK_INC }; >> >> inc = xadd(&lock->tickets, inc); >> - if (likely(inc.head == inc.tail)) >> + if (likely(inc.head == (inc.tail & ~TICKET_SLOWPATH_FLAG))) > good point, we can get rid of this as well. > The intent of this conditional was to be the quickest possible path when > taking a fastpat...

...OCKS */ >> >> static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) >> @@ -84,7 +105,7 @@ static __always_inline void arch_spin_lock(arch_spinlock_t *lock) >> register struct __raw_tickets inc = { .tail = TICKET_LOCK_INC }; >> >> inc = xadd(&lock->tickets, inc); >> - if (likely(inc.head == inc.tail)) >> + if (likely(inc.head == (inc.tail & ~TICKET_SLOWPATH_FLAG))) > good point, we can get rid of this as well. > The intent of this conditional was to be the quickest possible path when > taking a fastpat...

On 02/10/2015 06:23 AM, Linus Torvalds wrote: > On Mon, Feb 9, 2015 at 4:02 AM, Peter Zijlstra <peterz at infradead.org> wrote: >> On Mon, Feb 09, 2015 at 03:04:22PM +0530, Raghavendra K T wrote: >>> So we have 3 choices, >>> 1. xadd >>> 2. continue with current approach. >>> 3. a read before unlock and also after that. >> >> For the truly paranoid we have probe_kernel_address(), suppose the lock >> was in module space and the module just got unloaded under us. > > That's much too e...

On 02/10/2015 06:23 AM, Linus Torvalds wrote: > On Mon, Feb 9, 2015 at 4:02 AM, Peter Zijlstra <peterz at infradead.org> wrote: >> On Mon, Feb 09, 2015 at 03:04:22PM +0530, Raghavendra K T wrote: >>> So we have 3 choices, >>> 1. xadd >>> 2. continue with current approach. >>> 3. a read before unlock and also after that. >> >> For the truly paranoid we have probe_kernel_address(), suppose the lock >> was in module space and the module just got unloaded under us. > > That's much too e...

....ibm.com> 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 b...

....ibm.com> 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 b...

...- this removes the write after unlock. note that keeping slowpath flag would > > result in unnecessary kicks. > > By moving the slowpath flag from the tail to the head ticket we also avoid > > the need to access both the head and tail tickets on unlock. > > > > 2. use xadd to avoid read/write after unlock that checks the need for > > unlock_kick (Linus): > > We further avoid the need for a read-after-release by using xadd; > > the prev head value will include the slowpath flag and indicate if we > > need to do PV kicking of suspended spinners...

...- this removes the write after unlock. note that keeping slowpath flag would > > result in unnecessary kicks. > > By moving the slowpath flag from the tail to the head ticket we also avoid > > the need to access both the head and tail tickets on unlock. > > > > 2. use xadd to avoid read/write after unlock that checks the need for > > unlock_kick (Linus): > > We further avoid the need for a read-after-release by using xadd; > > the prev head value will include the slowpath flag and indicate if we > > need to do PV kicking of suspended spinners...

...ted() call is pretty costly compared with other checks within mutex_spin_on_owner() and rwsem_spin_on_owner(), they are done at a reduce frequency of once every 256 iterations. Signed-off-by: Waiman Long <longman at redhat.com> --- kernel/locking/mutex.c | 5 ++++- kernel/locking/rwsem-xadd.c | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c index ad2d9e2..2ece0c4 100644 --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -423,6 +423,7 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *o...

...15 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 th...

...15 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 th...

...lock, and clear it again on the first (try)lock. -- this removes the write after unlock. note that keeping slowpath flag would result in unnecessary kicks. By moving the slowpath flag from the tail to the head ticket we also avoid the need to access both the head and tail tickets on unlock. 2. use xadd to avoid read/write after unlock that checks the need for unlock_kick (Linus): We further avoid the need for a read-after-release by using xadd; the prev head value will include the slowpath flag and indicate if we need to do PV kicking of suspended spinners -- on modern chips xadd isn't (much)...

...lock, and clear it again on the first (try)lock. -- this removes the write after unlock. note that keeping slowpath flag would result in unnecessary kicks. By moving the slowpath flag from the tail to the head ticket we also avoid the need to access both the head and tail tickets on unlock. 2. use xadd to avoid read/write after unlock that checks the need for unlock_kick (Linus): We further avoid the need for a read-after-release by using xadd; the prev head value will include the slowpath flag and indicate if we need to do PV kicking of suspended spinners -- on modern chips xadd isn't (much)...