Re: [PATCH] mm,oom: do not loop !__GFP_FS allocation if the OOM killer is disabled.

[Michal Hocko <mhocko@suse.cz>]

On Mon 11-01-16 17:02:16, Johannes Weiner wrote:
> On Tue, Jan 12, 2016 at 06:30:15AM +0900, Tetsuo Handa wrote:
> > Michal Hocko wrote:
> > > > Scratch my objection to this patch then. But please do add to/update
> > > > that XXX comment above that line, or it'll be confusing. Hm?
> > > > 
> > > > 			/*
> > > > 			 * XXX: Page reclaim didn't yield anything,
> > > > 			 * and the OOM killer can't be invoked, but
> > > > 			 * keep looping as per tradition. Unless the
> > > > 			 * system is trying to enter a quiescent state
> > > > 			 * during suspend and the OOM killer has been
> > > > 			 * shut off already. Give up like with other
> > > > 			 * !__GFP_NOFAIL allocations in that case.
> > > > 			 */
> > > > 			*did_some_progress = !oom_killer_disabled;
> > > 
> > > Yes this makes it more clear IMO.
> > > 
> > If you don't want to expose oom_killer_disabled outside of the OOM proper,
> > can't we move this "if (!(gfp_mask & __GFP_FS)) { ... }" block to before
> > constraint = constrained_alloc(oc, &totalpages) line in out_of_memory() ?
> 
> I think your patch is fine as it is.
> 
> It's better to pull out oom_killer_disabled. We want the logic that
> filters OOM invocation based on allocation type in one place. And as
> per the XXX we eventually want to drop that bogus *did_some_progress
> setting anyway.

Completely agreed.

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