search for: ticket_shift

From: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> Hi all, This series does two major things: 1. It converts the bulk of the implementation to C, and makes the "small ticket" and "large ticket" code common. Only the actual size-dependent asm instructions are specific to the ticket size. The resulting generated asm is very similar to the current

From: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> Hi all, This series does two major things: 1. It converts the bulk of the implementation to C, and makes the "small ticket" and "large ticket" code common. Only the actual size-dependent asm instructions are specific to the ticket size. The resulting generated asm is very similar to the current

From: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> Hi all, This series does two major things: 1. It converts the bulk of the implementation to C, and makes the "small ticket" and "large ticket" code common. Only the actual size-dependent asm instructions are specific to the ticket size. The resulting generated asm is very similar to the current

From: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> Hi all, This is a revised version of the pvticket lock series. The early part of the series is mostly unchanged: it converts the bulk of the ticket lock code into C and makes the "small" and "large" ticket code common. The only changes are the incorporation of various review comments. The latter part of

From: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> Hi all, This is a revised version of the pvticket lock series. The early part of the series is mostly unchanged: it converts the bulk of the ticket lock code into C and makes the "small" and "large" ticket code common. The only changes are the incorporation of various review comments. The latter part of

From: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> Hi all, This is a revised version of the pvticket lock series. The early part of the series is mostly unchanged: it converts the bulk of the ticket lock code into C and makes the "small" and "large" ticket code common. The only changes are the incorporation of various review comments. The latter part of

...ckup problem that Sasha reported) together with changing that 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 ta...

...ckup problem that Sasha reported) together with changing that 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 ta...

...e: > >> 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...

...e: > >> 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...

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

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

...__always_inline int arch_spin_trylock(arch_spinlock_t *lock) @@ -109,10 +153,12 @@ static __always_inline int arch_spin_trylock(arch_spinlock_t *lock) if (old.tickets.head != (old.tickets.tail & ~TICKET_SLOWPATH_FLAG)) return 0; - new.head_tail = old.head_tail + (TICKET_LOCK_INC << TICKET_SHIFT); + new.head_tail = old.head_tail + (TICKET_LOCK_TAIL_INC << TICKET_SHIFT); + new.head_tail |= TICKET_LOCK_LOCK_INC; /* cmpxchg is a full barrier, so nothing can move before it */ - return cmpxchg(&lock->head_tail, old.head_tail, new.head_tail) == old.head_tail; + return cmpxchg(&a...

...__always_inline int arch_spin_trylock(arch_spinlock_t *lock) @@ -109,10 +153,12 @@ static __always_inline int arch_spin_trylock(arch_spinlock_t *lock) if (old.tickets.head != (old.tickets.tail & ~TICKET_SLOWPATH_FLAG)) return 0; - new.head_tail = old.head_tail + (TICKET_LOCK_INC << TICKET_SHIFT); + new.head_tail = old.head_tail + (TICKET_LOCK_TAIL_INC << TICKET_SHIFT); + new.head_tail |= TICKET_LOCK_LOCK_INC; /* cmpxchg is a full barrier, so nothing can move before it */ - return cmpxchg(&lock->head_tail, old.head_tail, new.head_tail) == old.head_tail; + return cmpxchg(&a...

On 02/10, Jeremy Fitzhardinge wrote: > > On 02/10/2015 05:26 AM, Oleg Nesterov wrote: > > On 02/10, Raghavendra K T wrote: > >> 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

On 02/10, Jeremy Fitzhardinge wrote: > > On 02/10/2015 05:26 AM, Oleg Nesterov wrote: > > On 02/10, Raghavendra K T wrote: > >> 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

...; /* make sure nothing creeps before the lock is taken */ } @@ -109,7 +122,8 @@ static __always_inline int arch_spin_trylock(arch_spinlock_t *lock) if (old.tickets.head != (old.tickets.tail & ~TICKET_SLOWPATH_FLAG)) return 0; - new.head_tail = old.head_tail + (TICKET_LOCK_INC << TICKET_SHIFT); + new.head_tail = old.head_tail + (TICKET_LOCK_TAIL_INC << TICKET_SHIFT); + new.head_tail |= TICKET_LOCK_HEAD_INC; /* cmpxchg is a full barrier, so nothing can move before it */ return cmpxchg(&lock->head_tail, old.head_tail, new.head_tail) == old.head_tail; @@ -123,7 +137,7 @@...

...; /* make sure nothing creeps before the lock is taken */ } @@ -109,7 +122,8 @@ static __always_inline int arch_spin_trylock(arch_spinlock_t *lock) if (old.tickets.head != (old.tickets.tail & ~TICKET_SLOWPATH_FLAG)) return 0; - new.head_tail = old.head_tail + (TICKET_LOCK_INC << TICKET_SHIFT); + new.head_tail = old.head_tail + (TICKET_LOCK_TAIL_INC << TICKET_SHIFT); + new.head_tail |= TICKET_LOCK_HEAD_INC; /* cmpxchg is a full barrier, so nothing can move before it */ return cmpxchg(&lock->head_tail, old.head_tail, new.head_tail) == old.head_tail; @@ -123,7 +137,7 @@...

...; copy */ >> - old.tickets.head += TICKET_LOCK_INC; > > NB (see below) Thanks for pointing, this solved the hang issue. I missed this exact addition. > >> - >> - /* Clear the slowpath flag */ >> - new.head_tail = old.head_tail & ~(TICKET_SLOWPATH_FLAG << TICKET_SHIFT); >> - >> - /* >> - * If the lock is uncontended, clear the flag - use cmpxchg in >> - * case it changes behind our back though. >> - */ >> - if (new.tickets.head != new.tickets.tail || >> - cmpxchg(&lock->head_tail, old.head_tail, >> -...

...; copy */ >> - old.tickets.head += TICKET_LOCK_INC; > > NB (see below) Thanks for pointing, this solved the hang issue. I missed this exact addition. > >> - >> - /* Clear the slowpath flag */ >> - new.head_tail = old.head_tail & ~(TICKET_SLOWPATH_FLAG << TICKET_SHIFT); >> - >> - /* >> - * If the lock is uncontended, clear the flag - use cmpxchg in >> - * case it changes behind our back though. >> - */ >> - if (new.tickets.head != new.tickets.tail || >> - cmpxchg(&lock->head_tail, old.head_tail, >> -...