Re: [PATCH] mm: consolidate GFP_NOFAIL checks in the allocator slowpath
From: Johannes Weiner
Date: Wed Dec 14 2016 - 15:40:51 EST
On Wed, Dec 14, 2016 at 04:07:06PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@xxxxxxxx>
> 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@xxxxxxxx>
> Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
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@xxxxxxxxxxx>