* [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node @ 2016-05-25 21:00 Yang Shi 2016-05-25 22:23 ` Andrew Morton 0 siblings, 1 reply; 4+ messages in thread From: Yang Shi @ 2016-05-25 21:00 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, linux-mm, linaro-kernel, yang.shi register_page_bootmem_info_node() is invoked in mem_init(), so it will be called before page_alloc_init_late() if CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled. But, pfn_to_nid() depends on memmap which won't be fully setup until page_alloc_init_late() is done, so replace pfn_to_nid() by early_pfn_to_nid(). Signed-off-by: Yang Shi <yang.shi@linaro.org> --- mm/memory_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index caf2a14..b8ee080 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -300,7 +300,7 @@ void register_page_bootmem_info_node(struct pglist_data *pgdat) * multiple nodes we check that this pfn does not already * reside in some other nodes. */ - if (pfn_valid(pfn) && (pfn_to_nid(pfn) == node)) + if (pfn_valid(pfn) && (early_pfn_to_nid(pfn) == node)) register_page_bootmem_info_section(pfn); } } -- 2.0.2 -- 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> ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node 2016-05-25 21:00 [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node Yang Shi @ 2016-05-25 22:23 ` Andrew Morton 2016-05-25 22:36 ` Shi, Yang 0 siblings, 1 reply; 4+ messages in thread From: Andrew Morton @ 2016-05-25 22:23 UTC (permalink / raw) To: Yang Shi; +Cc: linux-kernel, linux-mm, linaro-kernel On Wed, 25 May 2016 14:00:07 -0700 Yang Shi <yang.shi@linaro.org> wrote: > register_page_bootmem_info_node() is invoked in mem_init(), so it will be > called before page_alloc_init_late() if CONFIG_DEFERRED_STRUCT_PAGE_INIT > is enabled. But, pfn_to_nid() depends on memmap which won't be fully setup > until page_alloc_init_late() is done, so replace pfn_to_nid() by > early_pfn_to_nid(). What are the runtime effects of this fix? -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node 2016-05-25 22:23 ` Andrew Morton @ 2016-05-25 22:36 ` Shi, Yang 2016-05-26 0:37 ` Joonsoo Kim 0 siblings, 1 reply; 4+ messages in thread From: Shi, Yang @ 2016-05-25 22:36 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, linux-mm, linaro-kernel On 5/25/2016 3:23 PM, Andrew Morton wrote: > On Wed, 25 May 2016 14:00:07 -0700 Yang Shi <yang.shi@linaro.org> wrote: > >> register_page_bootmem_info_node() is invoked in mem_init(), so it will be >> called before page_alloc_init_late() if CONFIG_DEFERRED_STRUCT_PAGE_INIT >> is enabled. But, pfn_to_nid() depends on memmap which won't be fully setup >> until page_alloc_init_late() is done, so replace pfn_to_nid() by >> early_pfn_to_nid(). > > What are the runtime effects of this fix? I didn't experience any problem without the fix. During working on the page_ext_init() fix (replace to early_pfn_to_nid()), I added printk before each pfn_to_nid() calls to check which one might be called before page_alloc_init_late(), then this one is caught. From the code perspective, it sounds not right since register_page_bootmem_info_section() may miss some pfns when CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, just like the problem happened in page_ext_init(). Thanks, Yang > -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node 2016-05-25 22:36 ` Shi, Yang @ 2016-05-26 0:37 ` Joonsoo Kim 0 siblings, 0 replies; 4+ messages in thread From: Joonsoo Kim @ 2016-05-26 0:37 UTC (permalink / raw) To: Shi, Yang Cc: Andrew Morton, linux-kernel, linux-mm, linaro-kernel, Mel Gorman Ccing Mel. On Wed, May 25, 2016 at 03:36:48PM -0700, Shi, Yang wrote: > On 5/25/2016 3:23 PM, Andrew Morton wrote: > >On Wed, 25 May 2016 14:00:07 -0700 Yang Shi <yang.shi@linaro.org> wrote: > > > >>register_page_bootmem_info_node() is invoked in mem_init(), so it will be > >>called before page_alloc_init_late() if CONFIG_DEFERRED_STRUCT_PAGE_INIT > >>is enabled. But, pfn_to_nid() depends on memmap which won't be fully setup > >>until page_alloc_init_late() is done, so replace pfn_to_nid() by > >>early_pfn_to_nid(). > > > >What are the runtime effects of this fix? > > I didn't experience any problem without the fix. During working on > the page_ext_init() fix (replace to early_pfn_to_nid()), I added > printk before each pfn_to_nid() calls to check which one might be > called before page_alloc_init_late(), then this one is caught. > > From the code perspective, it sounds not right since > register_page_bootmem_info_section() may miss some pfns when > CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, just like the problem > happened in page_ext_init(). Hello, Mel. There was an issue in page_ext [1] due to your deferred struct page init feature. Before your change, we assumed that we can use pfn_to_nid() after memmap init is called. But, after your change, we can use pfn_to_nid() after page_alloc_init_late(). Yang found two call sites that uses pfn_to_nid() before page_alloc_init_late() and they could be fixed by using early_pfn_to_nid(). I guess that there are more problems due to this change so it's better to check it by patch author. One thing I have noticed is that dirty_limit could be set wrongly. It is intialized by using freepage count. Since it is intialized before page_alloc_init_late(), freepages are not initialized yet and it could be wrong. If my analysis is correct, please fix it. And, could you check again that there is no more problem? Thanks. [1] http://lkml.kernel.org/r/CAAmzW4OUmyPwQjvd7QUfc6W1Aic__TyAuH80MLRZNMxKy0-wPQ@mail.gmail.com -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-26 0:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-25 21:00 [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node Yang Shi 2016-05-25 22:23 ` Andrew Morton 2016-05-25 22:36 ` Shi, Yang 2016-05-26 0:37 ` Joonsoo Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).