Messages in this thread Patch in this message | | | Date | Wed, 11 May 2016 10:28:53 +0200 | From | Michal Hocko <> | Subject | Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable |
| |
On Wed 11-05-16 09:23:57, Michal Hocko wrote: > On Tue 10-05-16 14:38:06, Peter Zijlstra wrote: [...] > > However, at the time I was thinking that if we have: > > > > reader (owner) > > writer (pending) > > reader (blocked on writer) > > > > and writer would get cancelled, the up_read() would do a wakeup and kick > > the blocked reader. > > > > But yes, immediately kicking further pending waiters might be better. > > OK, that makes sense. We shouldn't be waiting for the first reader to > do up_read.
I am still trying to wrap my head around the wake up logic (I managed to forget all the juicy details). I still do not see the correctness issue with the current code so the following should be "just an optimization". But as I've said I might be missing something here.
Does the following look correct/reasonable? This is absolutely untested and more for a discussion: --- diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index df4dcb883b50..726620c97366 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -516,7 +516,27 @@ EXPORT_SYMBOL(rwsem_down_write_failed); __visible struct rw_semaphore * __sched rwsem_down_write_failed_killable(struct rw_semaphore *sem) { - return __rwsem_down_write_failed_common(sem, TASK_KILLABLE); + struct rw_semaphore * ret; + + ret = __rwsem_down_write_failed_common(sem, TASK_KILLABLE); + if (IS_ERR(ret)) { + long count; + + raw_spin_lock_irq(&sem->wait_lock); + count = READ_ONCE(sem->count); + + /* + * If there were already threads queued before us and there are + * no active writers, the lock must be read owned; so we try to + * wake any read locks that were queued ahead of us so they + * do not have to wait for up_read to wake up. + */ + if (count > RWSEM_WAITING_BIAS) + sem = __rwsem_do_wake(sem, RWSEM_WAKE_READERS); + raw_spin_unlock_irq(&sem->wait_lock); + } + + return ret; } EXPORT_SYMBOL(rwsem_down_write_failed_killable); -- Michal Hocko SUSE Labs
| |