linux-kernel - Re: [v3 PATCH 02/11] mm: vmscan: consolidate shrinker

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:   Thu, 7 Jan 2021 09:29:55 -0800
From:   Yang Shi <shy828301@...il.com>
To:     Roman Gushchin <guro@...com>
Cc:     Kirill Tkhai <ktkhai@...tuozzo.com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Dave Chinner <david@...morbit.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Michal Hocko <mhocko@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux MM <linux-mm@...ck.org>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [v3 PATCH 02/11] mm: vmscan: consolidate shrinker_maps handling code

On Wed, Jan 6, 2021 at 4:14 PM Roman Gushchin <guro@...com> wrote:
>
> On Tue, Jan 05, 2021 at 02:58:08PM -0800, Yang Shi wrote:
> > The shrinker map management is not really memcg specific, it's just allocation
>
> In the current form it doesn't look so, especially because each name
> has a memcg_ prefix and each function takes a memcg argument.

That statement from commit log might be ambiguous and confusing. "Not
really memcg specific" doesn't mean it has nothing to do with memcg.
It is the intersection between memcg and shrinker. So, I don't think
of why it can't take a memcg argument. There are plenty of functions
from vmscan.c that take memcg as argument.

The direct reason for this consolidation is actually the following
patch which uses shrinker_rwsem to protect shrinker_maps allocation.
With this code consolidation we could keep the use of shrinker_rwsem
in one single file. And it also makes some sense to have shrinker
related code in vmscan.c, just like lruvec.

>
> It begs for some refactorings (Kirill suggested some) and renamings.

I apologize that I can't remember what specific suggestions from
Kirill you mean. Removing the "memcg_" prefix makes some sense to me,
we don't have "memcg_" prefix for lruvec either.

>
> Thanks!