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@xxxxxxxxxx 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@xxxxxxxxx";> email@xxxxxxxxx </a>