Re: [PATCH] mm/sparse: Consistently do not zero memmap

[Vincent Whitchurch <vincent.whitchurch@axis.com>]

On Thu, Oct 31, 2019 at 08:25:55AM +0100, Michal Hocko wrote:
> On Wed 30-10-19 18:31:23, Michal Hocko wrote:
> [...]
> > What about this? It still aligns to the size but that should be
> > correctly done to the section size level.
> > 
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index 72f010d9bff5..ab1e6175ac9a 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -456,8 +456,7 @@ struct page __init *__populate_section_memmap(unsigned long pfn,
> >  	if (map)
> >  		return map;
> >  
> > -	map = memblock_alloc_try_nid(size,
> > -					  PAGE_SIZE, addr,
> > +	map = memblock_alloc_try_nid(size, size, addr,
> >  					  MEMBLOCK_ALLOC_ACCESSIBLE, nid);
> >  	if (!map)
> >  		panic("%s: Failed to allocate %lu bytes align=0x%lx nid=%d from=%pa\n",
> > @@ -474,8 +473,13 @@ static void __init sparse_buffer_init(unsigned long size, int nid)
> >  {
> >  	phys_addr_t addr = __pa(MAX_DMA_ADDRESS);
> >  	WARN_ON(sparsemap_buf);	/* forgot to call sparse_buffer_fini()? */
> > +	/*
> > +	 * Pre-allocated buffer is mainly used by __populate_section_memmap
> > +	 * and we want it to be properly aligned to the section size - this is
> > +	 * especially the case for VMEMMAP which maps memmap to PMDs
> > +	 */
> >  	sparsemap_buf =
> > -		memblock_alloc_try_nid_raw(size, PAGE_SIZE,
> > +		memblock_alloc_try_nid_raw(size, section_map_size(),
> >  						addr,
> >  						MEMBLOCK_ALLOC_ACCESSIBLE, nid);
> >  	sparsemap_buf_end = sparsemap_buf + size;
>
> Vincent, could you give this a try please? It would be even better if
> you could add some debugging to measure the overhead. Let me know if you
> need any help with a debugging patch.

I've tested this patch and it works on my platform:  The allocations
from sparse_buffer_alloc() now succeed and the fallback path is not
taken.

I'm not sure what kind of overhead you're interested in.  The memory
overhead of the initial allocations (as measured via the "Memory: XX/YY
available" prints and MemTotal), is identical with and without this
patch.  I don't see any difference in the time taken for the
initializations either.