Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable

From: Peter Zijlstra
Date: Wed May 11 2016 - 05:17:51 EST


On Wed, May 11, 2016 at 11:04:42AM +0200, Michal Hocko wrote:
> On Wed 11-05-16 10:44:01, Peter Zijlstra wrote:
> [...]
> > @@ -504,6 +502,18 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state)
> > raw_spin_unlock_irq(&sem->wait_lock);
> >
> > return ret;
> > +
> > +out_nolock:
> > + __set_current_state(TASK_RUNNING);
> > + raw_spin_lock_irq(&sem->wait_lock);
> > + list_del(&waiter.list);
> > + if (list_empty(&sem->wait_list))
> > + rwsem_atomic_update(-RWSEM_WAITING_BIAS, sem);
> > + else
> > + __rwsem_do_wake(sem, RWSEM_WAKE_READERS);
> > + raw_spin_unlock_irq(&sem->wait_lock);
> > +
> > + return ERR_PTR(-EINTR);
> > }
>
> Looks much better but don't we have to check the count for potentially
> pending writers?

Ah, so I was thinking that if we get here, there must still be an owner,
otherwise we'd have acquired the lock. And if there is an owner, we
cannot go wake writers. Hence the WAKE_READERS thing.

Then again, WAKE_ANY would not harm, in that if we do wake a pending
writer it will not proceed if it cannot and it'll go back to sleep
again.

So yeah, maybe WAKE_ANY is the prudent thing to do.