Re: [PATCH v5] mm/oom_kill.c: futex: Close a race between do_exit and the oom_reaper

[Michal Hocko <mhocko@suse.com>]

Nico,

On Wed 23-03-22 10:17:29, Michal Hocko wrote:
> Let me skip over futex part which I need to digest and only focus on the
> oom side of the things for clarification.
> 
> On Tue 22-03-22 23:43:18, Thomas Gleixner wrote:
[...]
> > You can easily validate that by doing:
> > 
> > wake_oom_reaper(task)
> >    task->reap_time = jiffies + HZ;
> >    queue_task(task);
> >    wakeup(reaper);
> > 
> > and then:
> > 
> > oom_reap_task(task)
> >     now = READ_ONCE(jiffies);
> >     if (time_before(now, task->reap_time)
> >         schedule_timeout_idle(task->reap_time - now);
> > 
> > before trying to actually reap the mm.
> > 
> > That will prevent the enforced race in most cases and allow the exiting
> > and/or killed processes to cleanup themself. Not pretty, but it should
> > reduce the chance of the reaper to win the race with the exiting and/or
> > killed process significantly.
> > 
> > It's not going to work when the problem is combined with a heavy VM
> > overload situation which keeps a guest (or one/some it's vCPUs) away
> > from being scheduled. See below for a discussion of guarantees.
> > 
> > If it failed to do so when the sleep returns, then you still can reap
> > it.
> 
> Yes, this is certainly an option. Please note that the oom_reaper is not
> the only way to trigger this. process_mrelease syscall performs the same
> operation from the userspace. Arguably process_mrelease could be used
> sanely/correctly because the userspace oom killer can do pro-cleanup
> steps before going to final SIGKILL & process_mrelease. One way would be
> to send SIGTERM in the first step and allow the victim to perform its
> cleanup.

are you working on another version of the fix/workaround based on the
discussion so far?

I guess the most reasonable way forward is to rework
oom_reaper processing to be delayed. This can be either done by a
delayed wake up or as Thomas suggests above by postponing the
processing. I think the delayed wakeup would be _slightly_ easier to
handle because the queue can contain many tasks to be reaped.

More specifically something like delayed work but we cannot rely on
the WQ here. I guess we do not have any delayed wait queue interface
but the same trick with the timer should be applicable here as well.
exit_mmap would then cancel the timer after __oom_reap_task_mm is done.
Actually the timer could be canceled after mmu_notifier_release already
but this shouldn't make much of a difference.
-- 
Michal Hocko
SUSE Labs