linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@suse.cz>
To: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-mm@kvack.org, cgroups@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 2/2] memcg: do not sleep on OOM waitqueue with full charge context
Date: Thu, 13 Jun 2013 15:48:26 +0200	[thread overview]
Message-ID: <20130613134826.GE23070@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.02.1306121343500.24902@chino.kir.corp.google.com>

On Wed 12-06-13 13:49:47, David Rientjes wrote:
> On Wed, 12 Jun 2013, Michal Hocko wrote:
> 
> > The patch is a big improvement with a minimum code overhead. Blocking
> > any task which sits on top of an unpredictable amount of locks is just
> > broken. So regardless how many users are affected we should merge it and
> > backport to stable trees. The problem is there since ever. We seem to
> > be surprisingly lucky to not hit this more often.
> > 
> 
> Right now it appears that that number of users is 0 and we're talking 
> about a problem that was reported in 3.2 that was released a year and a 
> half ago.  The rules of inclusion in stable also prohibit such a change 
> from being backported, specifically "It must fix a real bug that bothers 
> people (not a, "This could be a problem..." type thing)".

As you can see there is an user seeing this in 3.2. The bug is _real_ and
I do not see what you are objecting against. Do you really think that
sitting on a time bomb is preferred more?

> We have deployed memcg on a very large number of machines and I can run a 
> query over all software watchdog timeouts that have occurred by 
> deadlocking on i_mutex during memcg oom.  It returns 0 results.

Do you capture /prc/<pid>/stack for each of them to find that your
deadlock (and you have reported that they happen) was in fact caused by
a locking issue? These kind of deadlocks might got unnoticed especially
when the oom is handled by userspace by increasing the limit (my mmecg
is stuck and increasing the limit a bit always helped).

> > I am not quite sure I understand your reservation about the patch to be
> > honest. Andrew still hasn't merged this one although 1/2 is in.
> 
> Perhaps he is as unconvinced?  The patch adds 100 lines of code, including 
> fields to task_struct for memcg, for a problem that nobody can reproduce.  
> My question still stands: can anybody, even with an instrumented kernel to 
> make it more probable, reproduce the issue this is addressing?

So the referenced discussion is not sufficient?

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

WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@suse.cz>
To: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-mm@kvack.org, cgroups@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 2/2] memcg: do not sleep on OOM waitqueue with full charge context
Date: Thu, 13 Jun 2013 15:48:26 +0200	[thread overview]
Message-ID: <20130613134826.GE23070@dhcp22.suse.cz> (raw)
Message-ID: <20130613134826.uVaDvTK2NPr2ccoPOxZe8bKANG1DqvCo59xf2i05kGU@z> (raw)
In-Reply-To: <alpine.DEB.2.02.1306121343500.24902@chino.kir.corp.google.com>

On Wed 12-06-13 13:49:47, David Rientjes wrote:
> On Wed, 12 Jun 2013, Michal Hocko wrote:
> 
> > The patch is a big improvement with a minimum code overhead. Blocking
> > any task which sits on top of an unpredictable amount of locks is just
> > broken. So regardless how many users are affected we should merge it and
> > backport to stable trees. The problem is there since ever. We seem to
> > be surprisingly lucky to not hit this more often.
> > 
> 
> Right now it appears that that number of users is 0 and we're talking 
> about a problem that was reported in 3.2 that was released a year and a 
> half ago.  The rules of inclusion in stable also prohibit such a change 
> from being backported, specifically "It must fix a real bug that bothers 
> people (not a, "This could be a problem..." type thing)".

As you can see there is an user seeing this in 3.2. The bug is _real_ and
I do not see what you are objecting against. Do you really think that
sitting on a time bomb is preferred more?

> We have deployed memcg on a very large number of machines and I can run a 
> query over all software watchdog timeouts that have occurred by 
> deadlocking on i_mutex during memcg oom.  It returns 0 results.

Do you capture /prc/<pid>/stack for each of them to find that your
deadlock (and you have reported that they happen) was in fact caused by
a locking issue? These kind of deadlocks might got unnoticed especially
when the oom is handled by userspace by increasing the limit (my mmecg
is stuck and increasing the limit a bit always helped).

> > I am not quite sure I understand your reservation about the patch to be
> > honest. Andrew still hasn't merged this one although 1/2 is in.
> 
> Perhaps he is as unconvinced?  The patch adds 100 lines of code, including 
> fields to task_struct for memcg, for a problem that nobody can reproduce.  
> My question still stands: can anybody, even with an instrumented kernel to 
> make it more probable, reproduce the issue this is addressing?

So the referenced discussion is not sufficient?

-- 
Michal Hocko
SUSE Labs

  parent reply	other threads:[~2013-06-13 13:48 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-06  3:09 [patch 1/2] arch: invoke oom-killer from page fault Johannes Weiner
2013-06-06  3:09 ` Johannes Weiner
2013-06-06  3:09 ` [patch 2/2] memcg: do not sleep on OOM waitqueue with full charge context Johannes Weiner
2013-06-06  3:09   ` Johannes Weiner
2013-06-06  4:10   ` David Rientjes
2013-06-06  4:10     ` David Rientjes
2013-06-06  5:33     ` Johannes Weiner
2013-06-06  5:33       ` Johannes Weiner
2013-06-06 17:33       ` Johannes Weiner
2013-06-06 17:33         ` Johannes Weiner
2013-06-06 20:11         ` David Rientjes
2013-06-06 20:11           ` David Rientjes
2013-06-06 21:54           ` Johannes Weiner
2013-06-06 21:54             ` Johannes Weiner
2013-06-06 22:18             ` David Rientjes
2013-06-06 22:18               ` David Rientjes
2013-06-07  0:02               ` Johannes Weiner
2013-06-07  0:02                 ` Johannes Weiner
2013-06-11 21:57                 ` David Rientjes
2013-06-11 21:57                   ` David Rientjes
2013-06-12  8:28                   ` Michal Hocko
2013-06-12  8:28                     ` Michal Hocko
     [not found]                     ` <20130612082817.GA6706-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-06-12 20:12                       ` David Rientjes
2013-06-12 20:12                         ` David Rientjes
2013-06-12 20:37                         ` Michal Hocko
2013-06-12 20:37                           ` Michal Hocko
2013-06-12 20:49                           ` David Rientjes
2013-06-12 20:49                             ` David Rientjes
2013-06-13 13:48                             ` Michal Hocko [this message]
2013-06-13 13:48                               ` Michal Hocko
2013-06-13 20:34                               ` David Rientjes
2013-06-13 20:34                                 ` David Rientjes
2013-06-14  9:29                                 ` Michal Hocko
2013-06-14  9:29                                   ` Michal Hocko
2013-06-06 15:23     ` Michal Hocko
2013-06-06 15:23       ` Michal Hocko
2013-06-06  3:57 ` [patch 1/2] arch: invoke oom-killer from page fault David Rientjes
2013-06-06  3:57   ` David Rientjes
2013-06-06  4:36   ` Johannes Weiner
2013-06-06  4:36     ` Johannes Weiner
2013-06-06  4:43     ` David Rientjes
2013-06-06  6:49     ` Vineet Gupta
2013-06-06  6:49       ` Vineet Gupta
     [not found]     ` <20130606043620.GA9406-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2013-06-06 14:59       ` Michal Hocko
2013-06-06 14:59         ` Michal Hocko
2013-06-06  4:55 ` 刘胜蛟

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=20130613134826.GE23070@dhcp22.suse.cz \
    --to=mhocko@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.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).