Re: [PATCH] mm/page_alloc: call set_pageblock_order() once for each node

[Michal Hocko <mhocko@suse.com>]

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