Messages in this thread |  | | Subject | Re: [PATCH 2/2] locking/qspinlock: Limit # of spins in _Q_PENDING_VAL wait loop | From | Waiman Long <> | Date | Wed, 11 Apr 2018 14:06:12 -0400 |
| |
On 04/11/2018 11:22 AM, Catalin Marinas wrote: > Hi Waiman, > > On Mon, Apr 09, 2018 at 02:08:52PM -0400, Waiman Long wrote: >> @@ -311,13 +320,19 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val) >> return; >> >> /* >> - * wait for in-progress pending->locked hand-overs >> + * wait for in-progress pending->locked hand-overs with a >> + * limited number of spins. >> * >> * 0,1,0 -> 0,0,1 >> */ >> if (val == _Q_PENDING_VAL) { >> - while ((val = atomic_read(&lock->val)) == _Q_PENDING_VAL) >> + int cnt = _Q_PENDING_LOOP; >> + >> + while ((val = atomic_read(&lock->val)) == _Q_PENDING_VAL) { >> + if (!--cnt) >> + goto queue; >> cpu_relax(); >> + } >> } > In my model, the pathological case is not this loop but the following > one (trylock || pending): > > P0: P1: > queued_spin_lock() fails queued_spin_lock() succeeds > queued_spin_lock_slowpath() > val == _Q_LOCKED_VAL > new = _Q_LOCKED_VAL | > _Q_PENDING_VAL > queued_spin_unlock() > lock->val == 0 > cmpxchg(&lock->val, val, new) > fails > val = old (0) > repeat for (;;) loop: > new = _Q_LOCKED_VAL > queued_spin_lock() succeeds > lock->val == _Q_LOCKED_VAL > cmpxchg(&lock->val, val, new) > fails > val = old (_Q_LOCKED_VAL) > repeat for (;;) loop: > > ... and we are back to the P0 state above when it first entered > the loop, hence no progress. P1 never enters slowpath.
I don't see any problem in removing this second loop. Thanks for running tool to check for problem.
-Longman
|  |