* [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node
@ 2016-05-25 21:00 ` Yang Shi
0 siblings, 0 replies; 8+ 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node
@ 2016-05-25 21:00 ` Yang Shi
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [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
-1 siblings, 0 replies; 8+ 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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node
@ 2016-05-25 22:23 ` Andrew Morton
0 siblings, 0 replies; 8+ 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] 8+ 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
-1 siblings, 0 replies; 8+ 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node
@ 2016-05-25 22:36 ` Shi, Yang
0 siblings, 0 replies; 8+ 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] 8+ 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
-1 siblings, 0 replies; 8+ 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mm: use early_pfn_to_nid in register_page_bootmem_info_node
@ 2016-05-26 0:37 ` Joonsoo Kim
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2016-05-26 0:36 UTC | newest]
Thread overview: 8+ 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 21:00 ` Yang Shi
2016-05-25 22:23 ` Andrew Morton
2016-05-25 22:23 ` Andrew Morton
2016-05-25 22:36 ` Shi, Yang
2016-05-25 22:36 ` Shi, Yang
2016-05-26 0:37 ` Joonsoo Kim
2016-05-26 0:37 ` Joonsoo Kim
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.