linux-kernel - Re: [PATCH] memcg: Always call cond

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: <aDbU/ApoHK9SRXzv@gmail.com>
Date: Wed, 28 May 2025 02:18:52 -0700
From: Breno Leitao <leitao@...ian.org>
To: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: Johannes Weiner <hannes@...xchg.org>, Michal Hocko <mhocko@...nel.org>,
	Roman Gushchin <roman.gushchin@...ux.dev>,
	Muchun Song <muchun.song@...ux.dev>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Chen Ridong <chenridong@...wei.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Michal Hocko <mhocko@...e.com>, cgroups@...r.kernel.org,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	kernel-team@...a.com, Michael van der Westhuizen <rmikey@...a.com>,
	Usama Arif <usamaarif642@...il.com>,
	Pavel Begunkov <asml.silence@...il.com>,
	Rik van Riel <riel@...riel.com>
Subject: Re: [PATCH] memcg: Always call cond_resched() after fn()

Hello Shakeel,

On Tue, May 27, 2025 at 09:54:10AM -0700, Shakeel Butt wrote:
> On Tue, May 27, 2025 at 03:03:34AM -0700, Breno Leitao wrote:
> > 
> > Not sure I followed you here. __oom_kill_process is doing the following:
> > 
> >   static void __oom_kill_process(struct task_struct *victim, const char *message)
> >   {
> > 	...
> >         pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB, UID:%u pgtables:%lukB oom_score_adj:%hd\n",
> > 
> > 
> > Would you use a buffer to print to, and them flush it at the same time
> > (with pr_err()?)
> > 
> 
> Something similar to what mem_cgroup_print_oom_meminfo() does with
> seq_buf.

Right, where do you want to flush this buffer? I suppose we want to do
it at once, in the caller (mem_cgroup_scan_tasks()), otherwise we will
have the same problem, I would say.

This is the code flow we are executing when I got this issue:

mem_cgroup_scan_tasks() {
	for_each_mem_cgroup_tree(iter, memcg) {
		ret = fn(task, arg); 		 //where fn() is oom_kill_memcg_member()
			oom_kill_memcg_member() {
				__oom_kill_process() {
					pr_info()
					pr_err()
				}
			}
	}
}

So, basically it prints one/two message(s) for each process, and goes to
2k processes, so, __oom_kill_process() is called 2k times when the memcg
is dying out.

mem_cgroup_print_oom_meminfo() seems a bit different, where it coalesces
a bunch of message in the same function and print them all at the same
time.

Another option is to create a buffer to mem_cgroup_scan_tasks(), but
then we need to pass it to all fn(), and pushing down the
replacement of printk() by seq_buf_printf(). Is this what you meant?

If so, another concern I have is the buffer size to be printed at once.
Let's suppose we have 2k "Killed process..." message in the buffer. Do
we want to print it at once? (without a cond_resched()?)

Thanks for the discussion,
--breno