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

[Joshua Hahn <joshua.hahnjy@gmail.com>]

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.

> 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).

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]

>  	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)