From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: akpm@linux-foundation.org, hannes@cmpxchg.org,
rientjes@google.com, mgorman@suse.de, hillf.zj@alibaba-inc.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically
Date: Wed, 21 Dec 2016 09:15:56 +0100 [thread overview]
Message-ID: <20161221081556.GG16502@dhcp22.suse.cz> (raw)
In-Reply-To: <201612210031.BFD48914.VMtHSFFJOLQFOO@I-love.SAKURA.ne.jp>
On Wed 21-12-16 00:31:47, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index c8eed66d8abb..2dda7c3eba52 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3098,32 +3098,31 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
> > if (page)
> > goto out;
> >
> > - if (!(gfp_mask & __GFP_NOFAIL)) {
> > - /* Coredumps can quickly deplete all memory reserves */
> > - if (current->flags & PF_DUMPCORE)
> > - goto out;
> > - /* The OOM killer will not help higher order allocs */
> > - if (order > PAGE_ALLOC_COSTLY_ORDER)
> > - goto out;
> > - /* The OOM killer does not needlessly kill tasks for lowmem */
> > - if (ac->high_zoneidx < ZONE_NORMAL)
> > - goto out;
> > - if (pm_suspended_storage())
> > - goto out;
> > - /*
> > - * XXX: GFP_NOFS allocations should rather fail than rely on
> > - * other request to make a forward progress.
> > - * We are in an unfortunate situation where out_of_memory cannot
> > - * do much for this context but let's try it to at least get
> > - * access to memory reserved if the current task is killed (see
> > - * out_of_memory). Once filesystems are ready to handle allocation
> > - * failures more gracefully we should just bail out here.
> > - */
> > + /* Coredumps can quickly deplete all memory reserves */
> > + if (current->flags & PF_DUMPCORE)
> > + goto out;
> > + /* The OOM killer will not help higher order allocs */
> > + if (order > PAGE_ALLOC_COSTLY_ORDER)
> > + goto out;
> > + /* The OOM killer does not needlessly kill tasks for lowmem */
> > + if (ac->high_zoneidx < ZONE_NORMAL)
> > + goto out;
> > + if (pm_suspended_storage())
> > + goto out;
> > + /*
> > + * XXX: GFP_NOFS allocations should rather fail than rely on
> > + * other request to make a forward progress.
> > + * We are in an unfortunate situation where out_of_memory cannot
> > + * do much for this context but let's try it to at least get
> > + * access to memory reserved if the current task is killed (see
> > + * out_of_memory). Once filesystems are ready to handle allocation
> > + * failures more gracefully we should just bail out here.
> > + */
> > +
> > + /* The OOM killer may not free memory on a specific node */
> > + if (gfp_mask & __GFP_THISNODE)
> > + goto out;
> >
> > - /* The OOM killer may not free memory on a specific node */
> > - if (gfp_mask & __GFP_THISNODE)
> > - goto out;
> > - }
> > /* Exhausted what can be done so it's blamo time */
> > if (out_of_memory(&oc) || WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL)) {
> > *did_some_progress = 1;
>
> Why do we need to change this part in this patch?
>
> This change silently prohibits invoking the OOM killer for e.g. costly
> GFP_KERNEL allocation.
We have never allowed coslty GFP_KERNEL requests to invoke the oom
killer. And there is a good reason for it which is even mentioned in the
changelog. The only change here is that GFP_NOFAIL doesn't override this
decision - again for reasons mentioned in the changelog.
> While it would be better if vmalloc() can be used,
> there might be users who cannot accept vmalloc() as a fallback (e.g.
> CONFIG_MMU=n where vmalloc() == kmalloc() ?).
I haven't ever heard any complains about this in the past. If there is a
valid usecase then we can treat nommu specialy. That would require more
changes though.
> This change is not "do not enforce OOM killer automatically" but
> "never allow OOM killer". No exception is allowed. If we change
> this part, title for this part should be something strong like
> "mm,oom: Never allow OOM killer for coredumps, costly allocations,
> lowmem etc.".
Sigh. We didn't allow the oom killer for all those cases and the only
thing that is changed here is to not override those decisions with
__GFP_NOFAIL which is imho reflected in the title. If that is not clear
then I would suggest "mm, oom: do not override OOM killer decisions with
__GFP_NOFAIL".
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2016-12-21 8:16 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-20 13:49 [PATCH 0/3 -v3] GFP_NOFAIL cleanups Michal Hocko
2016-12-20 13:49 ` [PATCH 1/3] mm: consolidate GFP_NOFAIL checks in the allocator slowpath Michal Hocko
2016-12-20 13:49 ` [PATCH 2/3] mm, oom: do not enfore OOM killer for __GFP_NOFAIL automatically Michal Hocko
2016-12-20 15:31 ` Tetsuo Handa
2016-12-21 8:15 ` Michal Hocko [this message]
2017-01-19 18:41 ` Johannes Weiner
2017-01-20 8:33 ` Hillf Danton
2017-01-24 12:40 ` Michal Hocko
2017-01-25 7:00 ` Hillf Danton
2017-01-25 7:59 ` Michal Hocko
2017-01-25 8:41 ` Hillf Danton
2017-01-25 10:19 ` Michal Hocko
2016-12-20 13:49 ` [PATCH 3/3] mm: help __GFP_NOFAIL allocations which do not trigger OOM killer Michal Hocko
2017-01-02 15:49 ` [PATCH 0/3 -v3] GFP_NOFAIL cleanups Michal Hocko
2017-01-03 1:36 ` Tetsuo Handa
2017-01-03 8:42 ` Michal Hocko
2017-01-03 14:38 ` Tetsuo Handa
2017-01-03 16:25 ` Vlastimil Babka
2017-01-03 20:40 ` Michal Hocko
2017-01-04 14:22 ` Tetsuo Handa
2017-01-04 15:20 ` Michal Hocko
2017-01-05 10:50 ` Tetsuo Handa
2017-01-05 11:54 ` Michal Hocko
2017-01-18 18:42 ` 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=20161221081556.GG16502@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=hillf.zj@alibaba-inc.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--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 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).