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

[jane.chu@oracle.com]

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

> 
>