Re: [patch] memcg: give current access to memory reserves if it's trying to die

[David Rientjes <rientjes@google.com>]

On Thu, 17 Mar 2011, Andrew Morton wrote:

> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -537,6 +537,17 @@ void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t gfp_mask)
> >  	unsigned int points = 0;
> >  	struct task_struct *p;
> >  
> > +	/*
> > +	 * If current has a pending SIGKILL, then automatically select it.  The
> > +	 * goal is to allow it to allocate so that it may quickly exit and free
> > +	 * its memory.
> > +	 */
> > +	if (fatal_signal_pending(current)) {
> > +		set_thread_flag(TIF_MEMDIE);
> > +		boost_dying_task_prio(current, NULL);
> > +		return;
> > +	}
> > +
> >  	check_panic_on_oom(CONSTRAINT_MEMCG, gfp_mask, 0, NULL);
> >  	limit = mem_cgroup_get_limit(mem) >> PAGE_SHIFT;
> >  	read_lock(&tasklist_lock);
> 
> The code duplication seems a bit gratuitous.
> 

I thought it was small enough to appear in two functions as opposed to 
doing something like

	static bool oom_current_has_sigkill(void)
	{
		if (fatal_signal_pending(current)) {
			set_thread_flag(TIF_MEMDIE);
			boost_dying_task_prio(current, NULL);
			return true;
		}
		return false;
	}

then doing

	if (oom_current_has_sigkill())
		return;

in mem_cgroup_out_of_memory() and out_of_memory().  If you'd prefer 
oom_current_has_sigkill(), let me know and I'll propose an alternate 
version.

> Was it deliberate that mem_cgroup_out_of_memory() ignores the oom
> notifier callbacks?
> 

Yes, the memory controller requires that something be killed (or, in the 
above case, simply allowing something to exit) to return under the hard 
limit; that's why we automatically kill current if nothing else eligible 
is found in select_bad_process().  Using oom notifier callbacks wouldn't 
guarantee there was freeing that would impact this memcg anyway.

> (Why does that notifier list exist at all?  Wouldn't it be better to do
> this via a vmscan shrinker?  Perhaps altered to be passed the scanning
> priority?)
> 

A vmscan shrinker seems more appropriate in the page allocator and not the 
oom killer before we call out_of_memory() and, as already mentioned, 
oom_notify_list doesn't do much at all (and is the wrong thing to do for 
cpusets or mempolicy ooms).  I've been reluctant to remove it because it 
doesn't have any impact for our systems but was obviously introduced for 
somebody's advantage in a rather unintrusive way. 

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>