Re: __GFP_NOFAIL and oom_killer_disabled?

[Michal Hocko <mhocko@suse.cz>]

On Wed 25-02-15 20:22:22, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > This commit hasn't introduced any behavior changes. GFP_NOFAIL
> > allocations fail when OOM killer is disabled since beginning
> > 7f33d49a2ed5 (mm, PM/Freezer: Disable OOM killer when tasks are frozen).
> 
> I thought that
> 
> -       out_of_memory(ac->zonelist, gfp_mask, order, ac->nodemask, false);
> -       *did_some_progress = 1;
> +       if (out_of_memory(ac->zonelist, gfp_mask, order, ac->nodemask, false))
> +               *did_some_progress = 1;
> 
> in commit c32b3cbe0d067a9c "oom, PM: make OOM detection in the freezer
> path raceless" introduced a code path which fails to set
> *did_some_progress to non 0 value.

But this commit had also the following hunk:
@@ -2317,9 +2315,6 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
 
        *did_some_progress = 0;
 
-       if (oom_killer_disabled)
-               return NULL;
-

so we even wouldn't get down to out_of_memory and returned with
did_some_progress=0 right away. So the patch hasn't changed the logic.

> > "
> > We haven't seen any bug reports since 2009 so I haven't marked the patch
> > for stable. I have no problem to backport it to stable trees though if
> > people think it is a good precaution.
> > "
> 
> Until 3.18, GFP_NOFAIL for GFP_NOFS / GFP_NOIO did not fail with
> oom_killer_disabled == true because of
> 
> ----------
>         if (!did_some_progress) {
>                 if (oom_gfp_allowed(gfp_mask)) {
>                         if (oom_killer_disabled)
>                                 goto nopage;
> 			(...snipped...)
>                         goto restart;
>                 }
>         }
> 	(...snipped...)
> 	goto rebalance;
> ----------
> 
> and that might be the reason you did not see bug reports.
> In 3.19, GFP_NOFAIL for GFP_NOFS / GFP_NOIO started to fail with
> oom_killer_disabled == true because of

OK, that would change the bahavior for __GFP_NOFAIL|~__GFP_FS
allocations. The patch from Johannes which reverts GFP_NOFS failure mode
should go to stable and that should be sufficient IMO.
 
[...]

> So, it is commit 9879de7373fc "mm: page_alloc: embed OOM killing naturally
> into allocation slowpath" than commit c32b3cbe0d067a9c "oom, PM: make OOM
> detection in the freezer path raceless" that introduced behavior changes?

Yes.

-- 
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>