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>
next prev parent 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).