From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Vlastimil Babka <vbabka@suse.cz>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Mel Gorman <mgorman@suse.de>,
David Rientjes <rientjes@google.com>,
linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>,
Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH] mm: consolidate GFP_NOFAIL checks in the allocator slowpath
Date: Wed, 14 Dec 2016 15:35:30 -0500 [thread overview]
Message-ID: <20161214203530.GA18561@cmpxchg.org> (raw)
In-Reply-To: <20161214150706.27412-1-mhocko@kernel.org>
On Wed, Dec 14, 2016 at 04:07:06PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> Tetsuo Handa has pointed out that 0a0337e0d1d1 ("mm, oom: rework oom
> detection") has subtly changed semantic for costly high order requests
> with __GFP_NOFAIL and withtout __GFP_REPEAT and those can fail right now.
> My code inspection didn't reveal any such users in the tree but it is
> true that this might lead to unexpected allocation failures and
> subsequent OOPs.
>
> __alloc_pages_slowpath wrt. GFP_NOFAIL is hard to follow currently.
> There are few special cases but we are lacking a catch all place to be
> sure we will not miss any case where the non failing allocation might
> fail. This patch reorganizes the code a bit and puts all those special
> cases under nopage label which is the generic go-to-fail path. Non
> failing allocations are retried or those that cannot retry like
> non-sleeping allocation go to the failure point directly. This should
> make the code flow much easier to follow and make it less error prone
> for future changes.
>
> While we are there we have to move the stall check up to catch
> potentially looping non-failing allocations.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
It's not the nicest thing that we have to duplicate all the conditions
to warn on, but it's preferable over unreliable GFP_NOFAIL handling.
Consolidating the handling of this flag makes a lot of sense to me.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
--
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: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Vlastimil Babka <vbabka@suse.cz>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Mel Gorman <mgorman@suse.de>,
David Rientjes <rientjes@google.com>,
linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>,
Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH] mm: consolidate GFP_NOFAIL checks in the allocator slowpath
Date: Wed, 14 Dec 2016 15:35:30 -0500 [thread overview]
Message-ID: <20161214203530.GA18561@cmpxchg.org> (raw)
In-Reply-To: <20161214150706.27412-1-mhocko@kernel.org>
On Wed, Dec 14, 2016 at 04:07:06PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> Tetsuo Handa has pointed out that 0a0337e0d1d1 ("mm, oom: rework oom
> detection") has subtly changed semantic for costly high order requests
> with __GFP_NOFAIL and withtout __GFP_REPEAT and those can fail right now.
> My code inspection didn't reveal any such users in the tree but it is
> true that this might lead to unexpected allocation failures and
> subsequent OOPs.
>
> __alloc_pages_slowpath wrt. GFP_NOFAIL is hard to follow currently.
> There are few special cases but we are lacking a catch all place to be
> sure we will not miss any case where the non failing allocation might
> fail. This patch reorganizes the code a bit and puts all those special
> cases under nopage label which is the generic go-to-fail path. Non
> failing allocations are retried or those that cannot retry like
> non-sleeping allocation go to the failure point directly. This should
> make the code flow much easier to follow and make it less error prone
> for future changes.
>
> While we are there we have to move the stall check up to catch
> potentially looping non-failing allocations.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
It's not the nicest thing that we have to duplicate all the conditions
to warn on, but it's preferable over unreliable GFP_NOFAIL handling.
Consolidating the handling of this flag makes a lot of sense to me.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
next prev parent reply other threads:[~2016-12-14 20:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-14 15:07 [PATCH] mm: consolidate GFP_NOFAIL checks in the allocator slowpath Michal Hocko
2016-12-14 15:07 ` Michal Hocko
2016-12-14 20:35 ` Johannes Weiner [this message]
2016-12-14 20:35 ` Johannes Weiner
2016-12-15 7:54 ` Hillf Danton
2016-12-15 7:54 ` Hillf Danton
2016-12-15 10:28 ` [PATCH v2] " Michal Hocko
2016-12-15 10:28 ` Michal Hocko
2016-12-16 11:39 ` Tetsuo Handa
2016-12-16 11:39 ` Tetsuo Handa
2016-12-16 11:50 ` Michal Hocko
2016-12-16 11: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=20161214203530.GA18561@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=mhocko@kernel.org \
--cc=mhocko@suse.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=rientjes@google.com \
--cc=vbabka@suse.cz \
/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.