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

[jane.chu@oracle.com]

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