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

From: Mike Rapoport <[email-masked]>
Date: Mon, 13 Apr 2026 20:20:07 +0300

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.