Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756198AbcLNUks (ORCPT ); Wed, 14 Dec 2016 15:40:48 -0500 Received: from gum.cmpxchg.org ([85.214.110.215]:52642 "EHLO gum.cmpxchg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752366AbcLNUkq (ORCPT ); Wed, 14 Dec 2016 15:40:46 -0500 Date: Wed, 14 Dec 2016 15:35:30 -0500 From: Johannes Weiner To: Michal Hocko Cc: Andrew Morton , Vlastimil Babka , Tetsuo Handa , Mel Gorman , David Rientjes , linux-mm@kvack.org, LKML , Michal Hocko Subject: Re: [PATCH] mm: consolidate GFP_NOFAIL checks in the allocator slowpath Message-ID: <20161214203530.GA18561@cmpxchg.org> References: <20161214150706.27412-1-mhocko@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161214150706.27412-1-mhocko@kernel.org> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1541 Lines: 31 On Wed, Dec 14, 2016 at 04:07:06PM +0100, Michal Hocko wrote: > From: Michal Hocko > > 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 > Acked-by: Vlastimil Babka 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