linux-kernel - Re: [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff

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>] [day] [month] [year] [list]

Date:   Tue, 18 Oct 2022 22:49:02 -0400
From:   Waiman Long <longman@...hat.com>
To:     Hillf Danton <hdanton@...a.com>
Cc:     Mukesh Ojha <quic_mojha@...cinc.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        linux-kernel@...r.kernel.org, john.p.donnelly@...cle.com,
        Ting11 Wang 王婷 <wangting11@...omi.com>
Subject: Re: [PATCH v3 4/5] locking/rwsem: Enable direct rwsem lock handoff


On 10/18/22 22:29, Hillf Danton wrote:
> On 18 Oct 2022 20:39:59 -0400 Waiman Long <longman@...hat.com>
>> On 10/18/22 19:51, Hillf Danton wrote:
>>> On 18 Oct 2022 13:37:20 -0400 Waiman Long <longman@...hat.com>
>>>> On 10/18/22 10:13, Mukesh Ojha wrote:
>>>>> On 10/18/2022 4:44 PM, Hillf Danton wrote:
>>>>>> On 17 Oct 2022 17:13:55 -0400 Waiman Long <longman@...hat.com>
>>>>>>> @@ -1067,13 +1119,33 @@ rwsem_down_read_slowpath(struct rw_semaphore
>>>>>>>                 return sem;
>>>>>>>             }
>>>>>>>             adjustment += RWSEM_FLAG_WAITERS;
>>>>>>> +    } else if ((count & RWSEM_FLAG_HANDOFF) &&
>>>>>>> +          ((count & RWSEM_LOCK_MASK) == RWSEM_READER_BIAS)) {
>>>>>> Could a couple of CPUs go read slow path in parallel?
>>>>>>
>>>> This is under wait_lock protection. So no parallel execution is possible.
>>> They individually add RWSEM_READER_BIAS to count before taking wait_lock,
>>> and the check for BIAS here does not cover the case of readers in parallel.
>>> Is this intended?
>>>
>>> Hillf
>> As I said in the patch description, the lock handoff can only be done if
>> we can be sure that there is no other active locks outstanding with the
>> handoff bit set. If at the time of the check, another reader come in and
>> adds its RWSEM_READER_BIAS, the check fail and the cpu will proceed to
>> put its waiter in the queue and begin sleeping. Hopefully, the last one
>> left will find that count has only its RWSEM_READER_BIAS and it can
>> start the handoff process.
> If handoff grants rwsem to a read waiter then the read fast path may revive.
I don't quite understand what you mean by "read fast path may revive".
> And at the time of the check, multiple readers do not break handoff IMO.

I am not saying that multiple readers will break handoff. They will just 
delay it until all their temporary RWSEM_READ_BIAS are taken off.

Cheers,
Longman