All of lore.kernel.org
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Tim Murray <timmurray@google.com>,
	Christoph Hellwig <hch@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-f2fs-devel@lists.sourceforge.net,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>
Subject: Re: [PATCH] f2fs: move f2fs to use reader-unfair rwsems
Date: Wed, 12 Jan 2022 10:08:18 -0500	[thread overview]
Message-ID: <fd02584d-03d7-f27b-c11c-6ed1f212f03c@redhat.com> (raw)
In-Reply-To: <Yd4f49lhnC7+vvAm@google.com>

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


WARNING: multiple messages have this Message-ID (diff)
From: Waiman Long <longman@redhat.com>
To: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Tim Murray <timmurray@google.com>,
	linux-f2fs-devel@lists.sourceforge.net,
	Christoph Hellwig <hch@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>
Subject: Re: [f2fs-dev] [PATCH] f2fs: move f2fs to use reader-unfair rwsems
Date: Wed, 12 Jan 2022 10:08:18 -0500	[thread overview]
Message-ID: <fd02584d-03d7-f27b-c11c-6ed1f212f03c@redhat.com> (raw)
In-Reply-To: <Yd4f49lhnC7+vvAm@google.com>

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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2022-01-12 15:08 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-08 16:46 [PATCH] f2fs: move f2fs to use reader-unfair rwsems Jaegeuk Kim
2022-01-08 16:46 ` [f2fs-dev] " Jaegeuk Kim
2022-01-10  8:05 ` Christoph Hellwig
2022-01-10  8:05   ` [f2fs-dev] " Christoph Hellwig
2022-01-10 15:02   ` Peter Zijlstra
2022-01-10 15:02     ` [f2fs-dev] " Peter Zijlstra
2022-01-10 16:18   ` Waiman Long
2022-01-10 16:18     ` [f2fs-dev] " Waiman Long
2022-01-10 18:45     ` Peter Zijlstra
2022-01-10 18:45       ` [f2fs-dev] " Peter Zijlstra
2022-01-10 18:55       ` Waiman Long
2022-01-10 18:55         ` [f2fs-dev] " Waiman Long
2022-01-10 19:41     ` Tim Murray
2022-01-10 19:41       ` [f2fs-dev] " Tim Murray via Linux-f2fs-devel
2022-01-11  4:15       ` Waiman Long
2022-01-11  4:15         ` [f2fs-dev] " Waiman Long
2022-01-11  6:53         ` Tim Murray
2022-01-11  6:53           ` [f2fs-dev] " Tim Murray via Linux-f2fs-devel
2022-01-11 15:04           ` Waiman Long
2022-01-11 15:04             ` [f2fs-dev] " Waiman Long
2022-01-11 17:07             ` Jaegeuk Kim
2022-01-11 17:07               ` [f2fs-dev] " Jaegeuk Kim
2022-01-11 22:01               ` Waiman Long
2022-01-11 22:01                 ` [f2fs-dev] " Waiman Long
2022-01-12  0:25                 ` Jaegeuk Kim
2022-01-12  0:25                   ` [f2fs-dev] " Jaegeuk Kim
2022-01-12 15:08                   ` Waiman Long [this message]
2022-01-12 15:08                     ` Waiman Long
2022-01-12 14:06       ` Peter Zijlstra
2022-01-12 14:06         ` [f2fs-dev] " Peter Zijlstra
2022-01-12 22:06         ` Tim Murray
2022-01-12 22:06           ` [f2fs-dev] " Tim Murray via Linux-f2fs-devel
2022-02-23  7:42         ` Christoph Hellwig
2022-02-23  7:42           ` [f2fs-dev] " Christoph Hellwig
2022-02-23 17:48           ` Jaegeuk Kim
2022-02-23 17:48             ` [f2fs-dev] " Jaegeuk Kim
2022-02-24  8:47 ` Hillf Danton
2022-02-25  1:47   ` Hillf Danton
2022-02-25  8:32     ` Hillf Danton
2022-02-28  1:12       ` Hillf Danton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fd02584d-03d7-f27b-c11c-6ed1f212f03c@redhat.com \
    --to=longman@redhat.com \
    --cc=boqun.feng@gmail.com \
    --cc=hch@infradead.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=timmurray@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.