From: David Rientjes <rientjes@google.com> To: Vlastimil Babka <vbabka@suse.cz> Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>, Mel Gorman <mgorman@suse.de>, Greg Thelen <gthelen@google.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, Christoph Lameter <cl@linux.com>, Pekka Enberg <penberg@kernel.org>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Subject: Re: [RFC v2 1/4] mm: make alloc_pages_exact_node pass __GFP_THISNODE Date: Fri, 24 Jul 2015 16:09:05 -0700 (PDT) [thread overview] Message-ID: <alpine.DEB.2.10.1507241606270.12744@chino.kir.corp.google.com> (raw) In-Reply-To: <55B2A596.1010101@suse.cz> On Fri, 24 Jul 2015, Vlastimil Babka wrote: > > I assume you looked at the collapse_huge_page() case and decided that it > > needs no modification since the gfp mask is used later for other calls? > > Yeah. Not that the memcg charge parts would seem to care about __GFP_THISNODE, > though. > Hmm, not sure that memcg would ever care about __GFP_THISNODE. I wonder if it make more sense to remove setting __GFP_THISNODE in collapse_huge_page()? khugepaged_alloc_page() seems fine with the new alloc_pages_exact_node() semantics. > >> diff --git a/mm/migrate.c b/mm/migrate.c > >> index f53838f..d139222 100644 > >> --- a/mm/migrate.c > >> +++ b/mm/migrate.c > >> @@ -1554,10 +1554,8 @@ static struct page *alloc_misplaced_dst_page(struct page *page, > >> struct page *newpage; > >> > >> newpage = alloc_pages_exact_node(nid, > >> - (GFP_HIGHUSER_MOVABLE | > >> - __GFP_THISNODE | __GFP_NOMEMALLOC | > >> - __GFP_NORETRY | __GFP_NOWARN) & > >> - ~GFP_IOFS, 0); > >> + (GFP_HIGHUSER_MOVABLE | __GFP_NOMEMALLOC | > >> + __GFP_NORETRY | __GFP_NOWARN) & ~GFP_IOFS, 0); > >> > >> return newpage; > >> } > > [snip] > > > > What about the alloc_pages_exact_node() in new_page_node()? > > Oops, seems I missed that one. So the API seems ok otherwise? > Yup! And I believe that this patch doesn't cause any regression after the new_page_node() issue is fixed.
WARNING: multiple messages have this Message-ID (diff)
From: David Rientjes <rientjes@google.com> To: Vlastimil Babka <vbabka@suse.cz> Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>, Mel Gorman <mgorman@suse.de>, Greg Thelen <gthelen@google.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, Christoph Lameter <cl@linux.com>, Pekka Enberg <penberg@kernel.org>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Subject: Re: [RFC v2 1/4] mm: make alloc_pages_exact_node pass __GFP_THISNODE Date: Fri, 24 Jul 2015 16:09:05 -0700 (PDT) [thread overview] Message-ID: <alpine.DEB.2.10.1507241606270.12744@chino.kir.corp.google.com> (raw) In-Reply-To: <55B2A596.1010101@suse.cz> On Fri, 24 Jul 2015, Vlastimil Babka wrote: > > I assume you looked at the collapse_huge_page() case and decided that it > > needs no modification since the gfp mask is used later for other calls? > > Yeah. Not that the memcg charge parts would seem to care about __GFP_THISNODE, > though. > Hmm, not sure that memcg would ever care about __GFP_THISNODE. I wonder if it make more sense to remove setting __GFP_THISNODE in collapse_huge_page()? khugepaged_alloc_page() seems fine with the new alloc_pages_exact_node() semantics. > >> diff --git a/mm/migrate.c b/mm/migrate.c > >> index f53838f..d139222 100644 > >> --- a/mm/migrate.c > >> +++ b/mm/migrate.c > >> @@ -1554,10 +1554,8 @@ static struct page *alloc_misplaced_dst_page(struct page *page, > >> struct page *newpage; > >> > >> newpage = alloc_pages_exact_node(nid, > >> - (GFP_HIGHUSER_MOVABLE | > >> - __GFP_THISNODE | __GFP_NOMEMALLOC | > >> - __GFP_NORETRY | __GFP_NOWARN) & > >> - ~GFP_IOFS, 0); > >> + (GFP_HIGHUSER_MOVABLE | __GFP_NOMEMALLOC | > >> + __GFP_NORETRY | __GFP_NOWARN) & ~GFP_IOFS, 0); > >> > >> return newpage; > >> } > > [snip] > > > > What about the alloc_pages_exact_node() in new_page_node()? > > Oops, seems I missed that one. So the API seems ok otherwise? > Yup! And I believe that this patch doesn't cause any regression after the new_page_node() issue is fixed. -- 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>
next prev parent reply other threads:[~2015-07-24 23:09 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-07-24 14:45 [RFC v2 1/4] mm: make alloc_pages_exact_node pass __GFP_THISNODE Vlastimil Babka 2015-07-24 14:45 ` Vlastimil Babka 2015-07-24 14:45 ` [RFC v2 2/4] mm: unify checks in alloc_pages_node family of functions Vlastimil Babka 2015-07-24 14:45 ` Vlastimil Babka 2015-07-24 20:09 ` David Rientjes 2015-07-24 20:09 ` David Rientjes 2015-07-24 14:45 ` [RFC v2 3/4] mm: use numa_mem_id in alloc_pages_node() Vlastimil Babka 2015-07-24 14:45 ` Vlastimil Babka 2015-07-24 20:09 ` David Rientjes 2015-07-24 20:09 ` David Rientjes 2015-07-29 13:31 ` Mel Gorman 2015-07-29 13:31 ` Mel Gorman 2015-07-24 14:45 ` [RFC v2 4/4] mm: fallback for offline nodes in alloc_pages_node Vlastimil Babka 2015-07-24 14:45 ` Vlastimil Babka 2015-07-24 15:48 ` Christoph Lameter 2015-07-24 15:48 ` Christoph Lameter 2015-07-24 19:54 ` David Rientjes 2015-07-24 19:54 ` David Rientjes 2015-07-24 20:39 ` Vlastimil Babka 2015-07-24 20:39 ` Vlastimil Babka 2015-07-24 23:06 ` David Rientjes 2015-07-24 23:06 ` David Rientjes 2015-07-27 11:29 ` Vlastimil Babka 2015-07-27 11:29 ` Vlastimil Babka 2015-07-24 20:08 ` [RFC v2 1/4] mm: make alloc_pages_exact_node pass __GFP_THISNODE David Rientjes 2015-07-24 20:08 ` David Rientjes 2015-07-24 20:52 ` Vlastimil Babka 2015-07-24 20:52 ` Vlastimil Babka 2015-07-24 23:09 ` David Rientjes [this message] 2015-07-24 23:09 ` David Rientjes 2015-07-27 15:39 ` Johannes Weiner 2015-07-27 15:39 ` Johannes Weiner 2015-07-27 15:47 ` Vlastimil Babka 2015-07-27 15:47 ` Vlastimil Babka 2015-07-29 13:30 ` Mel Gorman 2015-07-29 13:30 ` Mel Gorman 2015-07-30 14:33 ` Christoph Lameter 2015-07-30 14:33 ` Christoph Lameter 2015-07-30 15:14 ` Johannes Weiner 2015-07-30 15:14 ` Johannes Weiner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.DEB.2.10.1507241606270.12744@chino.kir.corp.google.com \ --to=rientjes@google.com \ --cc=akpm@linux-foundation.org \ --cc=aneesh.kumar@linux.vnet.ibm.com \ --cc=cl@linux.com \ --cc=gthelen@google.com \ --cc=iamjoonsoo.kim@lge.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@suse.de \ --cc=n-horiguchi@ah.jp.nec.com \ --cc=penberg@kernel.org \ --cc=vbabka@suse.cz \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.