linux-kernel - Re: [PATCH v2 0/3] mm: Make unregistration of super

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]

Message-ID: <ZIJPFuIxYpk1+TC5@dread.disaster.area>
Date:   Fri, 9 Jun 2023 07:58:46 +1000
From:   Dave Chinner <david@...morbit.com>
To:     Qi Zheng <qi.zheng@...ux.dev>
Cc:     Kirill Tkhai <tkhai@...ru>, akpm@...ux-foundation.org,
        roman.gushchin@...ux.dev, vbabka@...e.cz, viro@...iv.linux.org.uk,
        brauner@...nel.org, djwong@...nel.org, hughd@...gle.com,
        paulmck@...nel.org, muchun.song@...ux.dev, linux-mm@...ck.org,
        linux-fsdevel@...r.kernel.org, linux-xfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, Qi Zheng <zhengqi.arch@...edance.com>
Subject: Re: [PATCH v2 0/3] mm: Make unregistration of super_block shrinker
 more faster

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@...r.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@...morbit.com