Messages in this thread | | | Date | Sat, 31 Jan 2015 10:29:21 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 4/5] locking/rwsem: Avoid deceiving lock spinners |
| |
On Fri, Jan 30, 2015 at 01:14:26AM -0800, Davidlohr Bueso wrote: > +++ b/kernel/locking/rwsem-xadd.c > @@ -337,21 +337,30 @@ static inline bool owner_running(struct rw_semaphore *sem, > static noinline > bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner) > { > + long count; > + > rcu_read_lock(); > while (owner_running(sem, owner)) { > + /* abort spinning when need_resched */ > + if (need_resched()) { > + rcu_read_unlock(); > + return false; > + } > > cpu_relax_lowlatency(); > } > rcu_read_unlock(); > > + if (READ_ONCE(sem->owner)) > + return true; /* new owner, continue spinning */ > +
Same concern as Tim; also the mutex code seems to terminate the spin when owner changes. And I think we want to have writers behave similar to mutexes, no?
Does it make sense to change things to allow owner changes from NULL, but not to NULL?
> /* > + * When the owner is not set, the lock could be free or > + * held by readers. Check the counter to verify the > + * state. > */ > - return sem->owner == NULL; > + count = READ_ONCE(sem->count); > + return (count == 0 || count == RWSEM_WAITING_BIAS); > }
| |