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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6f0fd4d7-6ce0-6427-e5e2-39f94509b462@redhat.com>
Date:   Wed, 17 Apr 2019 12:35:06 -0400
From:   Waiman Long <longman@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>, Will Deacon <will.deacon@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org, x86@...nel.org,
        Davidlohr Bueso <dave@...olabs.net>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        huang ying <huang.ying.caritas@...il.com>
Subject: Re: [PATCH v4 07/16] locking/rwsem: Implement lock handoff to prevent
 lock starvation

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