Re: [PATCH v2 6/6] mm: hugetlb: Refactor out hugetlb_alloc_folio()

[Ackerley Tng]

"Oscar Salvador (SUSE)" <osalvador@xxxxxxxxxx> writes:

> On Tue, May 12, 2026 at 03:25:30PM +0200, Oscar Salvador wrote:
>> On Wed, May 06, 2026 at 08:54:42AM -0700, Ackerley Tng via B4 Relay wrote:
>> > From: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>> >
>> > Refactor out hugetlb_alloc_folio() from alloc_hugetlb_folio(), which
>> > handles allocation of a folio and memory and HugeTLB charging to cgroups.
>> >
>> > This refactoring decouples the HugeTLB page allocation from VMAs,
>> > specifically:
>> >
>> > 1. Reservations (as in resv_map) are stored in the vma
>> > 2. mpol is stored at vma->vm_policy
>> > 3. A vma must be used for allocation even if the pages are not meant to be
>> >    used by host process.
>> >
>> > Without this coupling, VMAs are no longer a requirement for
>> > allocation. This opens up the allocation routine for usage without VMAs,
>> > which will allow guest_memfd to use HugeTLB as a more generic allocator of
>> > huge pages, since guest_memfd memory may not have any associated VMAs by
>> > design. In addition, direct allocations from HugeTLB could possibly be
>> > refactored to avoid the use of a pseudo-VMA.
>> >
>> > Also, this decouples HugeTLB page allocation from HugeTLBfs, where the
>> > subpool is stored at the fs mount. This is also a requirement for
>> > guest_memfd, where the plan is to have a subpool created per-fd and stored
>> > on the inode.
>> >
>> > No functional change intended.
>> >
>> > Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>>
>> I yet have to review more thoroughly, but I have a comment below:
>
> So, I thought about this some more and here it is what I came up with
>
> - Ideally this new hugetlb_alloc_folio() function should be as generic
>   as possible to try to fit other users in the future

Yes, glad you agree!

> - I would create a ctxt struct to pass all the parameters

This is a good idea, I will do this in the next revision.

> - charge_hugetlb_cgroup_rsvd and use_global_reservation could be a flags
>   thing (action_flags?) within the ctxt struct. We might want to add
>   more flags in the future to tweak the allocator behaviour.

I'll give this a shot in the next revision.

> - Ideally gfp_t mask should be created in hugetlb_alloc_folio() and tweak it in
>   there before being passed down the road, which means do
>
>   gfp_t gfp = gfp_mask & ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL)
>
>   in hugetlb_alloc_folio() instead of doing it in alloc_buddy_hugetlb_folio_with_mpol()
>

I think gfp can be in the ctxt passed into hugetlb_alloc_folio(), but
alloc_buddy_hugetlb_folio_with_mpol() has a different gfp from
dequeue_hugetlb_folio_with_mpol().

dequeue_hugetlb_folio_with_mpol() just uses gfp to select a zone to
dequeue from, but alloc_buddy_hugetlb_folio_with_mpol() is actually
doing allocation. Later in hugetlb_alloc_folio(),
mem_cgroup_charge_hugetlb() uses (gfp | __GFP_RETRY_MAYFAIL).

In the next revision I'll put gfp in the ctxt struct, which will just be
htlb_alloc_mask(h), and I think the other functions should modify gfp as
required from the ctxt.

>   As of right now, we define it in four different places:
>   hugetlb_alloc_folio, alloc_hugetlb_folio, dequeue_hugetlb_folio_with_mpol, and
>   alloc_buddy_hugetlb_folio_with_mpol.
>
> - I think we could strip _mpol from both   alloc_buddy_hugetlb_folio_with_mpol and
>   dequeue_hugetlb_folio_with_mpol, and pass a boolean "node_preferred_many".
>

Will do.

>
> I am probably missing something but I cannot remember it right now.
>
>
>
> --
> Oscar Salvador
> SUSE Labs