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

[Muchun Song <muchun.song@linux.dev>]

> On Apr 13, 2026, at 17:35, Mike Rapoport <rppt@kernel.org> wrote:
> 
> 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.

It will indeed become simpler and is a good cleanup direction, but there
is a slight change in semantics: the page tables used for vmemmap page
mapping will also be counted in memmap_boot_pages_add(). This might not
be an issue (after all, the size of the page tables is very small compared
to struct pages, right?).

Additionally, I still lean toward making no changes to this patch, because
this is a pure bugfix patch — of course, it is meant to facilitate backporting
for those who need it. The cleanup would involve many more changes, so I
prefer to do that in a separate patch. What do you think?

Thanks,
Muchun.

> 
>>> 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.