Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Wed, 29 Nov 2023 17:02:35 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     Kent Overstreet <kent.overstreet@linux.dev>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Qi Zheng <zhengqi.arch@bytedance.com>,
        Roman Gushchin <roman.gushchin@linux.dev>
Subject: Re: [PATCH 2/7] mm: shrinker: Add a .to_text() method for shrinkers
Message-ID: <ZWdgm2I3tF0zvHG9@tiehlicka>
References: <20231122232515.177833-1-kent.overstreet@linux.dev>
 <20231122232515.177833-3-kent.overstreet@linux.dev>
 <ZWW6bInvMA2x3mHC@tiehlicka>
 <20231128174853.vdpwullepoxg5blo@moria.home.lan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231128174853.vdpwullepoxg5blo@moria.home.lan>
Precedence: bulk

On Tue 28-11-23 12:48:53, Kent Overstreet wrote:
> On Tue, Nov 28, 2023 at 11:01:16AM +0100, Michal Hocko wrote:
> > On Wed 22-11-23 18:25:07, Kent Overstreet wrote:
> > [...]
> > > +void shrinkers_to_text(struct seq_buf *out)
> > > +{
> > > +	struct shrinker *shrinker;
> > > +	struct shrinker_by_mem {
> > > +		struct shrinker	*shrinker;
> > > +		unsigned long	mem;
> > > +	} shrinkers_by_mem[10];
> > > +	int i, nr = 0;
> > > +
> > > +	if (!mutex_trylock(&shrinker_mutex)) {
> > > +		seq_buf_puts(out, "(couldn't take shrinker lock)");
> > > +		return;
> > > +	}
> > > +
> > > +	list_for_each_entry(shrinker, &shrinker_list, list) {
> > > +		struct shrink_control sc = { .gfp_mask = GFP_KERNEL, };
> > 
> > This seems to be global reclaim specific. What about memcg reclaim?
> 
> I have no fsckin idea how memcg reclaim works - and, for that matter,
> the recent lockless shrinking work seems to have neglected to write even
> an iterator macro, leaving _that_ a nasty mess so I'm not touching that
> either.

OK, you could have made it more clearly that all of this is aiming at
the global OOM handling. With an outlook on what should be done if this
was ever required.

Another thing you want to look into is a NUMA constrained OOM (mbind,
cpuset) where this output could be actively misleading.

-- 
Michal Hocko
SUSE Labs