Re: [PATCH v2 3/3] mm: return zero_resv_unavail optimization

[Masayoshi Mizuma <msys.mizuma@gmail.com>]

On Fri, Sep 28, 2018 at 12:19:44AM +0000, Naoya Horiguchi wrote:
> On Tue, Sep 25, 2018 at 11:35:32AM -0400, Masayoshi Mizuma wrote:
> > From: Pavel Tatashin <pavel.tatashin@microsoft.com>
> > 
> > When checking for valid pfns in zero_resv_unavail(), it is not necessary to
> > verify that pfns within pageblock_nr_pages ranges are valid, only the first
> > one needs to be checked. This is because memory for pages are allocated in
> > contiguous chunks that contain pageblock_nr_pages struct pages.
> > 
> > Signed-off-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
> > Reviewed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> 
> According to convention, review tag is formatted like "Reviewed-by: ...",

Sorry for the typo...

> Otherwise, looks good to me.
> 
> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

Thanks!

- Masa

> 
> > ---
> >  mm/page_alloc.c | 46 ++++++++++++++++++++++++++--------------------
> >  1 file changed, 26 insertions(+), 20 deletions(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 3b9d89e..bd5b7e4 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -6440,6 +6440,29 @@ void __init free_area_init_node(int nid, unsigned long *zones_size,
> >  }
> >  
> >  #if defined(CONFIG_HAVE_MEMBLOCK) && !defined(CONFIG_FLAT_NODE_MEM_MAP)
> > +
> > +/*
> > + * Zero all valid struct pages in range [spfn, epfn), return number of struct
> > + * pages zeroed
> > + */
> > +static u64 zero_pfn_range(unsigned long spfn, unsigned long epfn)
> > +{
> > +	unsigned long pfn;
> > +	u64 pgcnt = 0;
> > +
> > +	for (pfn = spfn; pfn < epfn; pfn++) {
> > +		if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages))) {
> > +			pfn = ALIGN_DOWN(pfn, pageblock_nr_pages)
> > +				+ pageblock_nr_pages - 1;
> > +			continue;
> > +		}
> > +		mm_zero_struct_page(pfn_to_page(pfn));
> > +		pgcnt++;
> > +	}
> > +
> > +	return pgcnt;
> > +}
> > +
> >  /*
> >   * Only struct pages that are backed by physical memory are zeroed and
> >   * initialized by going through __init_single_page(). But, there are some
> > @@ -6455,7 +6478,6 @@ void __init free_area_init_node(int nid, unsigned long *zones_size,
> >  void __init zero_resv_unavail(void)
> >  {
> >  	phys_addr_t start, end;
> > -	unsigned long pfn;
> >  	u64 i, pgcnt;
> >  	phys_addr_t next = 0;
> >  
> > @@ -6465,34 +6487,18 @@ void __init zero_resv_unavail(void)
> >  	pgcnt = 0;
> >  	for_each_mem_range(i, &memblock.memory, NULL,
> >  			NUMA_NO_NODE, MEMBLOCK_NONE, &start, &end, NULL) {
> > -		if (next < start) {
> > -			for (pfn = PFN_DOWN(next); pfn < PFN_UP(start); pfn++) {
> > -				if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages)))
> > -					continue;
> > -				mm_zero_struct_page(pfn_to_page(pfn));
> > -				pgcnt++;
> > -			}
> > -		}
> > +		if (next < start)
> > +			pgcnt += zero_pfn_range(PFN_DOWN(next), PFN_UP(start));
> >  		next = end;
> >  	}
> > -	for (pfn = PFN_DOWN(next); pfn < max_pfn; pfn++) {
> > -		if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages)))
> > -			continue;
> > -		mm_zero_struct_page(pfn_to_page(pfn));
> > -		pgcnt++;
> > -	}
> > -
> > +	pgcnt += zero_pfn_range(PFN_DOWN(next), max_pfn);
> >  
> >  	/*
> >  	 * Struct pages that do not have backing memory. This could be because
> >  	 * firmware is using some of this memory, or for some other reasons.
> > -	 * Once memblock is changed so such behaviour is not allowed: i.e.
> > -	 * list of "reserved" memory must be a subset of list of "memory", then
> > -	 * this code can be removed.
> >  	 */
> >  	if (pgcnt)
> >  		pr_info("Zeroed struct page in unavailable ranges: %lld pages", pgcnt);
> > -
> >  }
> >  #endif /* CONFIG_HAVE_MEMBLOCK && !CONFIG_FLAT_NODE_MEM_MAP */
> >  
> > -- 
> > 2.18.0
> > 
> >