From: Dave Chinner <david@fromorbit.com>
To: Qi Zheng <qi.zheng@linux.dev>
Cc: Kirill Tkhai <tkhai@ya.ru>,
akpm@linux-foundation.org, roman.gushchin@linux.dev,
vbabka@suse.cz, viro@zeniv.linux.org.uk, brauner@kernel.org,
djwong@kernel.org, hughd@google.com, paulmck@kernel.org,
muchun.song@linux.dev, linux-mm@kvack.org,
linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
linux-kernel@vger.kernel.org,
Qi Zheng <zhengqi.arch@bytedance.com>
Subject: Re: [PATCH v2 0/3] mm: Make unregistration of super_block shrinker more faster
Date: Fri, 9 Jun 2023 07:58:46 +1000 [thread overview]
Message-ID: <ZIJPFuIxYpk1+TC5@dread.disaster.area> (raw)
In-Reply-To: <4176ef18-0125-dee8-f78a-837cb7a5c639@linux.dev>
On Wed, Jun 07, 2023 at 10:51:35AM +0800, Qi Zheng wrote:
> From my personal point of view, I think it is worth sacrificing the
> speed of unregistration alone compared to the benefits it brings
> (lockless shrink, etc).
Nobody is questioning whether this is a worthwhile improvement. The
lockless shrinker instance iteration is definitely a good direction
to move in. The problem is the -process- that has been followed has
lead to a very sub-optimal result.
> Of course, it would be better if there is a more perfect solution.
> If you have a better idea, it might be better to post the code first for
> discussion. Otherwise, I am afraid that if we just revert it, the
> problem of shrinker_rwsem will continue for many years.
No, a revert doesn't mean we don't want the change; a revert means
the way the change was attempted has caused unexpected problems.
We need to go back to the drawing board and work out a better way to
do this.
> And hi Dave, I know you're mad that I didn't cc you in the original
> patch.
No, I'm not mad at you.
If I'm annoyed at anyone, it's the senior mm developers and
maintainers that I'm annoyed at - not informing relevant parties
about modifications to shrinker infrastructure or implementations
has lead to regressions escaping out to user systems multiple times
in the past.
Yet here we are again....
> Sorry again. How about splitting shrinker-related codes into
> the separate files? Then we can add a MAINTAINERS entry to it and add
> linux-fsdevel@vger.kernel.org to this entry? So that future people
> will not miss to cc fs folks.
I don't think that fixes the problem, because the scope if much
wider than fs-devel: look at all the different subsystems
that have a shrinker.
The whole kernel development process has always worked by the rule
that we're changing common infrastructure, all the subsystems using
that infrastructure need to be cc'd on the changes to the
infrastructure they are using. Just cc'ing -fsdevel isn't enough,
we've also got shrinkers in graphics driver infrastructure, *RCU*,
virtio, DM, bcache and various other subsystems.
And I'm betting most of them don't know that significant changes
have been made to how the shrinkers work....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2023-06-08 21:58 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <168599103578.70911.9402374667983518835.stgit@pro.pro>
[not found] ` <ZH5ig590WleaH1Ed@dread.disaster.area>
[not found] ` <ef1b0ecd-5a03-4256-2a7a-3e22b755aa53@ya.ru>
2023-06-06 22:02 ` Dave Chinner
2023-06-07 2:51 ` Qi Zheng
2023-06-08 21:58 ` Dave Chinner [this message]
[not found] ` <168599180526.70911.14606767590861123431.stgit@pro.pro>
[not found] ` <ZH6AA72wOd4HKTKE@P9FQF9L96D>
[not found] ` <ZH6K0McWBeCjaf16@dread.disaster.area>
2023-06-06 21:21 ` [PATCH v2 3/3] fs: Use delayed shrinker unregistration Kirill Tkhai
2023-06-06 22:30 ` Dave Chinner
2023-06-08 16:36 ` Theodore Ts'o
2023-06-08 23:17 ` Dave Chinner
2023-06-09 0:27 ` Andrew Morton
2023-06-09 2:50 ` Qi Zheng
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=ZIJPFuIxYpk1+TC5@dread.disaster.area \
--to=david@fromorbit.com \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=djwong@kernel.org \
--cc=hughd@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=muchun.song@linux.dev \
--cc=paulmck@kernel.org \
--cc=qi.zheng@linux.dev \
--cc=roman.gushchin@linux.dev \
--cc=tkhai@ya.ru \
--cc=vbabka@suse.cz \
--cc=viro@zeniv.linux.org.uk \
--cc=zhengqi.arch@bytedance.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox