linux-kernel - Re: [RFC 1/1] Add memory allocation info for cgroup oom

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: <aKM3j3geY7JiPGQ8@devbig569.cln6.facebook.com>
Date: Mon, 18 Aug 2025 07:24:15 -0700
From: Yueyang Pan <pyyjason@...il.com>
To: Joshua Hahn <joshua.hahnjy@...il.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>,
	Kent Overstreet <kent.overstreet@...ux.dev>,
	Usama Arif <usamaarif642@...il.com>, Michal Hocko <mhocko@...e.com>,
	David Rientjes <rientjes@...gle.com>,
	Shakeel Butt <shakeel.butt@...ux.dev>,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [RFC 1/1] Add memory allocation info for cgroup oom

On Thu, Aug 14, 2025 at 01:11:08PM -0700, Joshua Hahn wrote:
> On Thu, 14 Aug 2025 10:11:57 -0700 Yueyang Pan <pyyjason@...il.com> wrote:
> 
> > Enable show_mem for the cgroup oom case. We will have memory allocation 
> > information in such case for the machine.
> 
> Hi Pan,
> 
> Thank you for your patch! This makes sense to me. As for your concerns from the
> cover letter on whether this is too much information: personally I don't think
> so, but perhaps other developers will have different opinions?
> 
> I just have a few comments / nits.

Thanks for your comment, Joshua.

> 
> > Signed-off-by: Yueyang Pan <pyyjason@...il.com>
> > ---
> >  mm/oom_kill.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 17650f0b516e..3ca224028396 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -465,8 +465,10 @@ static void dump_header(struct oom_control *oc)
> >  		pr_warn("COMPACTION is disabled!!!\n");
> >  
> >  	dump_stack();
> > -	if (is_memcg_oom(oc))
> > +	if (is_memcg_oom(oc)) {
> >  		mem_cgroup_print_oom_meminfo(oc->memcg);
> > +		show_mem();
> 
> Below, there is a direct call to __show_mem, which limits node and zone
> filtering. I am wondering whether it would make sense to also call __show_mem
> with the same arguments? show_mem() is just a wrapper around __show_mem with
> default parameters (i.e. not filtering out nodes, not filtering out
> zones).

The reason why I call show_mem here directly is because cgroup is not bound to 
a specific zone or node (correctly me if I am wrong). Thus I simply invoke 
show_mem to show system-wide memory info.

> 
> If you think this makes sense, we can even take it out of the if-else statement
> and call it unconditionally. But this is just my opinion, please feel free to
> keep the unfiltered call if you believe that fits better in here.
> 
> > +	}
> 
> NIT: Should this closing brace be on the same line as the following else
> statement, as per the kernel style guide [1]

Sorry for this. I will run checkpatch for my formal patch definitely

> 
> >  	else {
> >  		__show_mem(SHOW_MEM_FILTER_NODES, oc->nodemask, gfp_zone(oc->gfp_mask));
> >  		if (should_dump_unreclaim_slab())
> > -- 
> > 2.47.3
> 
> Thanks again Pan, I hope you have a great day!
> Joshua
> 
> [1] https://docs.kernel.org/process/coding-style.html
> 
> Sent using hkml (https://github.com/sjp38/hackermail)

Sorry that I forgot to cc some maintainers so I added them in this reply.
Pan