Re: [PATCH] mm: consolidate GFP_NOFAIL checks in the allocator slowpath

[Johannes Weiner]

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>