Re: [PATCH 4/5] oom: don't kill random process

David Rientjes
2011-05-23T22:32:55Z

On Fri, 20 May 2011, KOSAKI Motohiro wrote:

> CAI Qian reported oom-killer killed all system daemons in his
> system at first if he ran fork bomb as root. The problem is,
> current logic give them bonus of 3% of system ram. Example,
> he has 16GB machine, then root processes have ~500MB oom
> immune. It bring us crazy bad result. _all_ processes have
> oom-score=1 and then, oom killer ignore process memory usage
> and kill random process. This regression is caused by commit
> a63d83f427 (oom: badness heuristic rewrite).
>
> This patch changes select_bad_process() slightly. If oom points == 1,
> it's a sign that the system have only root privileged processes or
> similar. Thus, select_bad_process() calculate oom badness without
> root bonus and select eligible process.
>

You said earlier that you thought it was a good idea to do a proportional
based bonus for root processes. Do you have a specific objection to
giving root processes a 1% bonus for every 10% of used memory instead?

> Also, this patch move finding sacrifice child logic into
> select_bad_process(). It's necessary to implement adequate
> no root bonus recalculation. and it makes good side effect,
> current logic doesn't behave as the doc.
>

This is unnecessary and just makes the oom killer egregiously long. We
are already diagnosing problems here at Google where the oom killer holds
tasklist_lock on the readside for far too long, causing other cpus waiting
for a write_lock_irq(&tasklist_lock) to encounter issues when irqs are
disabled and it is spinning. A second tasklist scan is simply a
non-starter.

[ This is also one of the reasons why we needed to introduce
mm->oom_disable_count to prevent a second, expensive tasklist scan. ]

> Documentation/sysctl/vm.txt says
>
> oom_kill_allocating_task
>
> If this is set to non-zero, the OOM killer simply kills the task that
> triggered the out-of-memory condition. This avoids the expensive
> tasklist scan.
>
> IOW, oom_kill_allocating_task shouldn't search sacrifice child.
> This patch also fixes this issue.
>

oom_kill_allocating_task was introduced for SGI to prevent the expensive
tasklist scan, the task that is actually allocating the memory isn't
actually interesting and is usually random. This should be turned into a
documentation fix rather than changing the implementation.

Thanks.