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

[jane . chu]

On 4/11/2026 7:14 AM, Mike Rapoport wrote:
> Hi,
>
> 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()`.  As part of that cleanup,
> > it also uses a base-page index directly in `hugetlbfs_zero_partial_page()`,
> > where the byte offset is already page-granular. Overall, the change
> > makes the indexing model more obvious at the call sites and avoids
> > hiding the huge-index to base-index conversion inside a helper.
> >
> > Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> > Signed-off-by: Jane Chu <jane.chu@xxxxxxxxxx>
> > ---
> >   fs/hugetlbfs/inode.c    | 20 ++++++++++----------
> >   include/linux/hugetlb.h | 12 ------------
> >   mm/hugetlb.c            |  4 ++--
> >   3 files changed, 12 insertions(+), 24 deletions(-)
> >
> > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> > index cd6b22f6e2b1..cf79fb830377 100644
> > --- a/fs/hugetlbfs/inode.c
> > +++ b/fs/hugetlbfs/inode.c
> > @@ -242,9 +242,9 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >   	struct hstate *h = hstate_file(file);
> >   	struct address_space *mapping = file->f_mapping;
> >   	struct inode *inode = mapping->host;
> > -	unsigned long index = iocb->ki_pos >> huge_page_shift(h);
> > +	unsigned long idx = iocb->ki_pos >> huge_page_shift(h);
>
> Is it necessary to rename index to idx?
>
> >   	unsigned long offset = iocb->ki_pos & ~huge_page_mask(h);
> > -	unsigned long end_index;
> > +	unsigned long end_idx;
> >   	loff_t isize;
> >   	ssize_t retval = 0;
>    
> ...
>
> > @@ -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;
>
> And idx to index?
>
> Maybe let's pick one and rename the other or just leave them be.

As I just replied to Oscar, I found the mixture of idx/index both could 
represent both huge page index as well as base page index creates a bit 
dizzying situation to the reader.  So through out the patches, 'idx' is 
made to carry the notion of huge page index while 'index' carry the 
notion of base page index.

thanks,
-jane

> >   	struct folio *folio;
> >