search for: trylock_pend

...val) > { > struct __qspinlock *l = (void *)lock; > > - ACCESS_ONCE(l->locked_pending) = 1; > + ACCESS_ONCE(l->locked_pending) = _Q_LOCKED_VAL; > } > > /* > @@ -567,16 +563,16 @@ static __always_inline int get_qlock(struct qspinlock *lock) > /** > * trylock_pending - try to acquire queue spinlock using the pending bit > * @lock : Pointer to queue spinlock structure > - * @pval : Pointer to value of the queue spinlock 32-bit word > + * @val : Current value of the queue spinlock 32-bit word > * Return: 1 if lock acquired, 0 otherwise >...

On Wed, May 07, 2014 at 11:01:31AM -0400, Waiman Long wrote: > +/** > + * trylock_pending - try to acquire queue spinlock using the pending bit > + * @lock : Pointer to queue spinlock structure > + * @pval : Pointer to value of the queue spinlock 32-bit word > + * Return: 1 if lock acquired, 0 otherwise > + */ > +static inline int trylock_pending(struct qspinlock *lock...

On Wed, May 07, 2014 at 11:01:31AM -0400, Waiman Long wrote: > +/** > + * trylock_pending - try to acquire queue spinlock using the pending bit > + * @lock : Pointer to queue spinlock structure > + * @pval : Pointer to value of the queue spinlock 32-bit word > + * Return: 1 if lock acquired, 0 otherwise > + */ > +static inline int trylock_pending(struct qspinlock *lock...

On 04/18/2014 05:40 PM, Waiman Long wrote: > On 04/18/2014 03:05 PM, Peter Zijlstra wrote: >> On Fri, Apr 18, 2014 at 01:52:50PM -0400, Waiman Long wrote: >>> I am confused by your notation. >> Nah, I think I was confused :-) Make the 1 _Q_LOCKED_VAL though, as >> that's the proper constant to use. > > Everyone gets confused once in a while:-) I have plenty

On 04/18/2014 05:40 PM, Waiman Long wrote: > On 04/18/2014 03:05 PM, Peter Zijlstra wrote: >> On Fri, Apr 18, 2014 at 01:52:50PM -0400, Waiman Long wrote: >>> I am confused by your notation. >> Nah, I think I was confused :-) Make the 1 _Q_LOCKED_VAL though, as >> that's the proper constant to use. > > Everyone gets confused once in a while:-) I have plenty

On Thu, Apr 17, 2014 at 11:03:58AM -0400, Waiman Long wrote: > There is a problem in the current trylock_pending() function. When the > lock is free, but the pending bit holder hasn't grabbed the lock & > cleared the pending bit yet, the trylock_pending() function will fail. I remember seeing some of this.. > It can be seen that the queue spinlock is slower than the ticket > spinlock...

On Thu, Apr 17, 2014 at 11:03:58AM -0400, Waiman Long wrote: > There is a problem in the current trylock_pending() function. When the > lock is free, but the pending bit holder hasn't grabbed the lock & > cleared the pending bit yet, the trylock_pending() function will fail. I remember seeing some of this.. > It can be seen that the queue spinlock is slower than the ticket > spinlock...

There is a problem in the current trylock_pending() function. When the lock is free, but the pending bit holder hasn't grabbed the lock & cleared the pending bit yet, the trylock_pending() function will fail. As a result, the regular queuing code path will be used most of the time even when there is only 2 tasks contending for the lock...

There is a problem in the current trylock_pending() function. When the lock is free, but the pending bit holder hasn't grabbed the lock & cleared the pending bit yet, the trylock_pending() function will fail. As a result, the regular queuing code path will be used most of the time even when there is only 2 tasks contending for the lock...

