From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Michal Hocko <mhocko@suse.cz>
Cc: David Rientjes <rientjes@google.com>,
Oleg Nesterov <oleg@redhat.com>,
Konstantin Khlebnikov <khlebnikov@openvz.org>,
linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
Tejun Heo <tj@kernel.org>, Rusty Russell <rusty@rustcorp.com.au>
Subject: Re: [PATCH 1/2] oom: do not live lock on frozen tasks
Date: Mon, 26 Sep 2011 17:51:40 +0200 [thread overview]
Message-ID: <201109261751.40688.rjw@sisk.pl> (raw)
In-Reply-To: <20110926091440.GE10156@tiehlicka.suse.cz>
On Monday, September 26, 2011, Michal Hocko wrote:
> On Mon 26-09-11 01:56:57, David Rientjes wrote:
> > On Mon, 26 Sep 2011, Michal Hocko wrote:
> >
> > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > index 626303b..b9774f3 100644
> > > --- a/mm/oom_kill.c
> > > +++ b/mm/oom_kill.c
> > > @@ -32,6 +32,7 @@
> > > #include <linux/mempolicy.h>
> > > #include <linux/security.h>
> > > #include <linux/ptrace.h>
> > > +#include <linux/freezer.h>
> > >
> > > int sysctl_panic_on_oom;
> > > int sysctl_oom_kill_allocating_task;
> > > @@ -451,6 +452,9 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
> > > task_pid_nr(q), q->comm);
> > > task_unlock(q);
> > > force_sig(SIGKILL, q);
> > > +
> > > + if (frozen(q))
> > > + thaw_process(q);
> > > }
> > >
> > > set_tsk_thread_flag(p, TIF_MEMDIE);
> >
> > This is in the wrong place, oom_kill_task() iterates over all threads that
> > are _not_ in the same thread group as the chosen thread and kills them
> > without giving them access to memory reserves. The chosen task, p, could
> > still be frozen and may not exit.
>
> Ahh, right you are. I ave missed that one. Updated patch bellow.
>
> >
> > Once that's fixed, feel free to add my
> >
> > Acked-by: David Rientjes <rientjes@google.com>
>
> Thanks
>
> >
> > once Rafael sends his acked-by or reviewed-by.
> ---
> From f935ed4558c2fb033ef5c14e02b28e12a615f80e Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.cz>
> Date: Fri, 16 Sep 2011 11:23:15 +0200
> Subject: [PATCH] oom: do not live lock on frozen tasks
>
> Konstantin Khlebnikov has reported (https://lkml.org/lkml/2011/8/23/45)
> that OOM can end up in a live lock if select_bad_process picks up a frozen
> task.
> Unfortunately we cannot mark such processes as unkillable to ignore them
> because we could panic the system even though there is a chance that
> somebody could thaw the process so we can make a forward process (e.g. a
> process from another cpuset or with a different nodemask).
>
> Let's thaw an OOM selected frozen process right after we've sent fatal
> signal from oom_kill_task.
> Thawing is safe if the frozen task doesn't access any suspended device
> (e.g. by ioctl) on the way out to the userspace where we handle the
> signal and die. Note, we are not interested in the kernel threads because
> they are not oom killable.
>
> Accessing suspended devices by a userspace processes shouldn't be an
> issue because devices are suspended only after userspace is already
> frozen and oom is disabled at that time.
>
> run_guest (drivers/lguest/core.c) calls try_to_freeze with an user
> context but it seems it is able to cope with signals because it
> explicitly checks for pending signals so we should be safe.
>
> Other than that userspace accesses the fridge only from the
> signal handling routines so we are able to handle SIGKILL without any
> negative side effects.
>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> Reported-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> mm/oom_kill.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 626303b..c419a7e 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -32,6 +32,7 @@
> #include <linux/mempolicy.h>
> #include <linux/security.h>
> #include <linux/ptrace.h>
> +#include <linux/freezer.h>
>
> int sysctl_panic_on_oom;
> int sysctl_oom_kill_allocating_task;
> @@ -451,10 +452,15 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
> task_pid_nr(q), q->comm);
> task_unlock(q);
> force_sig(SIGKILL, q);
> +
> + if (frozen(q))
> + thaw_process(q);
> }
>
> set_tsk_thread_flag(p, TIF_MEMDIE);
> force_sig(SIGKILL, p);
> + if (frozen(p))
> + thaw_process(p);
>
> return 0;
> }
>
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-09-26 15:49 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-23 8:31 [PATCH] oom: skip frozen tasks Konstantin Khlebnikov
2011-08-23 9:15 ` KAMEZAWA Hiroyuki
2011-08-23 13:46 ` Michal Hocko
2011-08-23 20:18 ` David Rientjes
2011-08-24 10:19 ` Michal Hocko
2011-08-24 19:31 ` David Rientjes
2011-08-25 9:19 ` Michal Hocko
2011-08-25 15:18 ` Oleg Nesterov
2011-08-25 16:47 ` Michal Hocko
2011-08-25 21:14 ` David Rientjes
2011-08-26 7:09 ` Michal Hocko
2011-08-26 8:56 ` Michal Hocko
2011-08-26 9:21 ` David Rientjes
2011-08-26 9:53 ` Michal Hocko
2011-08-26 11:01 ` Michal Hocko
2011-08-26 18:13 ` David Rientjes
2011-09-26 8:28 ` [PATCH 1/2] oom: do not live lock on " Michal Hocko
2011-09-26 8:56 ` David Rientjes
2011-09-26 9:14 ` Michal Hocko
2011-09-26 9:25 ` KAMEZAWA Hiroyuki
2011-09-26 9:32 ` Michal Hocko
2011-09-26 15:51 ` Rafael J. Wysocki [this message]
2011-09-26 18:28 ` Michal Hocko
2011-09-27 1:03 ` David Rientjes
2011-09-27 7:52 ` Michal Hocko
2011-09-27 18:30 ` David Rientjes
2011-09-26 10:28 ` Rusty Russell
2011-09-26 11:05 ` Michal Hocko
2011-09-27 2:21 ` Rusty Russell
2011-09-27 7:03 ` [PATCH] lguest: move process freezing before pending signals check Michal Hocko
2011-09-26 8:35 ` [PATCH 2/2] oom: give bonus to frozen processes Michal Hocko
2011-09-26 9:02 ` David Rientjes
2011-09-26 9:31 ` KAMEZAWA Hiroyuki
2011-09-26 9:54 ` Michal Hocko
2011-08-26 21:03 ` [PATCH] oom: skip frozen tasks Rafael J. Wysocki
2011-08-26 10:03 ` Konstantin Khlebnikov
2011-08-26 10:48 ` Michal Hocko
2011-08-26 12:44 ` Konstantin Khlebnikov
2011-08-26 12:59 ` Michal Hocko
2011-08-26 7:35 ` Konstantin Khlebnikov
2011-08-26 9:09 ` David Rientjes
2011-08-26 9:59 ` Konstantin Khlebnikov
2011-08-26 18:09 ` David Rientjes
2011-08-25 21:03 ` Rafael J. Wysocki
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=201109261751.40688.rjw@sisk.pl \
--to=rjw@sisk.pl \
--cc=akpm@linux-foundation.org \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=khlebnikov@openvz.org \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.cz \
--cc=oleg@redhat.com \
--cc=rientjes@google.com \
--cc=rusty@rustcorp.com.au \
--cc=tj@kernel.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).