Re: [PATCH 01/49] mm/sparse: fix vmemmap accounting imbalance on memory hotplug error

[Mike Rapoport]

On Mon, Apr 13, 2026 at 12:19:50PM +0300, Mike Rapoport wrote:
> On Sun, Apr 05, 2026 at 08:51:52PM +0800, Muchun Song wrote:
> > In section_activate(), if populate_section_memmap() fails, the error
> > handling path calls section_deactivate() to roll back the state. This
> > approach introduces an accounting imbalance.
> > 
> > Since the commit c3576889d87b ("mm: fix accounting of memmap pages"),
> > memmap pages are accounted for only after populate_section_memmap()
> > succeeds. However, section_deactivate() unconditionally decrements the
> > vmemmap account. Consequently, a failure in populate_section_memmap()
> > leads to a negative offset (underflow) in the system's vmemmap tracking.
> > 
> > We can fix this by ensuring that the vmemmap accounting is incremented
> > immediately before checking for the success of populate_section_memmap().
> > If populate_section_memmap() fails, the subsequent call to
> > section_deactivate() will decrement the accounting, perfectly offsetting
> > the increment and maintaining balance.
> > 
> > Fixes: c3576889d87b ("mm: fix accounting of memmap pages")
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > ---
> >  mm/sparse-vmemmap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> > index 6eadb9d116e4..ee27d0c0efe2 100644
> > --- a/mm/sparse-vmemmap.c
> > +++ b/mm/sparse-vmemmap.c
> > @@ -822,11 +822,11 @@ static struct page * __meminit section_activate(int nid, unsigned long pfn,
> >  		return pfn_to_page(pfn);
> >  
> >  	memmap = populate_section_memmap(pfn, nr_pages, nid, altmap, pgmap);
> > +	memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE));
> 
> This logically belongs to success path in populate_section_memmap(). If we
> update the counter there, we won't need to temporarily increase it at all.
 
Not strictly related to this patchset, but it seems, we can have a single
memmap_boot_pages_add() in memmap_alloc() rather than to update the counter
in memmap_alloc() callers.

> >  	if (!memmap) {
> >  		section_deactivate(pfn, nr_pages, altmap);
> >  		return ERR_PTR(-ENOMEM);
> >  	}
> > -	memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE));
> >  
> >  	return memmap;
> >  }
> > -- 
> > 2.20.1
> > 
> 
> -- 
> Sincerely yours,
> Mike.

-- 
Sincerely yours,
Mike.