On Thu, Apr 17, 2014 at 11:03:55AM -0400, Waiman Long wrote: > +/** > + * trylock_pending - try to acquire queue spinlock using the pending bit > + * @lock : Pointer to queue spinlock structure > + * @pval : Pointer to value of the queue spinlock 32-bit word > + * Return: 1 if lock acquired, 0 otherwise > + */ > +static inline int trylock_pending(struct qspinlock *lock...

On Thu, Apr 17, 2014 at 11:03:55AM -0400, Waiman Long wrote: > +/** > + * trylock_pending - try to acquire queue spinlock using the pending bit > + * @lock : Pointer to queue spinlock structure > + * @pval : Pointer to value of the queue spinlock 32-bit word > + * Return: 1 if lock acquired, 0 otherwise > + */ > +static inline int trylock_pending(struct qspinlock *lock...

On 04/17/2014 12:36 PM, Peter Zijlstra wrote: > On Thu, Apr 17, 2014 at 11:03:58AM -0400, Waiman Long wrote: >> There is a problem in the current trylock_pending() function. When the >> lock is free, but the pending bit holder hasn't grabbed the lock& >> cleared the pending bit yet, the trylock_pending() function will fail. > I remember seeing some of this.. > >> It can be seen that the queue spinlock is slower than the t...

...x b97a1ad..6467bfc 100644 --- a/kernel/locking/qspinlock.c +++ b/kernel/locking/qspinlock.c @@ -83,23 +83,97 @@ static inline struct mcs_spinlock *decode_tail(u32 tail) return per_cpu_ptr(&mcs_nodes[idx], cpu); } +#define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK) + +/** + * trylock_pending - try to acquire queue spinlock using the pending bit + * @lock : Pointer to queue spinlock structure + * @pval : Pointer to value of the queue spinlock 32-bit word + * Return: 1 if lock acquired, 0 otherwise + */ +static inline int trylock_pending(struct qspinlock *lock, u32 *pval) +{ + u32 old...

...14 at 11:03:57AM -0400, Waiman Long wrote: > +static __always_inline void > +clear_pending_set_locked(struct qspinlock *lock, u32 val) > +{ > + struct __qspinlock *l = (void *)lock; > + > + ACCESS_ONCE(l->locked_pending) = 1; > +} > @@ -157,8 +251,13 @@ static inline int trylock_pending(struct qspinlock *lock, u32 *pval) > * we're pending, wait for the owner to go away. > * > * *,1,1 -> *,1,0 > + * > + * this wait loop must be a load-acquire such that we match the > + * store-release that clears the locked bit and create lock > + * seque...

...14 at 11:03:57AM -0400, Waiman Long wrote: > +static __always_inline void > +clear_pending_set_locked(struct qspinlock *lock, u32 val) > +{ > + struct __qspinlock *l = (void *)lock; > + > + ACCESS_ONCE(l->locked_pending) = 1; > +} > @@ -157,8 +251,13 @@ static inline int trylock_pending(struct qspinlock *lock, u32 *pval) > * we're pending, wait for the owner to go away. > * > * *,1,1 -> *,1,0 > + * > + * this wait loop must be a load-acquire such that we match the > + * store-release that clears the locked bit and create lock > + * seque...

...d; + } +} + /** * xchg_tail - Put in the new queue tail code word & retrieve previous one * @lock : Pointer to queue spinlock structure @@ -112,12 +201,17 @@ xchg_tail(struct qspinlock *lock, u32 tail, u32 *pval) *pval = new; return old; } +#endif /* _Q_PENDING_BITS == 8 */ /** * trylock_pending - try to acquire queue spinlock using the pending bit * @lock : Pointer to queue spinlock structure * @pval : Pointer to value of the queue spinlock 32-bit word * Return: 1 if lock acquired, 0 otherwise + * + * The pending bit won't be set as soon as one or more tasks queue up. + * Thi...

...__always_inline void > >>+clear_pending_set_locked(struct qspinlock *lock, u32 val) > >>+{ > >>+ struct __qspinlock *l = (void *)lock; > >>+ > >>+ ACCESS_ONCE(l->locked_pending) = 1; > >>+} > >>@@ -157,8 +251,13 @@ static inline int trylock_pending(struct qspinlock *lock, u32 *pval) > >> * we're pending, wait for the owner to go away. > >> * > >> * *,1,1 -> *,1,0 > >>+ * > >>+ * this wait loop must be a load-acquire such that we match the > >>+ * store-release that c...

...__always_inline void > >>+clear_pending_set_locked(struct qspinlock *lock, u32 val) > >>+{ > >>+ struct __qspinlock *l = (void *)lock; > >>+ > >>+ ACCESS_ONCE(l->locked_pending) = 1; > >>+} > >>@@ -157,8 +251,13 @@ static inline int trylock_pending(struct qspinlock *lock, u32 *pval) > >> * we're pending, wait for the owner to go away. > >> * > >> * *,1,1 -> *,1,0 > >>+ * > >>+ * this wait loop must be a load-acquire such that we match the > >>+ * store-release that c...

.... > >> */ > >>+ if (!(val& _Q_TAIL_MASK)&& queue_spin_trylock(lock)) > >>+ goto release; > >But you just did a potentially very expensive op; @val isn't > >representative anymore! > > That is not true. I pass in a pointer to val to trylock_pending() (the > pointer thing) so that it will store the latest value that it reads from the > lock back into val. I did miss one in the PV qspinlock exit loop. I will add > it back when I do the next version. But you did that read _before_ you touched a cold cacheline, that's 100s of cyc...

.... > >> */ > >>+ if (!(val& _Q_TAIL_MASK)&& queue_spin_trylock(lock)) > >>+ goto release; > >But you just did a potentially very expensive op; @val isn't > >representative anymore! > > That is not true. I pass in a pointer to val to trylock_pending() (the > pointer thing) so that it will store the latest value that it reads from the > lock back into val. I did miss one in the PV qspinlock exit loop. I will add > it back when I do the next version. But you did that read _before_ you touched a cold cacheline, that's 100s of cyc...