Re: [PATCH] locking/qrwlock: Allow multiple spinning readers

From: Peter Zijlstra
Date: Fri Apr 01 2016 - 06:29:41 EST


On Thu, Mar 31, 2016 at 06:12:38PM -0400, Waiman Long wrote:
> On 03/29/2016 04:20 PM, Peter Zijlstra wrote:
> >> cnts = atomic_add_return_acquire(_QR_BIAS,&lock->cnts) - _QR_BIAS;
> >>+ while ((cnts& _QW_WMASK) == _QW_LOCKED) {
> >>+ if (locked&& ((cnts>> _QR_SHIFT)< MAX_SPINNING_READERS)) {
> >>+ /*
> >>+ * Unlock the wait queue so that more readers can
> >>+ * come forward and waiting for the writer to exit
> >>+ * as long as no more than MAX_SPINNING_READERS
> >>+ * readers are present.
> >>+ */
> >>+ arch_spin_unlock(&lock->wait_lock);
> >>+ locked = false;
> >Only 1 more can come forward with this logic. How can you ever get to 4?
>
> Yes, each reader in the unlock path will release one in the queue. If the
> next one is also a reader, it will release one more and so on until the
> reader count reach 4 where the process will stop.
>
> >
> >Also, what says the next in queue is a reader?
>
> I did say in the changelog that the queue head could be a writer. In that
> case, the process will stop and the writer will wait until all the readers
> are gone.

Clearly I could not read not think straight when I looked at this patch.
You're right.

I'll go have another look ..