Re: [PATCH v2] mm/hugetlb: fix set_max_huge_pages() when there are surplus pages

[Oscar Salvador <osalvador@suse.de>]

On Tue, Apr 01, 2025 at 04:23:39PM +0800, Jinjiang Tu wrote:
> In set_max_huge_pages(), min_count should mean the acquired persistent
> huge pages, but it contains surplus huge pages. It will leads to failing
> to freeing free huge pages for a Node.
> 
> Steps to reproduce:
> 1) create 5 hugetlb folios in Node0
> 2) run a program to use all the hugetlb folios
> 3) echo 0 > nr_hugepages for Node0 to free the hugetlb folios. Thus the 5
> hugetlb folios in Node0 are accounted as surplus.
> 4) create 5 hugetlb folios in Node1
> 5) echo 0 > nr_hugepages for Node1 to free the hugetlb folios

> 
> The result:
>         Node0    Node1
> Total     5         5
> Free      0         5
> Surp      5         5
> 
> We couldn't subtract surplus_huge_pages from min_mount, since free hugetlb
> folios may be surplus due to HVO. In __update_and_free_hugetlb_folio(),
> hugetlb_vmemmap_restore_folio() may fail, add the folio back to pool and
> treat it as surplus. If we directly subtract surplus_huge_pages from
> min_mount, some free folios will be subtracted twice.
> 
> To fix it, check if count is less than the num of free huge pages that
> could be destroyed (i.e., available_huge_pages(h)), and remove hugetlb
> folios if so.
> 
> Since there may exist free surplus hugetlb folios, we should remove
> surplus folios first to make surplus count correct.
> 
> The result with this patch:
>         Node0    Node1
> Total     5         0
> Free      0         0
> Surp      5         0

Unfortunately, this will not fly.


Assume this:

# echo 3 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
# numactl -m 0 ./hugetlb_use_2_pages &
# echo 2 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages

Because now you're checking for available_huge_pages in the
remove_pool_hugetlb_folio block, you will fail to free that page, and
will be accounted as surplus, thus failing to reduce the pool.

Of course, once the program terminates, all will be cleaned up, but in
the meantime that page (or more, depending on your set up) will not be
freed into the buddy allocator, which is unexpected.

And there are other scenarios where the same will happen.

So, it seems to me that we have to find a more deterministic approach.
The whole operation is almost entirely covered with the lock, other than
when we free the pages we collected, so I am sure we can find a way that
does not need special casing things.


-- 
Oscar Salvador
SUSE Labs