linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: rientjes@google.com, akpm@linux-foundation.org, mgorman@suse.de,
	torvalds@linux-foundation.org, oleg@redhat.com, hughd@google.com,
	andrea@kernel.org, riel@redhat.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm,oom: Exclude TIF_MEMDIE processes from candidates.
Date: Tue, 12 Jan 2016 20:52:00 +0100	[thread overview]
Message-ID: <20160112195200.GB4515@dhcp22.suse.cz> (raw)
In-Reply-To: <201601122032.FHH13586.MOQVFFOJStFHOL@I-love.SAKURA.ne.jp>

On Tue 12-01-16 20:32:18, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Fri 08-01-16 00:38:43, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > @@ -333,6 +333,14 @@ static struct task_struct *select_bad_process(struct oom_control *oc,
> > > >  		if (points == chosen_points && thread_group_leader(chosen))
> > > >  			continue;
> > > >  
> > > > +		/*
> > > > +		 * If the current major task is already ooom killed and this
> > > > +		 * is sysrq+f request then we rather choose somebody else
> > > > +		 * because the current oom victim might be stuck.
> > > > +		 */
> > > > +		if (is_sysrq_oom(sc) && test_tsk_thread_flag(p, TIF_MEMDIE))
> > > > +			continue;
> > > > +
> > > >  		chosen = p;
> > > >  		chosen_points = points;
> > > >  	}
> > > 
> > > Do we want to require SysRq-f for each thread in a process?
> > > If g has 1024 p, dump_tasks() will do
> > > 
> > >   pr_info("[%5d] %5d %5d %8lu %8lu %7ld %7ld %8lu         %5hd %s\n",
> > > 
> > > for 1024 times? I think one SysRq-f per one process is sufficient.
> > 
> > I am not following you here. If we kill the process the whole process
> > group (aka all threads) will get killed which ever thread we happen to
> > send the sigkill to.
> 
> Please distinguish "sending SIGKILL to a process" and "all threads in that
> process terminate".

I didn't say anything about termination if your read my response again.

[...]

> > > How can we guarantee that find_lock_task_mm() from oom_kill_process()
> > > chooses !TIF_MEMDIE thread when try_to_sacrifice_child() somehow chose
> > > !TIF_MEMDIE thread? I think choosing !TIF_MEMDIE thread at
> > > find_lock_task_mm() is the simplest way.
> > 
> > find_lock_task_mm chosing TIF_MEMDIE thread shouldn't change anything
> > because the whole thread group will go down anyway. If you want to
> > guarantee that the sysrq+f never choses a task which has a TIF_MEMDIE
> > thread then we would have to check for fatal_signal_pending as well
> > AFAIU. Fiddling with find find_lock_task_mm will not help you though
> > unless I am missing something.
> 
> I do want to guarantee that the SysRq-f (and timeout based next victim
> selection) never chooses a process which has a TIF_MEMDIE thread.

Sigh... see what I have written in the paragraph you are replying to...

> I don't like current "oom: clear TIF_MEMDIE after oom_reaper managed to unmap
> the address space" patch unless both "mm,oom: exclude TIF_MEMDIE processes from
> candidates." patch and "mm,oom: Re-enable OOM killer using timers."

Those patches are definitely not a prerequisite from the functional
point of view and putting them together as a prerequisite sounds like
blocking a useful feature without technical grounds to me.

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

  reply	other threads:[~2016-01-12 19:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-29 13:58 [PATCH] mm,oom: Exclude TIF_MEMDIE processes from candidates Tetsuo Handa
2016-01-07  9:15 ` Michal Hocko
2016-01-07 13:31   ` Tetsuo Handa
2016-01-07 14:58     ` Michal Hocko
2016-01-07 15:38       ` Tetsuo Handa
2016-01-11 15:18         ` Michal Hocko
2016-01-12 11:32           ` Tetsuo Handa
2016-01-12 19:52             ` Michal Hocko [this message]
2016-01-13 10:15               ` Tetsuo Handa
2016-01-13 15:24                 ` Michal Hocko
2016-01-07 15:44       ` Michal Hocko
2016-01-08 10:09         ` [PATCH v2] " Tetsuo Handa
2016-01-13  0:32           ` David Rientjes
2016-01-13 10:52             ` Tetsuo Handa
2016-01-14  0:57               ` David Rientjes
2016-01-14 10:26                 ` Tetsuo Handa
2016-01-14 21:53                   ` David Rientjes
2016-01-14 22:21                     ` Tetsuo Handa
2016-01-07 16:28 ` [PATCH] " Johannes Weiner
2016-01-08 12:37   ` Michal Hocko
2016-01-08 13:14     ` Tetsuo Handa
2016-01-08 13:41       ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160112195200.GB4515@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@kernel.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=oleg@redhat.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).