Re: [PATCH 3/6] hugetlb: make hugetlb_fault_mutex_hash() take PAGE_SIZE index

jane.chu
2026-04-13T21:33:06Z

On 4/13/2026 10:43 AM, Oscar Salvador wrote:
> On Thu, Apr 09, 2026 at 05:41:54PM -0600, Jane Chu wrote:
>> hugetlb_fault_mutex_hash() is used to serialize faults and page cache
>> operations on the same hugetlb file offset. The helper currently expects
>> its index argument in hugetlb page granularity, so callers have to
>> open-code conversions from the PAGE_SIZE-based indices commonly used
>> in the rest of MM helpers.
>>
>> Change hugetlb_fault_mutex_hash() to take a PAGE_SIZE-based index
>> instead, and perform the hugetlb-granularity conversion inside the helper.
>> Update all callers accordingly.
>>
>> This makes the helper interface consistent with filemap_get_folio(),
>> and linear_page_index(), while preserving the same lock selection for
>> a given hugetlb file offset.
>>
>> Signed-off-by: Jane Chu <jane.chu@oracle.com>
>> ---
>> fs/hugetlbfs/inode.c | 19 ++++++++++---------
>> mm/hugetlb.c | 28 +++++++++++++++++++---------
>> mm/memfd.c | 11 ++++++-----
>> mm/userfaultfd.c | 7 +++----
>> 4 files changed, 38 insertions(+), 27 deletions(-)
>>
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index cf79fb830377..e24e9bf54e14 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -575,7 +575,7 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
>> struct address_space *mapping = &inode->i_data;
>> const pgoff_t end = lend >> PAGE_SHIFT;
>> struct folio_batch fbatch;
>> - pgoff_t next, index;
>> + pgoff_t next, idx;
>> int i, freed = 0;
>> bool truncate_op = (lend == LLONG_MAX);
>>
>> @@ -586,15 +586,15 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
>> struct folio *folio = fbatch.folios[i];
>> u32 hash = 0;
>>
>> - index = folio->index >> huge_page_order(h);
>> - hash = hugetlb_fault_mutex_hash(mapping, index);
>> + hash = hugetlb_fault_mutex_hash(mapping, folio->index);
>> mutex_lock(&hugetlb_fault_mutex_table[hash]);
>>
>> /*
>> * Remove folio that was part of folio_batch.
>> */
>> + idx = folio->index >> huge_page_order(h);
>> remove_inode_single_folio(h, inode, mapping, folio,
>> - index, truncate_op);
>> + idx, truncate_op);
>
> Since this is the only place we call remove_inode_single_folio(), and that we do not
> the index (at least index >> huge_page_order()) directly in this function, would it not be
> better to make remove_inode_single_folio do the conversion itself?

In PATCH 6/6, remove_inode_hugepages() is changed to call
remove_inode_single_folio() passing "folio->index" directly,
thus eliminating the above conversion altogether.
I apologize for dividing up the patches this way, function by function,
for my convenience, introduced some temporary changes. The overall
resulted code hopefully is clearer.

>
> Also, I am thinking out loud here but we do have a few places where we
> go: idx = index >> huge_page_order() to convert it into hugepage units, but the casual
> reader might be a bit puzzled about that.
> So, would it be worth to have implement an inline helper with an accurate name
> to do that? It might help whoever reads that?
>

Indeed, will add below inline helpers -
pgoff_t huge_to_base(pgoff_t idx);
pgoff_t base_to_huge(pgoff_t index);

thanks!
-jane