* [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
@ 2018-03-29 3:36 Wei Yang
2018-03-29 12:11 ` Mel Gorman
0 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2018-03-29 3:36 UTC (permalink / raw)
To: akpm, mhocko, mgorman; +Cc: linux-mm, Wei Yang
set_pageblock_order() is a standalone function which sets pageblock_order,
while current implementation calls this function on each ZONE of each node
in free_area_init_core().
Since free_area_init_node() is the only user of free_area_init_core(),
this patch moves set_pageblock_order() up one level to invoke
set_pageblock_order() only once on each node.
Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
mm/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8c964dcc3a9e..828f5014b006 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6169,7 +6169,6 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
if (!size)
continue;
- set_pageblock_order();
setup_usemap(pgdat, zone, zone_start_pfn, size);
init_currently_empty_zone(zone, zone_start_pfn, size);
memmap_init(size, nid, j, zone_start_pfn);
@@ -6254,6 +6253,7 @@ void __paginginit free_area_init_node(int nid, unsigned long *zones_size,
alloc_node_mem_map(pgdat);
reset_deferred_meminit(pgdat);
+ set_pageblock_order();
free_area_init_core(pgdat);
}
--
2.15.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
2018-03-29 3:36 [PATCH] mm/page_alloc: call set_pageblock_order() once for each node Wei Yang
@ 2018-03-29 12:11 ` Mel Gorman
2018-03-30 1:02 ` Wei Yang
0 siblings, 1 reply; 7+ messages in thread
From: Mel Gorman @ 2018-03-29 12:11 UTC (permalink / raw)
To: Wei Yang; +Cc: akpm, mhocko, linux-mm
On Thu, Mar 29, 2018 at 11:36:07AM +0800, Wei Yang wrote:
> set_pageblock_order() is a standalone function which sets pageblock_order,
> while current implementation calls this function on each ZONE of each node
> in free_area_init_core().
>
> Since free_area_init_node() is the only user of free_area_init_core(),
> this patch moves set_pageblock_order() up one level to invoke
> set_pageblock_order() only once on each node.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
The patch looks ok but given that set_pageblock_order returns immediately
if it has already been called, I expect the benefit is marginal. Was any
improvement in boot time measured?
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
2018-03-29 12:11 ` Mel Gorman
@ 2018-03-30 1:02 ` Wei Yang
2018-04-03 7:57 ` Michal Hocko
0 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2018-03-30 1:02 UTC (permalink / raw)
To: Mel Gorman; +Cc: Wei Yang, akpm, mhocko, linux-mm
On Thu, Mar 29, 2018 at 01:11:09PM +0100, Mel Gorman wrote:
>On Thu, Mar 29, 2018 at 11:36:07AM +0800, Wei Yang wrote:
>> set_pageblock_order() is a standalone function which sets pageblock_order,
>> while current implementation calls this function on each ZONE of each node
>> in free_area_init_core().
>>
>> Since free_area_init_node() is the only user of free_area_init_core(),
>> this patch moves set_pageblock_order() up one level to invoke
>> set_pageblock_order() only once on each node.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>The patch looks ok but given that set_pageblock_order returns immediately
>if it has already been called, I expect the benefit is marginal. Was any
>improvement in boot time measured?
No, I don't expect measurable improvement from this since the number of nodes
and zones are limited.
This is just a code refine from logic point of view.
>
>--
>Mel Gorman
>SUSE Labs
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
2018-03-30 1:02 ` Wei Yang
@ 2018-04-03 7:57 ` Michal Hocko
2018-04-04 1:27 ` Wei Yang
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2018-04-03 7:57 UTC (permalink / raw)
To: Wei Yang; +Cc: Mel Gorman, akpm, linux-mm
On Fri 30-03-18 09:02:43, Wei Yang wrote:
> On Thu, Mar 29, 2018 at 01:11:09PM +0100, Mel Gorman wrote:
> >On Thu, Mar 29, 2018 at 11:36:07AM +0800, Wei Yang wrote:
> >> set_pageblock_order() is a standalone function which sets pageblock_order,
> >> while current implementation calls this function on each ZONE of each node
> >> in free_area_init_core().
> >>
> >> Since free_area_init_node() is the only user of free_area_init_core(),
> >> this patch moves set_pageblock_order() up one level to invoke
> >> set_pageblock_order() only once on each node.
> >>
> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> >
> >The patch looks ok but given that set_pageblock_order returns immediately
> >if it has already been called, I expect the benefit is marginal. Was any
> >improvement in boot time measured?
>
> No, I don't expect measurable improvement from this since the number of nodes
> and zones are limited.
>
> This is just a code refine from logic point of view.
Then, please make sure it is a real refinement. Calling this function
per node is only half way to get there as the function is by no means
per node.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
2018-04-03 7:57 ` Michal Hocko
@ 2018-04-04 1:27 ` Wei Yang
2018-04-05 9:55 ` Michal Hocko
0 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2018-04-04 1:27 UTC (permalink / raw)
To: Michal Hocko; +Cc: Wei Yang, Mel Gorman, akpm, linux-mm
On Tue, Apr 03, 2018 at 09:57:37AM +0200, Michal Hocko wrote:
>On Fri 30-03-18 09:02:43, Wei Yang wrote:
>> On Thu, Mar 29, 2018 at 01:11:09PM +0100, Mel Gorman wrote:
>> >On Thu, Mar 29, 2018 at 11:36:07AM +0800, Wei Yang wrote:
>> >> set_pageblock_order() is a standalone function which sets pageblock_order,
>> >> while current implementation calls this function on each ZONE of each node
>> >> in free_area_init_core().
>> >>
>> >> Since free_area_init_node() is the only user of free_area_init_core(),
>> >> this patch moves set_pageblock_order() up one level to invoke
>> >> set_pageblock_order() only once on each node.
>> >>
>> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> >
>> >The patch looks ok but given that set_pageblock_order returns immediately
>> >if it has already been called, I expect the benefit is marginal. Was any
>> >improvement in boot time measured?
>>
>> No, I don't expect measurable improvement from this since the number of nodes
>> and zones are limited.
>>
>> This is just a code refine from logic point of view.
>
>Then, please make sure it is a real refinement. Calling this function
>per node is only half way to get there as the function is by no means
>per node.
>
Hi, Michal
I guess you are willing to see this function is only called once for the whole
system.
Yes, that is the ideal way, well I don't come up with an elegant way. The best
way is to move this to free_area_init_nodes(), while you can see not all arch
use this function.
Then I have two options:
A: Move this to free_area_init_nodes() for those arch using it. Call it
specifically for those arch not using free_area_init_nodes().
B: call it before setup_arch() in start_kernel()
Hmm... which one you would prefer? If you have a better idea, that would be
great.
>--
>Michal Hocko
>SUSE Labs
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
2018-04-04 1:27 ` Wei Yang
@ 2018-04-05 9:55 ` Michal Hocko
2018-04-06 1:46 ` Wei Yang
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2018-04-05 9:55 UTC (permalink / raw)
To: Wei Yang; +Cc: Mel Gorman, akpm, linux-mm
On Wed 04-04-18 09:27:34, Wei Yang wrote:
> On Tue, Apr 03, 2018 at 09:57:37AM +0200, Michal Hocko wrote:
> >On Fri 30-03-18 09:02:43, Wei Yang wrote:
> >> On Thu, Mar 29, 2018 at 01:11:09PM +0100, Mel Gorman wrote:
> >> >On Thu, Mar 29, 2018 at 11:36:07AM +0800, Wei Yang wrote:
> >> >> set_pageblock_order() is a standalone function which sets pageblock_order,
> >> >> while current implementation calls this function on each ZONE of each node
> >> >> in free_area_init_core().
> >> >>
> >> >> Since free_area_init_node() is the only user of free_area_init_core(),
> >> >> this patch moves set_pageblock_order() up one level to invoke
> >> >> set_pageblock_order() only once on each node.
> >> >>
> >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> >> >
> >> >The patch looks ok but given that set_pageblock_order returns immediately
> >> >if it has already been called, I expect the benefit is marginal. Was any
> >> >improvement in boot time measured?
> >>
> >> No, I don't expect measurable improvement from this since the number of nodes
> >> and zones are limited.
> >>
> >> This is just a code refine from logic point of view.
> >
> >Then, please make sure it is a real refinement. Calling this function
> >per node is only half way to get there as the function is by no means
> >per node.
> >
>
> Hi, Michal
>
> I guess you are willing to see this function is only called once for the whole
> system.
>
> Yes, that is the ideal way, well I don't come up with an elegant way. The best
> way is to move this to free_area_init_nodes(), while you can see not all arch
> use this function.
>
> Then I have two options:
>
> A: Move this to free_area_init_nodes() for those arch using it. Call it
> specifically for those arch not using free_area_init_nodes().
>
> B: call it before setup_arch() in start_kernel()
>
> Hmm... which one you would prefer? If you have a better idea, that would be
> great.
or
C: do nothing and/or just document why do we call it this way
(convenience).
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node
2018-04-05 9:55 ` Michal Hocko
@ 2018-04-06 1:46 ` Wei Yang
0 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2018-04-06 1:46 UTC (permalink / raw)
To: Michal Hocko; +Cc: Wei Yang, Mel Gorman, akpm, linux-mm
On Thu, Apr 05, 2018 at 11:55:50AM +0200, Michal Hocko wrote:
>On Wed 04-04-18 09:27:34, Wei Yang wrote:
>> On Tue, Apr 03, 2018 at 09:57:37AM +0200, Michal Hocko wrote:
>> >On Fri 30-03-18 09:02:43, Wei Yang wrote:
>> >> On Thu, Mar 29, 2018 at 01:11:09PM +0100, Mel Gorman wrote:
>> >> >On Thu, Mar 29, 2018 at 11:36:07AM +0800, Wei Yang wrote:
>> >> >> set_pageblock_order() is a standalone function which sets pageblock_order,
>> >> >> while current implementation calls this function on each ZONE of each node
>> >> >> in free_area_init_core().
>> >> >>
>> >> >> Since free_area_init_node() is the only user of free_area_init_core(),
>> >> >> this patch moves set_pageblock_order() up one level to invoke
>> >> >> set_pageblock_order() only once on each node.
>> >> >>
>> >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> >> >
>> >> >The patch looks ok but given that set_pageblock_order returns immediately
>> >> >if it has already been called, I expect the benefit is marginal. Was any
>> >> >improvement in boot time measured?
>> >>
>> >> No, I don't expect measurable improvement from this since the number of nodes
>> >> and zones are limited.
>> >>
>> >> This is just a code refine from logic point of view.
>> >
>> >Then, please make sure it is a real refinement. Calling this function
>> >per node is only half way to get there as the function is by no means
>> >per node.
>> >
>>
>> Hi, Michal
>>
>> I guess you are willing to see this function is only called once for the whole
>> system.
>>
>> Yes, that is the ideal way, well I don't come up with an elegant way. The best
>> way is to move this to free_area_init_nodes(), while you can see not all arch
>> use this function.
>>
>> Then I have two options:
>>
>> A: Move this to free_area_init_nodes() for those arch using it. Call it
>> specifically for those arch not using free_area_init_nodes().
>>
>> B: call it before setup_arch() in start_kernel()
>>
>> Hmm... which one you would prefer? If you have a better idea, that would be
>> great.
>
>or
>C: do nothing and/or just document why do we call it this way
>(convenience).
Ok, maybe we can leave it alone now.
>--
>Michal Hocko
>SUSE Labs
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-04-06 1:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-29 3:36 [PATCH] mm/page_alloc: call set_pageblock_order() once for each node Wei Yang
2018-03-29 12:11 ` Mel Gorman
2018-03-30 1:02 ` Wei Yang
2018-04-03 7:57 ` Michal Hocko
2018-04-04 1:27 ` Wei Yang
2018-04-05 9:55 ` Michal Hocko
2018-04-06 1:46 ` Wei Yang
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.