linux-kernel - Re: [patch -mm v2] mm: introduce oom_adj

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 Jul 2009 02:36:17 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Paul Menage <menage@...gle.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [patch -mm v2] mm: introduce oom_adj_child

On Fri, 31 Jul 2009, KAMEZAWA Hiroyuki wrote:

> > > Simply, reset_oom_adj_at_new_mm_context or some.
> > > 
> > 
> > I think it's preferred to keep the name relatively short which is an 
> > unfortuante requirement in this case.  I also prefer to start the name 
> > with "oom_adj" so it appears alongside /proc/pid/oom_adj when listed 
> > alphabetically.
> > 
> But misleading name is bad.
> 

Can you help think of any names that start with oom_adj_* and are 
relatively short?  I'd happily ack it.

> Why don't you think select_bad_process()-> oom_kill_task() implementation is bad ?

It livelocks if a thread is chosen and passed to oom_kill_task() while 
another per-thread oom_adj value is OOM_DISABLE for a thread sharing the 
same memory.

> IMHO, it's bad manner to fix an os-implementation problem by adding _new_ user
> interface which is hard to understand.
> 

How else do you propose the oom killer use oom_adj values on a per-thread 
basis without considering other threads sharing the same memory?  It does 
no good for the oom killer to kill a thread if another one sharing its 
memory is OOM_DISABLE because it can't kill all of them.  That means the 
memory cannot be freed and it must choose another task, but the end result 
is that it needlessly killed others.  That's not a desirable result, 
sorry.

> > In other words, the issue here is larger than the inheritance of the 
> > oom_adj value amongst children, it addresses a livelock that neither of 
> > your approaches solve.  The fix actually makes /proc/pid/oom_adj (and 
> > /proc/pid/oom_score) consistent with how the oom killer behaves.
> 
> This oom_adj_child itself is not related to livelock problem. Don't make
> the problem bigger than it is.
> oom_adj_child itself is just a problem how to handle vfork().
> 

Right, that's why it wasn't part of my original patchset which fixed the 
livelock.  We had seen that others had use cases where they still needed 
to set a newly initialized mm to have a starting oom_adj value different 
from its parent.  That's entirely understandable, and that's why I 
proposed oom_adj_child.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/