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

Mike Rapoport
2026-04-13T09:35:26Z

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@bytedance.com>
> > ---
> > 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.