All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: akpm@linux-foundation.org, rientjes@google.com,
	hannes@cmpxchg.org, guro@fb.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] mm, oom: do not rely on TIF_MEMDIE for memory reserves access
Date: Thu, 3 Aug 2017 09:06:06 +0200	[thread overview]
Message-ID: <20170803070606.GA12521@dhcp22.suse.cz> (raw)
In-Reply-To: <201708031039.GDG05288.OQJOHtLVFMSFFO@I-love.SAKURA.ne.jp>

On Thu 03-08-17 10:39:42, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Wed 02-08-17 00:30:33, Tetsuo Handa wrote:
> > > > @@ -3603,6 +3612,22 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
> > > >  	return alloc_flags;
> > > >  }
> > > >  
> > > > +static bool oom_reserves_allowed(struct task_struct *tsk)
> > > > +{
> > > > +	if (!tsk_is_oom_victim(tsk))
> > > > +		return false;
> > > > +
> > > > +	/*
> > > > +	 * !MMU doesn't have oom reaper so we shouldn't risk the memory reserves
> > > > +	 * depletion and shouldn't give access to memory reserves passed the
> > > > +	 * exit_mm
> > > > +	 */
> > > > +	if (!IS_ENABLED(CONFIG_MMU) && !tsk->mm)
> > > > +		return false;
> > > 
> > > Branching based on CONFIG_MMU is ugly. I suggest timeout based next OOM
> > > victim selection if CONFIG_MMU=n.
> > 
> > I suggest we do not argue about nommu without actually optimizing for or
> > fixing nommu which we are not here. I am even not sure memory reserves
> > can ever be depleted for that config.
> 
> I don't think memory reserves can deplete for CONFIG_MMU=n environment.
> But the reason the OOM reaper was introduced is not limited to handling
> depletion of memory reserves. The OOM reaper was introduced because
> OOM victims might get stuck indirectly waiting for other threads doing
> memory allocation. You said
> 
>   > Yes, exit_aio is the only blocking call I know of currently. But I would
>   > like this to be as robust as possible and so I do not want to rely on
>   > the current implementation. This can change in future and I can
>   > guarantee that nobody will think about the oom path when adding
>   > something to the final __mmput path.
> 
> at http://lkml.kernel.org/r/20170726054533.GA960@dhcp22.suse.cz , but
> how can you guarantee that nobody will think about the oom path
> when adding something to the final __mmput() path without thinking
> about possibility of getting stuck waiting for memory allocation in
> CONFIG_MMU=n environment?

Look, I really appreciate your sentiment for for nommu platform but with
an absolute lack of _any_ oom reports on that platform that I am aware
of nor any reports about lockups during oom I am less than thrilled to
add a code to fix a problem which even might not exist. Nommu is usually
very special with a very specific workload running (e.g. no overcommit)
so I strongly suspect that any OOM theories are highly academic.

All I do care about is to not regress nommu as much as possible. So can
we get back to the proposed patch and updates I have done to address
your review feedback please?
-- 
Michal Hocko
SUSE Labs

WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: akpm@linux-foundation.org, rientjes@google.com,
	hannes@cmpxchg.org, guro@fb.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] mm, oom: do not rely on TIF_MEMDIE for memory reserves access
Date: Thu, 3 Aug 2017 09:06:06 +0200	[thread overview]
Message-ID: <20170803070606.GA12521@dhcp22.suse.cz> (raw)
In-Reply-To: <201708031039.GDG05288.OQJOHtLVFMSFFO@I-love.SAKURA.ne.jp>

