All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>, William Dauchy <wdauchy@gmail.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	"Ma, Xindong" <xindong.ma@intel.com>,
	"rusty@rustcorp.com.au" <rusty@rustcorp.com.au>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	gregkh@linuxfoundation.org, "Tu,
	Xiaobing" <xiaobing.tu@intel.com>, azurIt <azurit@pobox.sk>,
	Sameer Nanda <snanda@chromium.org>
Subject: Re: [PATCH] Fix race between oom kill and task exit
Date: Fri, 6 Dec 2013 16:19:44 +0100	[thread overview]
Message-ID: <20131206151944.GC2674@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1312051531330.7717@chino.kir.corp.google.com>

On 12/05, David Rientjes wrote:
>
> On Thu, 5 Dec 2013, Oleg Nesterov wrote:
>
> > > Your v2 series looks good and I suspect anybody trying them doesn't have
> > > additional reports of the infinite loop?  Should they be marked for
> > > stable?
> >
> > Unlikely...
> >
> > I think the patch from Sameer makes more sense for stable as a temporary
> > (and obviously incomplete) fix.
>
> There's a problem because none of this is currently even in linux-next.  I
> think we could make a case for getting Sameer's patch at
> http://marc.info/?l=linux-kernel&m=138436313021133 to be merged for
> stable,

Probably.

