linux-mm.kvack.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-kernel@vger.kernel.org, linux-mm@kvack.org,
	cgroups@vger.kernel.org
Subject: Re: [patch 1/2] mm, memcg: avoid oom notification when current needs access to memory reserves
Date: Wed, 18 Dec 2013 21:04:34 +0100	[thread overview]
Message-ID: <20131218200434.GA4161@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.02.1312171240541.21640@chino.kir.corp.google.com>

On Tue 17-12-13 12:50:09, David Rientjes wrote:
> On Tue, 17 Dec 2013, Michal Hocko wrote:
> 
> > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > > index c72b03bf9679..fee25c5934d2 100644
> > > > --- a/mm/memcontrol.c
> > > > +++ b/mm/memcontrol.c
> > > > @@ -2692,7 +2693,8 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
> > > >  	 * MEMDIE process.
> > > >  	 */
> > > >  	if (unlikely(test_thread_flag(TIF_MEMDIE)
> > > > -		     || fatal_signal_pending(current)))
> > > > +		     || fatal_signal_pending(current))
> > > > +		     || current->flags & PF_EXITING)
> > > >  		goto bypass;
> > > >  
> > > >  	if (unlikely(task_in_memcg_oom(current)))
> > > > 
> > > > rather than the later checks down the oom_synchronize paths. The comment
> > > > already mentions dying process...
> > > > 
> > > 
> > > This is scary because it doesn't even try to reclaim memcg memory before 
> > > allowing the allocation to succeed.
> > 
> > Why should it reclaim in the first place when it simply is on the way to
> > release memory. In other words why should it increase the memory
> > pressure when it is in fact releasing it?
> > 
> 
> (Answering about removing the fatal_signal_pending() check as well here.)
> 
> For memory isolation, we'd only want to bypass memcg charges when 
> absolutely necessary and it seems like TIF_MEMDIE is the only case where 
> that's required.  We don't give processes with pending SIGKILLs or those 
> in the exit() path access to memory reserves in the page allocator without 
> first determining that reclaim can't make any progress for the same reason 
> and then we only do so by setting TIF_MEMDIE when calling the oom killer.  

While I do understand arguments about isolation I would also like to be
practical here. How many charges are we talking about? Dozen pages? Much
more?
Besides that all of those should be very short lived because the task
is going to die very soon and so the memory will be freed.

So from my POV I would like to see these heuristics as simple as
possible and placed at very few places. Doing a bypass before charge
- or even after a failed charge before doing reclaim sounds like an easy
enough heuristic without a big risk.
I have really hard time to see big benefits for forcing reclaim for a
very short lived charge because this might lead to different and much
worse side effects then a quantum noise.

Maybe I am missing something and we can charge a lot during exit but
then I think we should fix the exit path to not allocate that much.

> > I am really puzzled here. On one hand you are strongly arguing for not
> > notifying when we know we can prevent from OOM action and on the other
> > hand you are ok to get vmpressure/thresholds notification when an
> > exiting task triggers reclaim.
> > 
> > So I am really lost in what you are trying to achieve here. It sounds a
> > bit arbirtrary.
> > 
> 
> It's not arbitrary to define when memcg bypass is allowed and, in my 
> opinion, it should only be done in situations where it is unavoidable and 
> therefore breaking memory isolation is required.
> 
> (We wouldn't expect a 128MB memcg to be oom [and perhaps with a userspace 
> oom handler attached] when it has 100 children each 1MB in size just 
> because they all happen to be oom at the same time.  We set up the excess 

s/oom/exiting/ ?

> memory in the parent specifically for the memcg with the oom handler 
> attached.)

