linux-kernel - Re: [PATCH] locking/rwsem: Take read lock immediate if empty queue with no writer

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 for Android: free password hash cracker in your pocket

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

Message-ID: <c575a0ab-419b-c0ea-f550-7c5daebbba25@redhat.com>
Date:   Fri, 13 Jul 2018 14:32:45 -0400
From:   Waiman Long <longman@...hat.com>
To:     Will Deacon <will.deacon@....com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Mark Ray <mark.ray@....com>, Joe Mario <jmario@...hat.com>,
        Scott Norton <scott.norton@....com>
Subject: Re: [PATCH] locking/rwsem: Take read lock immediate if empty queue
 with no writer

On 07/13/2018 06:02 AM, Will Deacon wrote:
> On Tue, Jul 10, 2018 at 02:31:30PM -0400, Waiman Long wrote:
>> It was found that a constant stream of readers might cause the count to
>> go negative most of the time after an initial trigger by a writer even
>> if no writer was present afterward. As a result, most of the readers
>> would have to go through the slowpath reducing their performance.
>>
>> To avoid that from happening, an additional check is added to detect
>> the special case that the reader in the critical section is the only
>> one in the wait queue and no writer is present. When that happens, it
>> can just have the lock and return immediately without further action.
>> Other incoming readers won't see a waiter is present and be forced
>> into the slowpath.
>>
>> After the list_empty() calls, the CPU should have the lock cacheline
>> anyway, so an additional semaphore count check shouldn't have any
>> performance impact.
>>
>> Signed-off-by: Waiman Long <longman@...hat.com>
>> ---
>>  kernel/locking/rwsem-xadd.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
> This looks ok to me, but it would be nice to include some performance
> figures in the commit log. Do you have any? Phrases such as "shouldn't have
> any performance impact" and "probably generate better code" don't fill me
> with good feelings ;)
>
> Will
I just got the customer testing results back. I had posted a v2 patch
with the customer data. I also remove wordings that may cause confusion.

Cheers,
Longman