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] [day] [month] [year] [list]

Message-ID: <20250814201114.1921580-1-joshua.hahnjy@gmail.com>
Date: Thu, 14 Aug 2025 13:11:08 -0700
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Yueyang Pan <pyyjason@...il.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>,
	Kent Overstreet <kent.overstreet@...ux.dev>,
	Usama Arif <usamaarif642@...il.com>,
	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, 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.

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

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)