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

[Yueyang Pan <pyyjason@gmail.com>]

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@gmail.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@gmail.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