Re: [PATCH] mm/oom: Suppress unnecessary "sharing same memory" message.

[Michal Hocko <mhocko@suse.cz>]

On Wed 27-05-15 06:39:42, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Mon 25-05-15 23:33:31, Tetsuo Handa wrote:
> > > >From 3728807fe66ebc24a8a28455593754b9532bbe74 Mon Sep 17 00:00:00 2001
> > > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > > Date: Mon, 25 May 2015 22:26:07 +0900
> > > Subject: [PATCH] mm/oom: Suppress unnecessary "sharing same memory" message.
> > > 
> > > If the mm struct which the OOM victim is using is shared by e.g. 1000
> > > threads, and the lock dependency prevents all threads except the OOM
> > > victim thread from terminating until they get TIF_MEMDIE flag, the OOM
> > > killer will be invoked for 1000 times on this mm struct. As a result,
> > > the kernel would emit
> > > 
> > >   "Kill process %d (%s) sharing same memory\n"
> > > 
> > > line for 1000 * 1000 / 2 times. But once these threads got pending SIGKILL,
> > > emitting this information is nothing but noise. This patch filters them.
> > 
> > OK, I can see this might be really annoying. But reducing this message
> > will not help much because it is the dump_header which generates a lot
> > of output. And there is clearly no reason to treat the selected victim
> > any differently than the current so why not simply do the following
> > instead?
> > ---
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 5cfda39b3268..a67ce18b4b35 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -505,7 +505,7 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
> >  	 * its children or threads, just set TIF_MEMDIE so it can die quickly
> >  	 */
> >  	task_lock(p);
> > -	if (p->mm && task_will_free_mem(p)) {
> > +	if (p->mm && (fatal_signal_pending(p) || task_will_free_mem(p))) {
> >  		mark_oom_victim(p);
> >  		task_unlock(p);
> >  		put_task_struct(p);
> > 
> 
> I don't think this is good, for this will omit sending SIGKILL to threads
> sharing p->mm ("Kill all user processes sharing victim->mm in other thread
> groups, if any.")

threads? The whole thread group will die when the fatal signal is
send to the group leader no? This mm sharing handling is about
processes which are sharing mm but they are not in the same thread group
(aka CLONE_VM without CLONE_SIGHAND resp. CLONE_THREAD).

> when p already has pending SIGKILL.

yes we can select a task which has SIGKILL already pending and then
we wouldn't kill other processes which share the same mm but does it
matter?  I do not think so. Because if this is really the case and the
OOM condition continues even after p exits (which is very probable but
p alone might release some resources and free memory) we will find a
process with the same mm in the next round.

> By the way, if p with p->mm && task_will_free_mem(p) can get stuck due to
> memory allocation deadlock, is it OK that currently we are not sending SIGKILL
> to threads sharing p->mm ?

I am not sure I understand the question. Threads will die automatically
because we are sending group signal.
-- 
Michal Hocko
SUSE Labs

--
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>