I am not sure I understand what you meant here.

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

  reply	other threads:[~2013-12-18 20:04 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-31  1:39 [patch] mm, memcg: add memory.oom_control notification for system oom David Rientjes
2013-10-31  5:49 ` Johannes Weiner
2013-11-13 22:19   ` David Rientjes
2013-11-13 23:34     ` Johannes Weiner
2013-11-14  0:56       ` David Rientjes
2013-11-14  3:25         ` Johannes Weiner
2013-11-14 22:57           ` David Rientjes
2013-11-14 23:26             ` [patch 1/2] mm, memcg: avoid oom notification when current needs access to memory reserves David Rientjes
2013-11-14 23:26               ` [patch 2/2] mm, memcg: add memory.oom_control notification for system oom David Rientjes
2013-11-18 18:52                 ` Michal Hocko
2013-11-19  1:25                   ` David Rientjes
2013-11-19 12:41                     ` Michal Hocko
2013-11-18 12:52               ` [patch 1/2] mm, memcg: avoid oom notification when current needs access to memory reserves Michal Hocko
2013-11-18 12:55                 ` Michal Hocko
2013-11-19  1:19                   ` David Rientjes
2013-11-18 15:41               ` Johannes Weiner
2013-11-18 16:51                 ` Michal Hocko
2013-11-19  1:22                   ` David Rientjes
2013-11-22 16:51                   ` Johannes Weiner
2013-11-27  0:53                     ` David Rientjes
2013-11-27 16:34                       ` Johannes Weiner
2013-11-27 21:51                         ` David Rientjes
2013-11-27 23:19                           ` Johannes Weiner
2013-11-28  0:22                             ` David Rientjes
2013-11-28  2:28                               ` Johannes Weiner
2013-11-28  2:52                                 ` David Rientjes
2013-11-28  3:16                                   ` Johannes Weiner
2013-12-02 20:02                         ` Michal Hocko
2013-12-02 21:25                           ` Johannes Weiner
2013-12-03 12:04                             ` Michal Hocko
2013-12-03 20:17                               ` Johannes Weiner
2013-12-03 21:00                                 ` Michal Hocko
2013-12-03 21:23                                   ` Johannes Weiner
2013-12-03 23:50                               ` David Rientjes
2013-12-04  3:34                                 ` Johannes Weiner
2013-12-04 11:13                                 ` Michal Hocko
2013-12-05  0:23                                   ` David Rientjes
2013-12-09 12:48                                     ` Michal Hocko
2013-12-09 21:46                                       ` David Rientjes
2013-12-09 22:51                                         ` Johannes Weiner
2013-12-09 23:05                                         ` Johannes Weiner
2014-01-10  0:34                                           ` David Rientjes
2013-12-10 10:38                                         ` Michal Hocko
2013-12-11  1:03                                           ` David Rientjes
2013-12-11  9:55                                             ` Michal Hocko
2013-12-11 22:40                                               ` David Rientjes
2013-12-12 10:31                                                 ` Michal Hocko
2013-12-12 10:50                                                   ` Michal Hocko
2013-12-12 12:11                                                   ` Michal Hocko
2013-12-12 12:37                                                     ` Michal Hocko
2013-12-13 23:55                                                   ` David Rientjes
2013-12-17 16:23                                                     ` Michal Hocko
2013-12-17 20:50                                                       ` David Rientjes
2013-12-18 20:04                                                         ` Michal Hocko [this message]
2013-12-19  6:09                                                           ` David Rientjes
2013-12-19 14:41                                                             ` Michal Hocko
2014-01-08  0:25                                                               ` Andrew Morton
2014-01-08 10:33                                                                 ` Michal Hocko
2014-01-09 14:30                                                                   ` [PATCH] memcg: Do not hang on OOM when killed by userspace OOM " Michal Hocko
2014-01-09 21:40                                                                     ` David Rientjes
2014-01-10  8:23                                                                       ` Michal Hocko
2014-01-10 21:33                                                                         ` David Rientjes
2014-01-15 14:26                                                                           ` Michal Hocko
2014-01-15 21:19                                                                             ` David Rientjes
2014-01-16 10:12                                                                               ` Michal Hocko
2014-01-21  6:13                                                                                 ` David Rientjes
2014-01-21 13:21                                                                                   ` Michal Hocko
2014-01-09 21:34                                                                 ` [patch 1/2] mm, memcg: avoid oom notification when current needs " David Rientjes
2014-01-09 22:47                                                                   ` Andrew Morton
2014-01-10  0:01                                                                     ` David Rientjes
2014-01-10  0:12                                                                       ` Andrew Morton
2014-01-10  0:23                                                                         ` David Rientjes
2014-01-10  0:35                                                                           ` David Rientjes
2014-01-10 22:14                                                                           ` Johannes Weiner
2014-01-12 22:10                                                                             ` David Rientjes
2014-01-15 14:34                                                                               ` Michal Hocko
2014-01-15 21:23                                                                                 ` David Rientjes
2014-01-16  9:32                                                                                   ` Michal Hocko
2014-01-21  5:58                                                                                     ` David Rientjes
2014-01-21  6:04                                                                                       ` Greg Kroah-Hartmann
2014-01-21  6:08                                                                                         ` David Rientjes
2014-01-10  8:30                                                                       ` Michal Hocko
2014-01-10 21:38                                                                         ` David Rientjes
2014-01-10 22:34                                                                           ` Johannes Weiner
2014-01-12 22:14                                                                             ` David Rientjes
2013-11-18 15:54             ` [patch] mm, memcg: add memory.oom_control notification for system oom Johannes Weiner
2013-11-18 23:15               ` One Thousand Gnomes

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=20131218200434.GA4161@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-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).