Re: [patch v3] mm, oom: fix unnecessary killing of additional processes

From: David Rientjes
Date: Fri Jul 06 2018 - 20:05:55 EST

On Thu, 5 Jul 2018, Andrew Morton wrote:

> > +#ifdef CONFIG_DEBUG_FS
> > +static int oom_free_timeout_ms_read(void *data, u64 *val)
> > +{
> > + *val = oom_free_timeout_ms;
> > + return 0;
> > +}
> > +
> > +static int oom_free_timeout_ms_write(void *data, u64 val)
> > +{
> > + if (val > 60 * 1000)
> > + return -EINVAL;
> > +
> > + oom_free_timeout_ms = val;
> > + return 0;
> > +}
> > +DEFINE_SIMPLE_ATTRIBUTE(oom_free_timeout_ms_fops, oom_free_timeout_ms_read,
> > + oom_free_timeout_ms_write, "%llu\n");
> > +#endif /* CONFIG_DEBUG_FS */
> One of the several things I dislike about debugfs is that nobody
> bothers documenting it anywhere. But this should really be documented.
> I'm not sure where, but the documentation will find itself alongside a
> bunch of procfs things which prompts the question "why it *this* one in
> debugfs"?

The only reason I have placed it in debugfs, or making it tunable at all,
is to appease others. I know the non-default value we need to use to stop
millions of processes being oom killed unnecessarily. Michal suggested a
tunable to disable the oom reaper entirely, which is not what we want, so
I found this to be the best alternative.

I'd like to say that it is purposefully undocumented since it's not a
sysctl and nobody can suggest that it is becoming a permanent API that we
must maintain for backwards compatibility. Having it be configurable is
kind of ridiculous, but such is the nature of trying to get patches merged
these days to prevent millions of processes being oom killed

Blockable mmu notifiers and mlocked memory is not the extent of the
problem, if a process has a lot of virtual memory we must wait until
free_pgtables() completes in exit_mmap() to prevent unnecessary oom
killing. For implementations such as tcmalloc, which does not release
virtual memory, this is important because, well, it releases this only at
exit_mmap(). Of course we cannot do that with only the protection of
mm->mmap_sem for read.

This is a patch that we'll always need if we continue with the current
implementation of the oom reaper. I wouldn't suggest it as a configurable
value, but, owell.

I'll document the tunable and purposefully repeat myself that this is
addresses millions of processes being oom killed unnecessarily so the
rather important motivation of the change is clear to anyone who reads
this thread now or in the future. Nobody can guess an appropriate value
until they have been hit by the issue themselves and need to deal with the
loss of work from important processes being oom killed when some best
effort logging cron job uses too much memory. Or, of course, pissed off
users who have their jobs killed off and you find yourself in the rather
unfortunate situation of explaining why the Linux kernel in 2018 needs to
immediately SIGKILL processes because of an arbitrary nack related to a