Re: [RFC PATCH] mm, oom: move GFP_NOFS check to out_of_memory

[Michal Hocko]

On Thu 31-03-16 20:56:23, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Wed 30-03-16 20:46:48, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > On Tue 29-03-16 15:13:54, David Rientjes wrote:
> > > > > On Tue, 29 Mar 2016, Michal Hocko wrote:
> > > > > 
> > > > > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > > > > index 86349586eacb..1c2b7a82f0c4 100644
> > > > > > --- a/mm/oom_kill.c
> > > > > > +++ b/mm/oom_kill.c
> > > > > > @@ -876,6 +876,10 @@ bool out_of_memory(struct oom_control *oc)
> > > > > >  		return true;
> > > > > >  	}
> > > > > >  
> > > > > > +	/* The OOM killer does not compensate for IO-less reclaim. */
> > > > > > +	if (!(oc->gfp_mask & __GFP_FS))
> > > > > > +		return true;
> > > > > > +
> > > 
> > > This patch will disable pagefault_out_of_memory() because currently
> > > pagefault_out_of_memory() is passing oc->gfp_mask == 0.
> > > 
> > > Because of current behavior, calling oom notifiers from !__GFP_FS seems
> > > to be safe.
> > 
> > You are right! I have completely missed that and thought we were
> > providing GFP_KERNEL there. So we have two choices. Either we do
> > use GFP_KERNEL (same as we do for sysrq+f) or we special case
> > pagefault_out_of_memory in some way. The second option seems to be safer
> > because the gfp_mask has to contain at least ___GFP_DIRECT_RECLAIM to
> > trigger the OOM path.
> 
> Oops, I missed that this patch also disables out_of_memory() for !__GFP_FS &&
> __GFP_NOFAIL allocation requests.

True. The following should take care of that:

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 54aa4ec06889..32d8210b8773 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -882,7 +882,7 @@ bool out_of_memory(struct oom_control *oc)
 	 * make sure exclude 0 mask - all other users should have at least
 	 * ___GFP_DIRECT_RECLAIM to get here.
 	 */
-	if (oc->gfp_mask && !(oc->gfp_mask & __GFP_FS))
+	if (oc->gfp_mask && !(oc->gfp_mask & (__GFP_FS|__GFP_NOFAIL)))
 		return true;
 
 	/*

Thanks for spotting this!

[...]
-- 
Michal Hocko
SUSE Labs