Re: [patch -mm] memcg: make oom killer a no-op when no killable task can be found

[David Rientjes <rientjes@google.com>]

On Mon, 5 Apr 2010, Andrew Morton wrote:

> > > It's pointless to try to kill current if select_bad_process() did not
> > > find an eligible task to kill in mem_cgroup_out_of_memory() since it's
> > > guaranteed that current is a member of the memcg that is oom and it is,
> > > by definition, unkillable.
> > > 
> > > Signed-off-by: David Rientjes <rientjes@google.com>
> > > ---
> > >  mm/oom_kill.c |    5 +----
> > >  1 files changed, 1 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > --- a/mm/oom_kill.c
> > > +++ b/mm/oom_kill.c
> > > @@ -500,12 +500,9 @@ void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t gfp_mask)
> > >  	read_lock(&tasklist_lock);
> > >  retry:
> > >  	p = select_bad_process(&points, limit, mem, CONSTRAINT_NONE, NULL);
> > > -	if (PTR_ERR(p) == -1UL)
> > > +	if (!p || PTR_ERR(p) == -1UL)
> > >  		goto out;
> > >  
> > > -	if (!p)
> > > -		p = current;
> > > -
> > >  	if (oom_kill_process(p, gfp_mask, 0, points, limit, mem,
> > >  				"Memory cgroup out of memory"))
> > >  		goto retry;
> > > 
> > 
> > Are there any objections to merging this?  It's pretty straight-forward 
> > given the fact that oom_kill_process() would fail if select_bad_process() 
> > returns NULL even if p is set to current since it was not found to be 
> > eligible during the tasklist scan.
> 
> I've lost the plot on the oom-killer patches.  Half the things I'm
> seeing don't even apply.
> 

This patch applies cleanly on mmotm-2010-03-24-14-48 and I don't see 
anything that has been added since then that touches 
mem_cgroup_out_of_memory().

> Perhaps I should drop the lot and we start again.  We still haven't
> resolved the procfs back-compat issue, either.

I haven't seen any outstanding compatibility issues raised.  The only 
thing that isn't backwards compatible is consolidating 
/proc/sys/vm/oom_kill_allocating_task and /proc/sys/vm/oom_dump_tasks into 
/proc/sys/vm/oom_kill_quick.  We can do that because we've enabled 
oom_dump_tasks by default so that systems that use both of these tunables 
need to now disable oom_dump_tasks to avoid the costly tasklist scan.  
Both tunables would then have the same audience, i.e. users would never 
want to enable one without the other, so it's possible to consolidate 
them.

Nobody, to my knowledge, has objected to that reasoning and removing 
dozens of patches from -mm isn't the answer for (yet to be raised) 
questions about a single change.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>