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

[jane . chu]

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@xxxxxxxxxx>
> > ---
> >   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