Re: [PATCH 1/6] hugetlb: open-code hugetlb folio lookup index conversion

[jane.chu@oracle.com]

On 4/20/2026 11:27 AM, Matthew Wilcox wrote:
> On Thu, Apr 09, 2026 at 05:41:52PM -0600, Jane Chu wrote:
>> This patch removes `filemap_lock_hugetlb_folio()` and open-codes
>> the index conversion at each call site, making it explicit when
>> hugetlb code is translating a hugepage index into the base-page index
>> expected by `filemap_lock_folio()`
> 
> I think this is too large a piece to break off in a single patch.
> The first thing I did was look at hugetlbfs_read_iter() and wonder
> why we're not able to use generic_file_read_iter() here?  It used
> to be necessary because we used to index the page cache in units
> of hsize, but now we don't, it seems to me that we could use
> generic_file_read_iter() instead.
> 
> Now, what hugetlbfs_read_iter() does have is support for hwpoison
> handling.  I suspect this is something we want in
> generic_file_read_iter(), it's just nobody's done it yet.
> 
> So perhaps that's patch 1 -- add hwpoison support to
> generic_file_read_iter().  Then patch 2 removes hugetlbfs_read_iter() in
> favour of using generic_file_read_iter().
> 
> Patch 3 is purely this:
> (and you can put my Reviewed-by on it).
> 
>> @@ -652,10 +652,10 @@ static void hugetlbfs_zero_partial_page(struct hstate *h,
>>   					loff_t start,
>>   					loff_t end)
>>   {
>> -	pgoff_t idx = start >> huge_page_shift(h);
>> +	pgoff_t index = start >> PAGE_SHIFT;
>>   	struct folio *folio;
>>   
>> -	folio = filemap_lock_hugetlb_folio(h, mapping, idx);
>> +	folio = filemap_lock_folio(mapping, index);
>>   	if (IS_ERR(folio))
>>   		return;
>>   
> 
> Now for patch 4 ...
> 
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index a786034ac95c..38b39eaf46cc 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -5724,7 +5724,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
>>   	 * before we get page_table_lock.
>>   	 */
>>   	new_folio = false;
>> -	folio = filemap_lock_hugetlb_folio(h, mapping, vmf->pgoff);
>> +	folio = filemap_lock_folio(mapping, vmf->pgoff << huge_page_order(h));
>>   	if (IS_ERR(folio)) {
>>   		size = i_size_read(mapping->host) >> huge_page_shift(h);
>>   		if (vmf->pgoff >= size)
> 
> This points to a horrible problem.  Everywhere else in the VM has
> vmf->pgoff in PAGE_SIZE units, and of course hugetlb works in units of
> hpagesize.  So this is an entirely different piece of work where we
> convert vmf->pgoff to be in units of PAGE_SIZE.  That'll be fun!
> 
>> @@ -6208,7 +6208,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
>>   
>>   	if (is_continue) {
>>   		ret = -EFAULT;
>> -		folio = filemap_lock_hugetlb_folio(h, mapping, idx);
>> +		folio = filemap_lock_folio(mapping, idx << huge_page_order(h));
>>   		if (IS_ERR(folio))
>>   			goto out;
>>   		folio_in_pagecache = true;
> 
> This is a much smaller (more contained) problem.  At least idx is local
> to this function, so you can calculate it using linear_page_index()
> and modify the whole function.
> 
> Finally, you can delete filemap_lock_hugetlb_folio():

Thanks Matthew, these are great suggestions and I am looking into them.
Main concern is potential performance impact, will find out.

Thanks a lot!
-jane

> 
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index 9c098a02a09e..c64c6e5e50f5 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -829,12 +829,6 @@ static inline unsigned int blocks_per_huge_page(struct hstate *h)
>>   	return huge_page_size(h) / 512;
>>   }
>>   
>> -static inline struct folio *filemap_lock_hugetlb_folio(struct hstate *h,
>> -				struct address_space *mapping, pgoff_t idx)
>> -{
>> -	return filemap_lock_folio(mapping, idx << huge_page_order(h));
>> -}
>> -
>>   #include <asm/hugetlb.h>
>>   
>>   #ifndef is_hugepage_only_range
>> @@ -1106,12 +1100,6 @@ static inline struct hugepage_subpool *hugetlb_folio_subpool(struct folio *folio
>>   	return NULL;
>>   }
>>   
>> -static inline struct folio *filemap_lock_hugetlb_folio(struct hstate *h,
>> -				struct address_space *mapping, pgoff_t idx)
>> -{
>> -	return NULL;
>> -}
>> -
>>   static inline int isolate_or_dissolve_huge_folio(struct folio *folio,
>>   						struct list_head *list)
>>   {