Re: [PATCH] mm: memcontrol: don't lose track of exiting OOM victims

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] cgroup: zombie leader skipped in cgroup_migrate_add_task()
  breaks PROCS iter [1]
--

commit c9380747740bff9194e7da450313ada190919d85
Author: Chris Down <chris@chrisdown.name>

mm: memcontrol: don't lose track of exiting OOM victims

This commit passes CSS_TASK_ITER_WITH_DEAD to css_task_iter_start() in
mem_cgroup_scan_tasks() so that exiting OOM victims remain visible to the
OOM evaluator until their mm is torn down.

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 84f4a8fd4d16f..cb7cfae9ce5a6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1451,7 +1451,13 @@ void mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
>  		struct css_task_iter it;
>  		struct task_struct *task;
>  
> -		css_task_iter_start(&iter->css, CSS_TASK_ITER_PROCS, &it);
> +		/*
> +		 * An OOM victim can start exiting well before its mm is
> +		 * torn down. Keep it visible so oom_evaluate_task() can
> +		 * back off while there's still memory to free.
> +		 */
> +		css_task_iter_start(&iter->css, CSS_TASK_ITER_PROCS |
> +				    CSS_TASK_ITER_WITH_DEAD, &it);

[Severity: High]
This problem wasn't introduced by this patch, but can a CSS_TASK_ITER_PROCS
scan in mem_cgroup_scan_tasks() [1] miss a migrated multithreaded
process whose thread group leader has already exited?

When a thread group leader exits while sibling threads remain alive,
exit_notify() sets PF_EXITING on the leader and moves it to its css_set's
dying_tasks list while task->signal->live stays non-zero.  If the process is
then migrated via __cgroup_procs_write(), cgroup_migrate_add_task() skips
the zombie leader:

kernel/cgroup/cgroup.c:cgroup_migrate_add_task() {
	/* @task either already exited or can't exit until the end */
	if (task->flags & PF_EXITING)
		return;
    ...
	list_move_tail(&task->cg_list, &cset->mg_tasks);
}

Only the live non-leader threads move to the destination css_set, while the
zombie leader stays on the source css_set's dying_tasks list.  Because
css_task_iter_advance() under CSS_TASK_ITER_PROCS skips non-leader threads
and yields dying leaders while task->signal->live is non-zero,
CSS_TASK_ITER_PROCS iterators on the destination cgroup (including
mem_cgroup_scan_tasks() and __cgroup_kill()) skip the migrated process
entirely, while iterators on the source cgroup continue to see and act on it.

[1] https://sashiko.dev/#/bug/linux-5cd0ee80-1266-4ed8-90b6-cb34245e0333

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/avALaDR1_epaAfOp@chrisdown.name?part=1