From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: mhocko@kernel.org, vdavydov@virtuozzo.com
Cc: akpm@linux-foundation.org, rientjes@google.com,
hannes@cmpxchg.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: oom_kill_process: do not abort if the victim is exiting
Date: Wed, 25 May 2016 00:05:09 +0900 [thread overview]
Message-ID: <201605250005.GHH26082.JOtQOSLMFFOFVH@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20160524135042.GK8259@dhcp22.suse.cz>
Michal Hocko wrote:
> On Tue 24-05-16 15:24:02, Vladimir Davydov wrote:
> > After selecting an oom victim, we first check if it's already exiting
> > and if it is, we don't bother killing tasks sharing its mm. We do try to
> > reap its mm though, but we abort if any of the processes sharing it is
> > still alive. This might result in oom deadlock if an exiting task got
> > stuck trying to acquire a lock held by another task sharing the same mm
> > which needs memory to continue: if oom killer happens to keep selecting
> > the stuck task, we won't even try to kill other processes or reap the
> > mm.
>
> I plan to extend task_will_free_mem to catch this case because we will
> need it for other changes.
Isn't mm_is_reapable() more useful than playing with fatal_signal_pending()
or task_will_free_mem()?
bool mm_is_reapable(struct mm_struct *mm)
{
struct task_struct *p;
if (!mm)
return false;
if (test_bit(MMF_OOM_REAPABLE, &mm->flags))
return true;
if (!down_read_trylock(&mm->mmap_sem))
return false;
up_read(&mm->mmap_sem);
/*
* There might be other threads/processes which are either not
* dying or even not killable.
*/
if (atomic_read(&mm->mm_users) > 1) {
rcu_read_lock();
for_each_process(p) {
bool exiting;
if (!process_shares_mm(p, mm))
continue;
if (fatal_signal_pending(p))
continue;
/*
* If the task is exiting make sure the whole thread group
* is exiting and cannot acces mm anymore.
*/
spin_lock_irq(&p->sighand->siglock);
exiting = signal_group_exit(p->signal);
spin_unlock_irq(&p->sighand->siglock);
if (exiting)
continue;
/* Give up */
rcu_read_unlock();
return false;
}
rcu_read_unlock();
}
set_bit(MMF_OOM_REAPABLE, &mm->flags);
return true;
}
/*
- * If the task is already exiting, don't alarm the sysadmin or kill
- * its children or threads, just set TIF_MEMDIE so it can die quickly
+ * If the victim's memory is already reapable, don't alarm the sysadmin
+ * or kill its children or threads, just set TIF_MEMDIE and let the
+ * OOM reaper reap the victim's memory.
*/
task_lock(p);
- if (p->mm && task_will_free_mem(p)) {
+ if (mm_is_reapable(p->mm)) {
mark_oom_victim(p);
- try_oom_reaper(p);
+ wake_oom_reaper(p);
task_unlock(p);
put_task_struct(p);
return;
}
task_unlock(p);
I suggest doing mm_is_reapable() test at __oom_reap_task() side as well
so that we can proceed to next victim by always calling wake_oom_reaper()
whenever TIF_MEMDIE is set.
- if (can_oom_reap)
- wake_oom_reaper(victim);
+ wake_oom_reaper(victim);
p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN is not a problem if that p is
already killed (not by the OOM killer) or exiting. We don't need to needlessly
make can_oom_reap false. mm_is_reapable() should do correct test.
next prev parent reply other threads:[~2016-05-24 15:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-24 12:24 [PATCH] mm: oom_kill_process: do not abort if the victim is exiting Vladimir Davydov
2016-05-24 13:50 ` Michal Hocko
2016-05-24 15:05 ` Tetsuo Handa [this message]
2016-05-24 17:07 ` Vladimir Davydov
2016-05-25 8:09 ` Michal Hocko
2016-05-25 15:20 ` Vladimir Davydov
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=201605250005.GHH26082.JOtQOSLMFFOFVH@I-love.SAKURA.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=rientjes@google.com \
--cc=vdavydov@virtuozzo.com \
/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).