Ah, I just noticed that this change

	-	if (p->flags & PF_EXITING) {
	+	if (p->flags & PF_EXITING || !pid_alive(p)) {

is not needed. !pid_alive(p) obviously implies PF_EXITING.

> but then we'd have to revert it in linux-next

Or perhaps Sameer can just send his fix to stable/gregkh.

Just the changelog should clearly explain that this is the minimal
workaround for stable. Once again it doesn't (and can't) fix all
problems even in oom_kill_process() paths, but it helps anyway to
avoid the easy-to-trigger hang.

> before merging your
> series at http://marc.info/?l=linux-kernel&m=138616217925981.

Just in case, I won't mind to rediff my patches on top of Sameer's
patch and then add git-revert patch.

> All of the
> issues you present in that series seem to be stable material, so why not
> just go ahead with your series and mark it for stable for 3.13?

OK... I can do this too.

I do not really like this because it adds thread_head/node but doesn't
remove the old ->thread_group. We will do this later, but obviously
this is not the stable material.

IOW, if we send this to stable, thread_head/node/for_each_thread will
be only used by oom_kill.c.

And this is risky. For example, 1/4 depends on (at least) another patch
I sent in preparation for this change, commit 81907739851
"kernel/fork.c:copy_process(): don't add the uninitialized
child to thread/task/pid lists", perhaps on something else.

So personally I'd prefer to simply send the workaround for stable.

Oleg.

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

WARNING: multiple messages have this Message-ID (diff)
From: Oleg Nesterov <oleg@redhat.com>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>, William Dauchy <wdauchy@gmail.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	"Ma, Xindong" <xindong.ma@intel.com>,
	"rusty@rustcorp.com.au" <rusty@rustcorp.com.au>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	gregkh@linuxfoundation.org, "Tu,
	Xiaobing" <xiaobing.tu@intel.com>, azurIt <azurit@pobox.sk>,
	Sameer Nanda <snanda@chromium.org>
Subject: Re: [PATCH] Fix race between oom kill and task exit
Date: Fri, 6 Dec 2013 16:19:44 +0100	[thread overview]
Message-ID: <20131206151944.GC2674@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1312051531330.7717@chino.kir.corp.google.com>

On 12/05, David Rientjes wrote:
>
> On Thu, 5 Dec 2013, Oleg Nesterov wrote:
>
> > > Your v2 series looks good and I suspect anybody trying them doesn't have
> > > additional reports of the infinite loop?  Should they be marked for
> > > stable?
> >
> > Unlikely...
> >
> > I think the patch from Sameer makes more sense for stable as a temporary
> > (and obviously incomplete) fix.
>
> There's a problem because none of this is currently even in linux-next.  I
> think we could make a case for getting Sameer's patch at
> http://marc.info/?l=linux-kernel&m=138436313021133 to be merged for
> stable,

Probably.

Ah, I just noticed that this change

	-	if (p->flags & PF_EXITING) {
	+	if (p->flags & PF_EXITING || !pid_alive(p)) {

is not needed. !pid_alive(p) obviously implies PF_EXITING.

> but then we'd have to revert it in linux-next

Or perhaps Sameer can just send his fix to stable/gregkh.

Just the changelog should clearly explain that this is the minimal
workaround for stable. Once again it doesn't (and can't) fix all
problems even in oom_kill_process() paths, but it helps anyway to
avoid the easy-to-trigger hang.

> before merging your
> series at http://marc.info/?l=linux-kernel&m=138616217925981.

Just in case, I won't mind to rediff my patches on top of Sameer's
patch and then add git-revert patch.

> All of the
> issues you present in that series seem to be stable material, so why not
> just go ahead with your series and mark it for stable for 3.13?

OK... I can do this too.

I do not really like this because it adds thread_head/node but doesn't
remove the old ->thread_group. We will do this later, but obviously
this is not the stable material.

IOW, if we send this to stable, thread_head/node/for_each_thread will
be only used by oom_kill.c.

And this is risky. For example, 1/4 depends on (at least) another patch
I sent in preparation for this change, commit 81907739851
"kernel/fork.c:copy_process(): don't add the uninitialized
child to thread/task/pid lists", perhaps on something else.

So personally I'd prefer to simply send the workaround for stable.

Oleg.


  reply	other threads:[~2013-12-06 15:19 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-28  5:09 [PATCH] Fix race between oom kill and task exit Ma, Xindong
2013-11-28  5:09 ` Ma, Xindong
2013-11-28  6:35 ` Johannes Weiner
2013-11-28  6:35   ` Johannes Weiner
2013-11-28  8:41   ` William Dauchy
2013-11-28  8:41     ` William Dauchy
2013-11-28 12:00     ` Michal Hocko
2013-11-28 12:00       ` Michal Hocko
2013-11-28 17:57       ` Vladimir Murzin
2013-11-28 17:57         ` Vladimir Murzin
2013-11-28 18:38       ` Oleg Nesterov
2013-11-28 18:38         ` Oleg Nesterov
2013-11-29  2:06         ` Ma, Xindong
2013-11-29  2:06           ` Ma, Xindong
2013-11-29  2:08           ` Tu, Xiaobing
2013-11-29  2:08             ` Tu, Xiaobing
2013-12-02 14:12         ` Oleg Nesterov
2013-12-02 14:12           ` Oleg Nesterov
2013-12-05  0:56           ` David Rientjes
2013-12-05  0:56             ` David Rientjes
2013-12-05 17:29             ` Oleg Nesterov
2013-12-05 17:29               ` Oleg Nesterov
2013-12-05 23:35               ` David Rientjes
2013-12-05 23:35                 ` David Rientjes
2013-12-06 15:19                 ` Oleg Nesterov [this message]
2013-12-06 15:19                   ` Oleg Nesterov
2013-12-06 15:52                   ` Oleg Nesterov
2013-12-06 15:52                     ` Oleg Nesterov
2013-12-06 17:54                   ` Sameer Nanda
2013-12-06 17:54                     ` Sameer Nanda
2013-11-28  9:18   ` azurIt
2013-11-28  9:18     ` azurIt
2013-11-28 18:37 ` Oleg Nesterov
2013-11-28 18:37   ` Oleg Nesterov

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=20131206151944.GC2674@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=azurit@pobox.sk \
    --cc=gregkh@linuxfoundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=rusty@rustcorp.com.au \
    --cc=snanda@chromium.org \
    --cc=wdauchy@gmail.com \
    --cc=xiaobing.tu@intel.com \
    --cc=xindong.ma@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.