Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754209AbbKWJ3i (ORCPT ); Mon, 23 Nov 2015 04:29:38 -0500 Received: from mail-wm0-f49.google.com ([74.125.82.49]:37420 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753808AbbKWJ33 (ORCPT ); Mon, 23 Nov 2015 04:29:29 -0500 Date: Mon, 23 Nov 2015 10:29:26 +0100 From: Michal Hocko To: Vlastimil Babka Cc: Andrew Morton , Johannes Weiner , Andrea Arcangeli , Mel Gorman , David Rientjes , linux-mm@kvack.org, LKML Subject: Re: [PATCH] mm, oom: Give __GFP_NOFAIL allocations access to memory reserves Message-ID: <20151123092925.GB21050@dhcp22.suse.cz> References: <1447249697-13380-1-git-send-email-mhocko@kernel.org> <5651BB43.8030102@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5651BB43.8030102@suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2459 Lines: 64 On Sun 22-11-15 13:55:31, Vlastimil Babka wrote: > On 11.11.2015 14:48, mhocko@kernel.org wrote: > > mm/page_alloc.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 8034909faad2..d30bce9d7ac8 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -2766,8 +2766,16 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order, > > goto out; > > } > > /* Exhausted what can be done so it's blamo time */ > > - if (out_of_memory(&oc) || WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL)) > > + if (out_of_memory(&oc) || WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL)) { > > *did_some_progress = 1; > > + > > + if (gfp_mask & __GFP_NOFAIL) { > > + page = get_page_from_freelist(gfp_mask, order, > > + ALLOC_NO_WATERMARKS|ALLOC_CPUSET, ac); > > + WARN_ONCE(!page, "Unable to fullfil gfp_nofail allocation." > > + " Consider increasing min_free_kbytes.\n"); > > It seems redundant to me to keep the WARN_ON_ONCE also above in the if () part? They are warning about two different things. The first one catches a buggy code which uses __GFP_NOFAIL from oom disabled context while the second one tries to help the administrator with a hint that memory reserves are too small. > Also s/gfp_nofail/GFP_NOFAIL/ for consistency? Fair enough, changed. > Hm and probably out of scope of your patch, but I understand the WARN_ONCE > (WARN_ON_ONCE) to be _ONCE just to prevent a flood from a single task looping > here. But for distinct tasks and potentially far away in time, wouldn't we want > to see all the warnings? Would that be feasible to implement? I was thinking about that as well some time ago but it was quite hard to find a good enough API to tell when to warn again. The first WARN_ON_ONCE should trigger for all different _code paths_ no matter how frequently they appear to catch all the buggy callers. The second one would benefit from a new warning after min_free_kbytes was updated because it would tell the administrator that the last update was not sufficient for the workload. > > > + } > > + } > > out: > > mutex_unlock(&oom_lock); > > return page; > > Thanks! -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/