Re: [PATCH] memcg: Always call cond_resched() after fn()

[Breno Leitao <leitao@debian.org>]

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