Re: [PATCH] mm/page_alloc: optimize find_min_pfn_for_node() by geting the minimal pfn directly

[Michal Hocko <mhocko@kernel.org>]

On Wed 28-03-18 21:34:56, Wei Yang wrote:
> On Wed, Mar 28, 2018 at 01:58:53PM +0200, Michal Hocko wrote:
> >On Wed 28-03-18 11:47:52, Wei Yang wrote:
> >[...]
[...]
> >> @@ -6365,14 +6365,16 @@ unsigned long __init node_map_pfn_alignment(void)
> >>  /* Find the lowest pfn for a node */
> >>  static unsigned long __init find_min_pfn_for_node(int nid)
> >>  {
> >> -	unsigned long min_pfn = ULONG_MAX;
> >> -	unsigned long start_pfn;
> >> -	int i;
> >> +	unsigned long min_pfn;
> >> +	int i = -1;
> >>  
> >> -	for_each_mem_pfn_range(i, nid, &start_pfn, NULL, NULL)
> >> -		min_pfn = min(min_pfn, start_pfn);
> >> +	/*
> >> +	 * The first pfn on nid node is the minimal one, as the pfn's are
> >> +	 * stored in ascending order.
> >> +	 */
> >> +	first_mem_pfn(i, nid, &min_pfn);
> >>  
> >> -	if (min_pfn == ULONG_MAX) {
> >> +	if (i == -1) {
> >>  		pr_warn("Could not find start_pfn for node %d\n", nid);
> >>  		return 0;
> >>  	}
> >
> >I would just open code it. Other than that I strongly suspect this will
> >not have any measurable impact becauser we usually only have handfull of
> >memory ranges but why not. Just make the new implementation less ugly
> >than it is cuurrently - e.g. opencode first_mem_pfn and you can add
> 
> Open code here means use __next_mem_pfn_range() directly instead of using
> first_mem_pfn()?

Yes with the comment explaining how we rely on sorted ranges the way you
did.
-- 
Michal Hocko
SUSE Labs