linux-kernel - Re: [PATCH] f2fs: move f2fs to use reader-unfair rwsems

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]

Date:   Wed, 12 Jan 2022 10:08:18 -0500
From:   Waiman Long <longman@...hat.com>
To:     Jaegeuk Kim <jaegeuk@...nel.org>
Cc:     Tim Murray <timmurray@...gle.com>,
        Christoph Hellwig <hch@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-f2fs-devel@...ts.sourceforge.net,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>
Subject: Re: [PATCH] f2fs: move f2fs to use reader-unfair rwsems

On 1/11/22 19:25, Jaegeuk Kim wrote:
> On 01/11, Waiman Long wrote:
>>
>> v5.10 kernel still have reader optimistic spinning enabled in rwsem which
>> may have worsen the writer wait time. Could you try with a more up-to-date
>> kernel or backport the relevant rwsem patches into your test kernel to see
>> how much it can help?
> We're using https://android.googlesource.com/kernel/common/+/refs/heads/android12-5.10.
> By any chance, may I ask which upstream patches we need to backport?
>
I am referring to the following commits:

617f3ef95177 locking/rwsem: Remove reader optimistic spinning
1a728dff855a locking/rwsem: Enable reader optimistic lock stealing
2f06f702925b locking/rwsem: Prevent potential lock starvation
c8fe8b056438 locking/rwsem: Pass the current atomic count to 
rwsem_down_read_slowpath()

To apply cleanly on top of 5.10.y, you will also need the followings:

c995e638ccbb locking/rwsem: Fold __down_{read,write}*()
285c61aedf6b locking/rwsem: Introduce rwsem_write_trylock()
3379116a0ca9 locking/rwsem: Better collate rwsem_read_trylock()

Reading the commit log of 2f06f702925b ("locking/rwsem: Prevent 
potential lock starvation"), I realize that writer lock starvation is 
possible in the f2fs case. That can explain why there was a worst case 
lock wait time of 9.7s.

I believe that you will see a big improvement by applying those upstream 
commits. In hindsight, I think I should have put a "Fixes" tag in that 
commit.

>>
>>>> Anyway, AFAICS, this patch keeps readers out of the rwsem wait queue and so
>>>> only writers can go into it. We can make an unfair rwsem to give preference
>>>> to writers in the wait queue and wake up readers only if there is no more
>>>> writers in the wait queue or even in the optimistic spinning queue. That
>>>> should achieve the same effect as this patch.
>>> Can we get a patch for the variant to test a bit? Meanwhile, I think we can
>>> merge this patch to add a wraper first and switches to it later?
>> Give me a week or so and I can make a RFC patch to support unfair rwsem for
>> you to try out. You do need to try it on the latest kernel, though.
> Thank you so much. My thought flow is applying this in f2fs for all the old
> kernels shipped in Android devices. Then, we can try to backport upstream
> rwsem patches and yours to switch finally. Let me know if you have any concern.

Assuming that Tr is the worst case reader lock hold time with a single 
writer, I believe the worst case writer lock wait time should be about 
2*Tr with the above commits applied. Introducing a unfair rwsem option 
will reduce that to just Tr. So try this out by applying the above 
upstream commits to see if they can meet your requirement as you may not 
really need an unfair rwsem option.

Cheers,
Longman