On Tue, Mar 04, 2025 at 09:21:06PM +0800, Jinjiang Tu wrote:
> In dissolve_free_huge_page(), free huge pages are dissolved without
> adjusting surplus count. However, free huge pages may be accounted as
> surplus pages, and will lead to wrong surplus count.
>
> I reproduce this issue on qemu. The steps are:
> 1) Node1 is memory-less at first. Hot-add memory to node1 by executing
> the two commands in qemu monitor:
> object_add memory-backend-ram,id=mem1,size=1G
> device_add pc-dimm,id=dimm1,memdev=mem1,node=1
> 2) online one memory block of Node1 with：
> echo online_movable > /sys/devices/system/node/node1/memoryX/state
> 3) create 64 huge pages for node1
> 4) run a program to reserve (don't consume) all the huge pages
> 5) echo 0 > nr_huge_pages for node1. After this step, free huge pages in
> Node1 are surplus.
> 6) create 80 huge pages for node0
> 7) offline memory of node1, The memory range to offline contains the free
> surplus huge pages created in step3) ~ step5)
> echo offline > /sys/devices/system/node/node1/memoryX/state
> 8) kill the program in step 4)
>
> The result:
> Node0 Node1
> total 80 0
> free 80 0
> surplus 0 61
>
> To fix it, adjust surplus when destroying huge pages if the node has
> surplus pages in dissolve_free_hugetlb_folio().
>
> The result with this patch:
> Node0 Node1
> total 80 0
> free 80 0
> surplus 0 0
>
> Fixes: c8721bbbdd36 ("mm: memory-hotplug: enable memory hotplug to handle hugepage")
> Acked-by: David Hildenbrand <david@xxxxxxxxxx>
> Signed-off-by: Jinjiang Tu <tujinjiang@xxxxxxxxxx>
Acked-by: Oscar Salvador <osalvador@xxxxxxx>
> @@ -2157,7 +2159,9 @@ int dissolve_free_hugetlb_folio(struct folio *folio)
> goto retry;
> }
>
> - remove_hugetlb_folio(h, folio, false);
> + if (h->surplus_huge_pages_node[folio_nid(folio)])
> + adjust_surplus = true;
> + remove_hugetlb_folio(h, folio, adjust_surplus);
> h->max_huge_pages--;
> spin_unlock_irq(&hugetlb_lock);
>
> @@ -2177,7 +2181,7 @@ int dissolve_free_hugetlb_folio(struct folio *folio)
> rc = hugetlb_vmemmap_restore_folio(h, folio);
> if (rc) {
> spin_lock_irq(&hugetlb_lock);
> - add_hugetlb_folio(h, folio, false);
> + add_hugetlb_folio(h, folio, adjust_surplus);
I was about to point this out, but checking v1 I saw that David already
that.
My alternative would have been to just get rid of the adjust_surplus
boolean and to the checking right within the lock cycle e.g:
if (h->surplus_huge_pages_node[folio_nid(folio)])
add_hugetlb_folio(h, folio, true);
else
add_hugetlb_folio(h, folio, false);
But I guess that's fine as you already explained.
--
Oscar Salvador
SUSE Labs
