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

David Rientjes
2011-05-24T01:40:04Z

On Tue, 24 May 2011, KOSAKI Motohiro wrote:

> > > 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. ]
>
> You misunderstand the code. Both select_bad_process() and oom_kill_process()
> are under tasklist_lock(). IOW, no change lock holding time.
>

A second iteration through the tasklist in select_bad_process() will
extend the time that tasklist_lock is held, which is what your patch does.