Re: [PATCH v4 07/16] locking/rwsem: Implement lock handoff to prevent lock starvation

[Waiman Long]

On 04/17/2019 03:35 AM, Peter Zijlstra wrote:
> On Tue, Apr 16, 2019 at 02:16:11PM -0400, Waiman Long wrote:
>
>>>> @@ -324,6 +364,12 @@ static void __rwsem_mark_wake(struct rw_semaphore *sem,
>>>>  		adjustment -= RWSEM_FLAG_WAITERS;
>>>>  	}
>>>>  
>>>> +	/*
>>>> +	 * Clear the handoff flag
>>>> +	 */
>>> Right, but that is a trivial comment in the 'increment i' style, it
>>> clearly states what the code does, but completely fails to elucidate the
>>> code.
>>>
>>> Maybe:
>>>
>>> 	/*
>>> 	 * When we've woken a reader, we no longer need to force writers
>>> 	 * to give up the lock and we can clear HANDOFF.
>>> 	 */
>>>
>>> And I suppose this is required if we were the pickup of the handoff set
>>> above, but is there a guarantee that the HANDOFF was not set by a
>>> writer?
>> I can change the comment. The handoff bit is always cleared in
>> rwsem_try_write_lock() when the lock is successfully acquire. Will add a
>> comment to document that.
> That doesn't help much, because it drops ->wait_lock between setting it
> and acquiring it. So the read-acquire can interleave.
>
> I _think_ it works, but I'm having trouble explaining how exactly. I
> think because readers don't spin yet and thus wakeups abide by queue
> order.
>
> And the other way around should have (write) spinners terminate the
> moment they see HANDOFF set by a readers, but I'm not immediately seeing
> that either.
>
> I'll continue staring at that.
>
All writers acquire the lock by cmpxchg and they did check for the
handoff bit before attempting to acquire. So there is no way for write
spinners to acquire after they see the handoff bit.

Cheers,
Longman