Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 19 Apr 2022 15:46:49 -0400
From:   Kent Overstreet <kent.overstreet@gmail.com>
To:     Roman Gushchin <roman.gushchin@linux.dev>
Cc:     linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
        Dave Chinner <dchinner@redhat.com>,
        linux-kernel@vger.kernel.org, Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        Shakeel Butt <shakeelb@google.com>,
        Yang Shi <shy828301@gmail.com>
Subject: Re: [PATCH rfc 0/5] mm: introduce shrinker sysfs interface
Message-ID: <20220419194649.32f3v45fgr2wztf5@moria.home.lan>
References: <20220416002756.4087977-1-roman.gushchin@linux.dev>
 <20220419182030.idqqmtim4slhbked@moria.home.lan>
 <Yl8GOJd/iUHkTEL3@carbon>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yl8GOJd/iUHkTEL3@carbon>
Precedence: bulk

On Tue, Apr 19, 2022 at 11:58:00AM -0700, Roman Gushchin wrote:
> On Tue, Apr 19, 2022 at 02:20:30PM -0400, Kent Overstreet wrote:
> > On Fri, Apr 15, 2022 at 05:27:51PM -0700, Roman Gushchin wrote:
> > > There are 50+ different shrinkers in the kernel, many with their own bells and
> > > whistles. Under the memory pressure the kernel applies some pressure on each of
> > > them in the order of which they were created/registered in the system. Some
> > > of them can contain only few objects, some can be quite large. Some can be
> > > effective at reclaiming memory, some not.
> > > 
> > > The only existing debugging mechanism is a couple of tracepoints in
> > > do_shrink_slab(): mm_shrink_slab_start and mm_shrink_slab_end. They aren't
> > > covering everything though: shrinkers which report 0 objects will never show up,
> > > there is no support for memcg-aware shrinkers. Shrinkers are identified by their
> > > scan function, which is not always enough (e.g. hard to guess which super
> > > block's shrinker it is having only "super_cache_scan"). They are a passive
> > > mechanism: there is no way to call into counting and scanning of an individual
> > > shrinker and profile it.
> > > 
> > > To provide a better visibility and debug options for memory shrinkers
> > > this patchset introduces a /sys/kernel/shrinker interface, to some extent
> > > similar to /sys/kernel/slab.
> > > 
> > > For each shrinker registered in the system a folder is created. The folder
> > > contains "count" and "scan" files, which allow to trigger count_objects()
> > > and scan_objects() callbacks. For memcg-aware and numa-aware shrinkers
> > > count_memcg, scan_memcg, count_node, scan_node, count_memcg_node
> > > and scan_memcg_node are additionally provided. They allow to get per-memcg
> > > and/or per-node object count and shrink only a specific memcg/node.
> > 
> > Cool!
> > 
> > I've been starting to sketch out some shrinker improvements of my own, perhaps
> > we could combine efforts.
> 
> Thanks! Absolutely!
> 
> > The issue I've been targeting is that when we hit an
> > OOM, we currently don't get a lot of useful information - shrinkers ought to be
> > included, and we really want information on shrinker's internal state (e.g.
> > object dirtyness) if we're to have a chance at understanding why memory isn't
> > getting reclaimed.
> > 
> > https://evilpiepirate.org/git/bcachefs.git/log/?h=shrinker_to_text
> > 
> > This adds a .to_text() method - a pretty-printer - that shrinkers can
> > implement, and then on OOM we report on the top 10 shrinkers by memory usage, in
> > sorted order.
> 
> We must be really careful with describing what's allowed and not allowed
> by these callbacks. In-kernel OOM is the last-resort mechanism and it should
> be able to make forward progress in really nasty circumstances. So there are
> significant (and not very well described) limitations on what can be done
> from the oom context.

Yep. The only "interesting" thing my patches add is that we heap-allocate the
strings the .to_text methods generate (which is good! it means they can be used
both for printing to the console, and by sysfs code). Memory allocation failure
here is hardly the end of the world; those messages will just get truncated, and
I'm also going to mempool-ify printbufs (might do that today).

> > Another thing I'd like to do is have shrinkers report usage not just in object
> > counts but in bytes; I think it should be obvious why that's desirable.
> 
> I totally agree, it's actually on my short-term todo list.

Wonderful. A request I often get is for bcachefs's caches to show up as cached
memory via the free command - a perfectly reasonable request - and reporting
byte counts would make this possible.