From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f199.google.com (mail-wr0-f199.google.com [209.85.128.199]) by kanga.kvack.org (Postfix) with ESMTP id 4655E6B025F for ; Fri, 1 Sep 2017 10:28:49 -0400 (EDT) Received: by mail-wr0-f199.google.com with SMTP id y21so580592wrd.3 for ; Fri, 01 Sep 2017 07:28:49 -0700 (PDT) Received: from mx1.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id z203si165877wmg.179.2017.09.01.07.28.47 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 01 Sep 2017 07:28:47 -0700 (PDT) Date: Fri, 1 Sep 2017 16:28:45 +0200 From: Michal Hocko Subject: Re: [PATCH] mm,page_alloc: apply gfp_allowed_mask before the first allocation attempt. Message-ID: <20170901142845.nqcn2na4vy6giyhm@dhcp22.suse.cz> References: <1504275091-4427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1504275091-4427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> Sender: owner-linux-mm@kvack.org List-ID: To: Tetsuo Handa Cc: akpm@linux-foundation.org, linux-mm@kvack.org, David Rientjes , Vlastimil Babka , Hillf Danton , Jesper Dangaard Brouer , Mel Gorman On Fri 01-09-17 23:11:31, Tetsuo Handa wrote: > We are by error initializing alloc_flags before gfp_allowed_mask is > applied. Apply gfp_allowed_mask before initializing alloc_flags so that > the first allocation attempt uses correct flags. It would be worth noting that this will not matter in most cases, actually when only the node reclaim is enabled we can misbehave because NOFS request for PM paths would be ignored. > Fixes: 9cd7555875bb09da ("mm, page_alloc: split alloc_pages_nodemask()") AFAICS this patch hasn't changed the logic and it was broken since 83d4ca8148fd ("mm, page_alloc: move __GFP_HARDWALL modifications out of the fastpath") > Signed-off-by: Tetsuo Handa > Cc: Mel Gorman > Cc: Hillf Danton > Cc: Vlastimil Babka > Cc: Jesper Dangaard Brouer Other than that this looks correct to me. Acked-by: Michal Hocko I wish we can finally get rid of gfp_allowed_mask. I have it on my todo list but never got to it. Thanks! > --- > mm/page_alloc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 6dbc49e..a123dee 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4179,10 +4179,11 @@ struct page * > { > struct page *page; > unsigned int alloc_flags = ALLOC_WMARK_LOW; > - gfp_t alloc_mask = gfp_mask; /* The gfp_t that was actually used for allocation */ > + gfp_t alloc_mask; /* The gfp_t that was actually used for allocation */ > struct alloc_context ac = { }; > > gfp_mask &= gfp_allowed_mask; > + alloc_mask = gfp_mask; > if (!prepare_alloc_pages(gfp_mask, order, preferred_nid, nodemask, &ac, &alloc_mask, &alloc_flags)) > return NULL; > > -- > 1.8.3.1 > -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f71.google.com (mail-oi0-f71.google.com [209.85.218.71]) by kanga.kvack.org (Postfix) with ESMTP id BB3986B025F for ; Fri, 1 Sep 2017 11:17:11 -0400 (EDT) Received: by mail-oi0-f71.google.com with SMTP id u206so884216oif.5 for ; Fri, 01 Sep 2017 08:17:11 -0700 (PDT) Received: from www262.sakura.ne.jp (www262.sakura.ne.jp. [2001:e42:101:1:202:181:97:72]) by mx.google.com with ESMTPS id w9si280456oib.470.2017.09.01.08.17.09 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 01 Sep 2017 08:17:10 -0700 (PDT) Subject: Re: [PATCH] mm,page_alloc: apply gfp_allowed_mask before the first allocation attempt. From: Tetsuo Handa References: <1504275091-4427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20170901142845.nqcn2na4vy6giyhm@dhcp22.suse.cz> In-Reply-To: <20170901142845.nqcn2na4vy6giyhm@dhcp22.suse.cz> Message-Id: <201709020016.ADJ21342.OFLJHOOSMFVtFQ@I-love.SAKURA.ne.jp> Date: Sat, 2 Sep 2017 00:16:58 +0900 Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-linux-mm@kvack.org List-ID: To: mhocko@suse.com Cc: akpm@linux-foundation.org, linux-mm@kvack.org, rientjes@google.com, vbabka@suse.cz, brouer@redhat.com, mgorman@techsingularity.net Michal Hocko wrote: > On Fri 01-09-17 23:11:31, Tetsuo Handa wrote: > > We are by error initializing alloc_flags before gfp_allowed_mask is > > applied. Apply gfp_allowed_mask before initializing alloc_flags so that > > the first allocation attempt uses correct flags. > > It would be worth noting that this will not matter in most cases, > actually when only the node reclaim is enabled we can misbehave because > NOFS request for PM paths would be ignored. > > > Fixes: 9cd7555875bb09da ("mm, page_alloc: split alloc_pages_nodemask()") > > AFAICS this patch hasn't changed the logic and it was broken since > 83d4ca8148fd ("mm, page_alloc: move __GFP_HARDWALL modifications out of > the fastpath") Indeed. Updated patch follows. > > > Signed-off-by: Tetsuo Handa > > Cc: Mel Gorman > > Cc: Hillf Danton > > Cc: Vlastimil Babka > > Cc: Jesper Dangaard Brouer > > Other than that this looks correct to me. > Acked-by: Michal Hocko > > I wish we can finally get rid of gfp_allowed_mask. I have it on my todo > list but never got to it. > > Thanks! > ---------- >>From b454863bea884158a25460aa29a26c5feb16fe94 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Fri, 1 Sep 2017 23:11:31 +0900 Subject: [PATCH v2] mm,page_alloc: apply gfp_allowed_mask before the first allocation attempt. We are by error initializing alloc_flags before gfp_allowed_mask is applied. This could cause problems after pm_restrict_gfp_mask() is called during suspend operation. Apply gfp_allowed_mask before initializing alloc_flags so that the first allocation attempt uses correct flags. Fixes: 83d4ca8148fd9092 ("mm, page_alloc: move __GFP_HARDWALL modifications out of the fastpath") Signed-off-by: Tetsuo Handa Acked-by: Michal Hocko Cc: Mel Gorman Cc: Vlastimil Babka Cc: Jesper Dangaard Brouer --- mm/page_alloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 6dbc49e..a123dee 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4179,10 +4179,11 @@ struct page * { struct page *page; unsigned int alloc_flags = ALLOC_WMARK_LOW; - gfp_t alloc_mask = gfp_mask; /* The gfp_t that was actually used for allocation */ + gfp_t alloc_mask; /* The gfp_t that was actually used for allocation */ struct alloc_context ac = { }; gfp_mask &= gfp_allowed_mask; + alloc_mask = gfp_mask; if (!prepare_alloc_pages(gfp_mask, order, preferred_nid, nodemask, &ac, &alloc_mask, &alloc_flags)) return NULL; -- 1.8.3.1 -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f70.google.com (mail-pg0-f70.google.com [74.125.83.70]) by kanga.kvack.org (Postfix) with ESMTP id E78BD6B025F for ; Fri, 1 Sep 2017 11:19:11 -0400 (EDT) Received: by mail-pg0-f70.google.com with SMTP id u15so1047435pgb.7 for ; Fri, 01 Sep 2017 08:19:11 -0700 (PDT) Received: from www262.sakura.ne.jp (www262.sakura.ne.jp. [2001:e42:101:1:202:181:97:72]) by mx.google.com with ESMTPS id 59si275032plp.650.2017.09.01.08.19.10 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 01 Sep 2017 08:19:11 -0700 (PDT) From: Tetsuo Handa Subject: [PATCH] mm,page_alloc: apply gfp_allowed_mask before the first allocation attempt. Date: Fri, 1 Sep 2017 23:11:31 +0900 Message-Id: <1504275091-4427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org Cc: linux-mm@kvack.org, David Rientjes , Michal Hocko , Vlastimil Babka , Tetsuo Handa , Hillf Danton , Jesper Dangaard Brouer , Mel Gorman We are by error initializing alloc_flags before gfp_allowed_mask is applied. Apply gfp_allowed_mask before initializing alloc_flags so that the first allocation attempt uses correct flags. Fixes: 9cd7555875bb09da ("mm, page_alloc: split alloc_pages_nodemask()") Signed-off-by: Tetsuo Handa Cc: Mel Gorman Cc: Hillf Danton Cc: Vlastimil Babka Cc: Jesper Dangaard Brouer --- mm/page_alloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 6dbc49e..a123dee 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4179,10 +4179,11 @@ struct page * { struct page *page; unsigned int alloc_flags = ALLOC_WMARK_LOW; - gfp_t alloc_mask = gfp_mask; /* The gfp_t that was actually used for allocation */ + gfp_t alloc_mask; /* The gfp_t that was actually used for allocation */ struct alloc_context ac = { }; gfp_mask &= gfp_allowed_mask; + alloc_mask = gfp_mask; if (!prepare_alloc_pages(gfp_mask, order, preferred_nid, nodemask, &ac, &alloc_mask, &alloc_flags)) return NULL; -- 1.8.3.1 -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f197.google.com (mail-wr0-f197.google.com [209.85.128.197]) by kanga.kvack.org (Postfix) with ESMTP id 536556B0495 for ; Mon, 4 Sep 2017 04:23:03 -0400 (EDT) Received: by mail-wr0-f197.google.com with SMTP id 40so11783120wrv.4 for ; Mon, 04 Sep 2017 01:23:03 -0700 (PDT) Received: from mx1.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id r17si4788644wrg.102.2017.09.04.01.23.01 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 04 Sep 2017 01:23:01 -0700 (PDT) Subject: Re: [PATCH] mm,page_alloc: apply gfp_allowed_mask before the first allocation attempt. References: <1504275091-4427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20170901142845.nqcn2na4vy6giyhm@dhcp22.suse.cz> <201709020016.ADJ21342.OFLJHOOSMFVtFQ@I-love.SAKURA.ne.jp> From: Vlastimil Babka Message-ID: Date: Mon, 4 Sep 2017 10:22:59 +0200 MIME-Version: 1.0 In-Reply-To: <201709020016.ADJ21342.OFLJHOOSMFVtFQ@I-love.SAKURA.ne.jp> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Tetsuo Handa , mhocko@suse.com Cc: akpm@linux-foundation.org, linux-mm@kvack.org, rientjes@google.com, brouer@redhat.com, mgorman@techsingularity.net On 09/01/2017 05:16 PM, Tetsuo Handa wrote: > Michal Hocko wrote: >> On Fri 01-09-17 23:11:31, Tetsuo Handa wrote: >>> We are by error initializing alloc_flags before gfp_allowed_mask is >>> applied. Apply gfp_allowed_mask before initializing alloc_flags so that >>> the first allocation attempt uses correct flags. >> >> It would be worth noting that this will not matter in most cases, >> actually when only the node reclaim is enabled we can misbehave because >> NOFS request for PM paths would be ignored. Hmm don't we have the same problem with the god-damned node reclaim by applying current_gfp_context() also only after the first attempt? But that would be present since 21caf2fc1931b. Hm, actually no, because reclaim calls current_gfp_context() by itself. Good. Maybe reclaim should also do the gfp_allowed_mask filtering? I wonder how safe the pm_restrict_gfp_mask() update is when an allocation is already looping in __alloc_pages_slowpath()... What exactly are your ideas to get rid of gfp_allowed_mask, Michal? >>> Fixes: 9cd7555875bb09da ("mm, page_alloc: split alloc_pages_nodemask()") >> >> AFAICS this patch hasn't changed the logic and it was broken since >> 83d4ca8148fd ("mm, page_alloc: move __GFP_HARDWALL modifications out of >> the fastpath") > > Indeed. Updated patch follows. > >> >>> Signed-off-by: Tetsuo Handa >>> Cc: Mel Gorman >>> Cc: Hillf Danton >>> Cc: Vlastimil Babka >>> Cc: Jesper Dangaard Brouer >> >> Other than that this looks correct to me. >> Acked-by: Michal Hocko >> >> I wish we can finally get rid of gfp_allowed_mask. I have it on my todo >> list but never got to it. >> >> Thanks! >> > ---------- >>>From b454863bea884158a25460aa29a26c5feb16fe94 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa > Date: Fri, 1 Sep 2017 23:11:31 +0900 > Subject: [PATCH v2] mm,page_alloc: apply gfp_allowed_mask before the first > allocation attempt. > > We are by error initializing alloc_flags before gfp_allowed_mask is > applied. This could cause problems after pm_restrict_gfp_mask() is > called during suspend operation. Apply gfp_allowed_mask before > initializing alloc_flags so that the first allocation attempt uses > correct flags. > > Fixes: 83d4ca8148fd9092 ("mm, page_alloc: move __GFP_HARDWALL modifications out of the fastpath") > Signed-off-by: Tetsuo Handa > Acked-by: Michal Hocko > Cc: Mel Gorman > Cc: Vlastimil Babka > Cc: Jesper Dangaard Brouer Acked-by: Vlastimil Babka > --- > mm/page_alloc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 6dbc49e..a123dee 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4179,10 +4179,11 @@ struct page * > { > struct page *page; > unsigned int alloc_flags = ALLOC_WMARK_LOW; > - gfp_t alloc_mask = gfp_mask; /* The gfp_t that was actually used for allocation */ > + gfp_t alloc_mask; /* The gfp_t that was actually used for allocation */ > struct alloc_context ac = { }; > > gfp_mask &= gfp_allowed_mask; > + alloc_mask = gfp_mask; > if (!prepare_alloc_pages(gfp_mask, order, preferred_nid, nodemask, &ac, &alloc_mask, &alloc_flags)) > return NULL; > > -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f69.google.com (mail-wm0-f69.google.com [74.125.82.69]) by kanga.kvack.org (Postfix) with ESMTP id 3A5C26B049B for ; Mon, 4 Sep 2017 04:47:19 -0400 (EDT) Received: by mail-wm0-f69.google.com with SMTP id u26so8791968wma.3 for ; Mon, 04 Sep 2017 01:47:19 -0700 (PDT) Received: from mx1.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id f15si4544769wmg.82.2017.09.04.01.47.17 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 04 Sep 2017 01:47:17 -0700 (PDT) Date: Mon, 4 Sep 2017 10:47:15 +0200 From: Michal Hocko Subject: Re: [PATCH] mm,page_alloc: apply gfp_allowed_mask before the first allocation attempt. Message-ID: <20170904084715.aeyckbfciif7g2z2@dhcp22.suse.cz> References: <1504275091-4427-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20170901142845.nqcn2na4vy6giyhm@dhcp22.suse.cz> <201709020016.ADJ21342.OFLJHOOSMFVtFQ@I-love.SAKURA.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Vlastimil Babka Cc: Tetsuo Handa , akpm@linux-foundation.org, linux-mm@kvack.org, rientjes@google.com, brouer@redhat.com, mgorman@techsingularity.net On Mon 04-09-17 10:22:59, Vlastimil Babka wrote: > On 09/01/2017 05:16 PM, Tetsuo Handa wrote: > > Michal Hocko wrote: > >> On Fri 01-09-17 23:11:31, Tetsuo Handa wrote: > >>> We are by error initializing alloc_flags before gfp_allowed_mask is > >>> applied. Apply gfp_allowed_mask before initializing alloc_flags so that > >>> the first allocation attempt uses correct flags. > >> > >> It would be worth noting that this will not matter in most cases, > >> actually when only the node reclaim is enabled we can misbehave because > >> NOFS request for PM paths would be ignored. > > Hmm don't we have the same problem with the god-damned node reclaim by > applying current_gfp_context() also only after the first attempt? But > that would be present since 21caf2fc1931b. > Hm, actually no, because reclaim calls current_gfp_context() by itself. > Good. Yes. > Maybe reclaim should also do the gfp_allowed_mask filtering? I would rather not spread it more than it is really needed. > I wonder how safe the pm_restrict_gfp_mask() update is when an > allocation is already looping in __alloc_pages_slowpath()... It will be broken > What exactly are your ideas to get rid of gfp_allowed_mask, Michal? Well I planned to actually examine why do we need it in the first place and whether the original intention still applies and if yes then replace it by memalloc_noio_save. It would still be proken in a similar way you pointed out but something tells me that it is just obsolete. -- 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: email@kvack.org