Re: [PATCH 6/6] hugetlb: pass hugetlb reservation ranges in base-page indices

jane.chu
2026-04-15T19:40:30Z

On 4/15/2026 1:01 AM, Oscar Salvador wrote:
> On Thu, Apr 09, 2026 at 05:41:57PM -0600, Jane Chu wrote:
>> hugetlb_reserve_pages() consume indices in hugepage granularity although
>> some callers naturally compute offsets in PAGE_SIZE units.
>>
>> Teach the reservation helpers to accept base-page index ranges and
>> convert to hugepage indices internally before operating on the
>> reservation map. This keeps the internal representation unchanged while
>> making the API contract more uniform for callers.
>>
>> Update hugetlbfs and memfd call sites to pass base-page indices, and
>> adjust the documentation to describe the new calling convention. Add
>> alignment warnings in hugetlb_reserve_pages() to catch invalid ranges
>> early.
>>
>> No functional changes.
>>
>> Signed-off-by: Jane Chu <jane.chu@oracle.com>
>> ---
>> Documentation/mm/hugetlbfs_reserv.rst | 12 +++++------
>> fs/hugetlbfs/inode.c | 29 ++++++++++++---------------
>> mm/hugetlb.c | 26 ++++++++++++++++--------
>> mm/memfd.c | 9 +++++----
>> 4 files changed, 42 insertions(+), 34 deletions(-)
>>
>> diff --git a/Documentation/mm/hugetlbfs_reserv.rst b/Documentation/mm/hugetlbfs_reserv.rst
>> index a49115db18c7..60a52b28f0b4 100644
>> --- a/Documentation/mm/hugetlbfs_reserv.rst
>> +++ b/Documentation/mm/hugetlbfs_reserv.rst
>> @@ -112,8 +112,8 @@ flag was specified in either the shmget() or mmap() call. If NORESERVE
>> was specified, then this routine returns immediately as no reservations
>> are desired.
>>
>> -The arguments 'from' and 'to' are huge page indices into the mapping or
>> -underlying file. For shmget(), 'from' is always 0 and 'to' corresponds to
>> +The arguments 'from' and 'to' are base page indices into the mapping or
>> +underlying file. For shmget(), 'from' is always 0 and 'to' corresponds to
>> the length of the segment/mapping. For mmap(), the offset argument could
>> be used to specify the offset into the underlying file. In such a case,
>> the 'from' and 'to' arguments have been adjusted by this offset.
>> @@ -136,10 +136,10 @@ to indicate this VMA owns the reservations.
>>
>> The reservation map is consulted to determine how many huge page reservations
>> are needed for the current mapping/segment. For private mappings, this is
>> -always the value (to - from). However, for shared mappings it is possible that
>> -some reservations may already exist within the range (to - from). See the
>> -section :ref:`Reservation Map Modifications <resv_map_modifications>`
>> -for details on how this is accomplished.
>> +always the number of huge pages covered by the range [from, to). However,
>> +for shared mappings it is possible that some reservations may already exist
>> +within the range [from, to). See the section :ref:`Reservation Map Modifications
>> +<resv_map_modifications>` for details on how this is accomplished.
>>
>> The mapping may be associated with a subpool. If so, the subpool is consulted
>> to ensure there is sufficient space for the mapping. It is possible that the
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index a72d46ff7980..ec05ed30b70f 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -157,10 +157,8 @@ static int hugetlbfs_file_mmap_prepare(struct vm_area_desc *desc)
>> if (inode->i_flags & S_PRIVATE)
>> vma_flags_set(&vma_flags, VMA_NORESERVE_BIT);
>>
>> - if (hugetlb_reserve_pages(inode,
>> - desc->pgoff >> huge_page_order(h),
>> - len >> huge_page_shift(h), desc,
>> - vma_flags) < 0)
>> + if (hugetlb_reserve_pages(inode, desc->pgoff, len >> PAGE_SHIFT, desc,
>> + vma_flags) < 0)
>
> Ok, this is something that I have been thinking every time I looked
> into hugetlb reserve code, but I think we should be really starting to
> put some meaningful names for from and to, and pass that to
> hugetlb_reserve_pages.
> Because "desc->pgoff" and "len >> PAGE_SHIFT", meh, and it is not that
> many places we need to touch, but we might want in clarity.
> The same goes for hugetlb_unreserve_pages() of course.

indeed, will try to work on that in v2.
>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 47ef41b6fb2e..eb4ab5bd0c9f 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -6532,10 +6532,11 @@ long hugetlb_change_protection(struct vm_area_struct *vma,
>> }
> [...]
>> @@ -6558,6 +6560,12 @@ long hugetlb_reserve_pages(struct inode *inode,
>> return -EINVAL;
>> }
>>
>> + VM_WARN_ON(!IS_ALIGNED(from, 1UL << huge_page_order(h)));
>> + VM_WARN_ON(!IS_ALIGNED(to, 1UL << huge_page_order(h)));
>
> If we want to scream if someone passes us unaligned indices, we might
> want to do the same in hugetlb_unreserve_pages() ?

Sure.
>
>> diff --git a/mm/memfd.c b/mm/memfd.c
>> index 56c8833c4195..59c174c7533c 100644
>> --- a/mm/memfd.c
>> +++ b/mm/memfd.c
>> @@ -80,14 +80,15 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t index)
>> struct inode *inode = file_inode(memfd);
>> struct hstate *h = hstate_file(memfd);
>> long nr_resv;
>> - pgoff_t idx;
>> + pgoff_t next_index;
>> int err = -ENOMEM;
>>
>> gfp_mask = htlb_alloc_mask(h);
>> gfp_mask &= ~(__GFP_HIGHMEM | __GFP_MOVABLE);
>> - idx = index >> huge_page_order(h);
>> + next_index = index + pages_per_huge_page(h);
>
> Trailing white space.

My bad, should have checked.

Thanks!
-jane

>
>