Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Wed, 29 Nov 2023 09:59:27 +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 4/7] mm: Centralize & improve oom reporting in show_mem.c
Message-ID: <ZWb9bwTQALvf5cMC@tiehlicka>
References: <20231122232515.177833-1-kent.overstreet@linux.dev>
 <20231122232515.177833-5-kent.overstreet@linux.dev>
 <ZWW71lfACwiHw3zk@tiehlicka>
 <20231128175439.6jarreie7cay74fn@moria.home.lan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231128175439.6jarreie7cay74fn@moria.home.lan>
Precedence: bulk

On Tue 28-11-23 12:54:39, Kent Overstreet wrote:
> On Tue, Nov 28, 2023 at 11:07:18AM +0100, Michal Hocko wrote:
[...]
> > > @@ -423,4 +426,21 @@ void __show_mem(unsigned int filter, nodemask_t *nodemask, int max_zone_idx)
> > >  #ifdef CONFIG_MEMORY_FAILURE
> > >  	printk("%lu pages hwpoisoned\n", atomic_long_read(&num_poisoned_pages));
> > >  #endif
> > > +
> > > +	buf = kmalloc(4096, GFP_ATOMIC);
> > 
> > I really do not think we want to allow allocations from the OOM context.
> > Is there any reason why this cannot be a statically allocated buffer?
> 
> You've made this claim before without ever giving any reasoning behind
> it.

We are actively out of memory at the stage you would like to allocate
memory. I am pretty sure I have mentioned this argument in the past.

> It's GFP_ATOMIC; it has to work from _interrupt_ context, OOM context is
> fine.

This is a completely _different_ contexts. GFP_ATOMIC works around IRQ
or any atomic context inability to wait for the reclaim by accessing the
memory reserves. At the _global_ oom situation those reserves are
extremely scarce resource. Debugging data is certainly not something we
would want to compete with e.g. oom victims or their dependencies that
might need to access those reserves in order to make a forward progress.
Especially in case where the said debugging data is not detrimental to
analyse the OOM situation. And to be completely honest, I haven't really
seen any strong arguments for shrinker internal state dumping other than
_in some very limited_ cases this _might_ be useful.

> And no, we don't want to burn 4k on a static buffer that is almost never
> used; people do care about making the kernel run on small systems.

WE DO NOT ALLOCATE FROM OOM context, not to mention for something as
disposable as debugging data which usefulness is not really clear. If
there are systems which cannot effort a 4kb for static buffer then just
disable this debugging data.
-- 
Michal Hocko
SUSE Labs