On Thu 03-08-17 10:39:42, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Wed 02-08-17 00:30:33, Tetsuo Handa wrote:
> > > > @@ -3603,6 +3612,22 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
> > > >  	return alloc_flags;
> > > >  }
> > > >  
> > > > +static bool oom_reserves_allowed(struct task_struct *tsk)
> > > > +{
> > > > +	if (!tsk_is_oom_victim(tsk))
> > > > +		return false;
> > > > +
> > > > +	/*
> > > > +	 * !MMU doesn't have oom reaper so we shouldn't risk the memory reserves
> > > > +	 * depletion and shouldn't give access to memory reserves passed the
> > > > +	 * exit_mm
> > > > +	 */
> > > > +	if (!IS_ENABLED(CONFIG_MMU) && !tsk->mm)
> > > > +		return false;
> > > 
> > > Branching based on CONFIG_MMU is ugly. I suggest timeout based next OOM
> > > victim selection if CONFIG_MMU=n.
> > 
> > I suggest we do not argue about nommu without actually optimizing for or
> > fixing nommu which we are not here. I am even not sure memory reserves
> > can ever be depleted for that config.
> 
> I don't think memory reserves can deplete for CONFIG_MMU=n environment.
> But the reason the OOM reaper was introduced is not limited to handling
> depletion of memory reserves. The OOM reaper was introduced because
> OOM victims might get stuck indirectly waiting for other threads doing
> memory allocation. You said
> 
>   > Yes, exit_aio is the only blocking call I know of currently. But I would
>   > like this to be as robust as possible and so I do not want to rely on
>   > the current implementation. This can change in future and I can
>   > guarantee that nobody will think about the oom path when adding
>   > something to the final __mmput path.
> 
> at http://lkml.kernel.org/r/20170726054533.GA960@dhcp22.suse.cz , but
> how can you guarantee that nobody will think about the oom path
> when adding something to the final __mmput() path without thinking
> about possibility of getting stuck waiting for memory allocation in
> CONFIG_MMU=n environment?

Look, I really appreciate your sentiment for for nommu platform but with
an absolute lack of _any_ oom reports on that platform that I am aware
of nor any reports about lockups during oom I am less than thrilled to
add a code to fix a problem which even might not exist. Nommu is usually
very special with a very specific workload running (e.g. no overcommit)
so I strongly suspect that any OOM theories are highly academic.

All I do care about is to not regress nommu as much as possible. So can
we get back to the proposed patch and updates I have done to address
your review feedback please?
-- 
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:[~2017-08-03  7:06 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-27  9:03 [PATCH 0/2] mm, oom: do not grant oom victims full memory reserves access Michal Hocko
2017-07-27  9:03 ` Michal Hocko
2017-07-27  9:03 ` [PATCH 1/2] mm, oom: do not rely on TIF_MEMDIE for " Michal Hocko
2017-07-27  9:03   ` Michal Hocko
2017-08-01 15:30   ` Tetsuo Handa
2017-08-01 15:30     ` Tetsuo Handa
2017-08-01 16:52     ` Michal Hocko
2017-08-01 16:52       ` Michal Hocko
2017-08-02  6:10       ` Michal Hocko
2017-08-02  6:10         ` Michal Hocko
2017-08-03  1:39       ` Tetsuo Handa
2017-08-03  1:39         ` Tetsuo Handa
2017-08-03  7:06         ` Michal Hocko [this message]
2017-08-03  7:06           ` Michal Hocko
2017-08-03  8:03           ` Tetsuo Handa
2017-08-03  8:03             ` Tetsuo Handa
2017-08-03  8:21             ` Michal Hocko
2017-08-03  8:21               ` Michal Hocko
2017-08-02  8:29   ` [PATCH v2 " Michal Hocko
2017-08-02  8:29     ` Michal Hocko
2017-08-03  9:37     ` Mel Gorman
2017-08-03  9:37       ` Mel Gorman
2017-08-03 11:00       ` Michal Hocko
2017-08-03 11:00         ` Michal Hocko
2017-08-03 12:22         ` Mel Gorman
2017-08-03 12:22           ` Mel Gorman
2017-07-27  9:03 ` [PATCH 2/2] mm: replace TIF_MEMDIE checks by tsk_is_oom_victim Michal Hocko
2017-07-27  9:03   ` Michal Hocko
2017-07-27 14:01   ` Tetsuo Handa
2017-07-27 14:01     ` Tetsuo Handa
2017-07-27 14:08     ` Tetsuo Handa
2017-07-27 14:08       ` Tetsuo Handa
2017-07-27 14:18     ` Michal Hocko
2017-07-27 14:18       ` Michal Hocko
2017-07-27 14:45     ` Michal Hocko
2017-07-27 14:45       ` Michal Hocko
2017-07-27 14:55       ` Roman Gushchin
2017-07-27 14:55         ` Roman Gushchin
2017-07-29  8:33   ` kbuild test robot
2017-07-31  6:46     ` Michal Hocko
2017-07-31  6:46       ` Michal Hocko
2017-08-01 12:16 ` [PATCH 0/2] mm, oom: do not grant oom victims full memory reserves access Michal Hocko
2017-08-01 12:16   ` Michal Hocko
2017-08-01 12:23   ` Roman Gushchin
2017-08-01 12:23     ` Roman Gushchin
2017-08-01 12:29     ` Michal Hocko
2017-08-01 12:29       ` Michal Hocko
2017-08-01 12:42       ` Roman Gushchin
2017-08-01 12:42         ` Roman Gushchin
2017-08-01 12:54         ` Michal Hocko
2017-08-01 12:54           ` Michal Hocko
2017-08-07 14:21 ` Michal Hocko
2017-08-07 14:21   ` Michal Hocko
2017-08-10  7:50 [PATCH v2 " Michal Hocko
2017-08-10  7:50 ` [PATCH 1/2] mm, oom: do not rely on TIF_MEMDIE for " Michal Hocko
2017-08-10  7:50   ` Michal Hocko

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=20170803070606.GA12521@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=guro@fb.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --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 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.