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

Mike Rapoport
2026-04-13T17:20:21Z

On Mon, Apr 13, 2026 at 10:16:20PM +0800, Muchun Song wrote:
> >> +++ b/mm/sparse-vmemmap.c
> >> @@ -656,7 +656,13 @@ static struct page * __meminit populate_section_memmap(unsigned long pfn,
> >> unsigned long nr_pages, int nid, struct vmem_altmap *altmap,
> >> struct dev_pagemap *pgmap)
> >> {
> >> - return __populate_section_memmap(pfn, nr_pages, nid, altmap, pgmap);
> >> + struct page *p = __populate_section_memmap(pfn, nr_pages, nid, altmap,
> >> + pgmap);
> >> +
> >> + if (p)
> >> + memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE));
> >
> > We don’t increase the counter on failure, then
> >
> >> +
> >> + return p;
> >> }
> >>
> >> static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
> >> @@ -826,7 +832,6 @@ static struct page * __meminit section_activate(int nid, unsigned long pfn,
> >> section_deactivate(pfn, nr_pages, altmap);
> >
> > here section_deactivate() is called, which decrease the counter unconditionally,
> > the issue still exists. We didn't fix anything.

You are right, I missed it :/

> >> return ERR_PTR(-ENOMEM);
> >> }
> >> - memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE));
> >>
> >> return memmap;
> >> }
> >>
> >>
> >> Then we'll better follow "all or nothing" principle and won't have
> >> exceptional cases in section_deactivate().
>
> To follow "all or nothing" principle here, I think we should not call
> section_deactivate() to do the cleanup in section_activate().
>
> After all, if section_activate() didn't succeed, how can we use
> section_deactivate() to release the resources? What do you think?

Yeah, if we could pull "upopulate" from section_deactivate() that would be
great.

> Thanks.

--
Sincerely yours,
Mike.