Re: [PATCH 1/5] mm: remove find_subpage()

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2024/8/17 17:51, Kefeng Wang wrote:
> After commit a08c7193e4f1 ("mm/filemap: remove hugetlb special casing
> in filemap.c"), the find_subpage() should remove hugetlb case as the
> folio_file_page(), furthermore, we could convert to use folio_file_page()
> to remove find_subpage().

There are some comments from David to the non-public send(forget to cc 
list),
the problem of find_subpage() is not described , so adding some here,


see commit a08c7193e4f1,

--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -789,9 +789,6 @@ static inline pgoff_t folio_next_index(struct folio 
*folio)
   */
  static inline struct page *folio_file_page(struct folio *folio, 
pgoff_t index)
  {
-       /* HugeTLBfs indexes the page cache in units of hpage_size */
-       if (folio_test_hugetlb(folio))
-               return &folio->page;
         return folio_page(folio, index & (folio_nr_pages(folio) - 1));
  }

It changes the granularity of ->index to the base page size rather than
the huge page size, so for hugetlb, the special handling(return head
page) is removed from folio_file_page(), so we need remove special
hugetlb handling find_subpage() too, maybe this is a bugfix as a
separate patch.

And after removing hugetlb handling in find_subpage(), there is
another issue about "head + (index & (thp_nr_pages(head) - 1))", for
hugetlb without sparsemem vmemmap, struct page is not guaranteed to be
contiguous beyond a section, so we need to use

  nth_page(head, (index & (thp_nr_pages(head) - 1))

and in order to reduce code maintain between folio_file_page() and
find_subpage()， just use folio_file_page() in find_subpage() to
fix above two issue.

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index d9c7edb6422b..e2553e4ac3ef 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -866,11 +866,9 @@ static inline bool folio_contains(struct folio 
*folio, pgoff_t index)
   */
  static inline struct page *find_subpage(struct page *head, pgoff_t index)
  {
-       /* HugeTLBfs wants the head page regardless */
-       if (PageHuge(head))
-               return head;
+       struct folio *folio = (struct folio *)head;

-       return head + (index & (thp_nr_pages(head) - 1));
+       return folio_file_page(folio);
  }

And this will correctly handle head/tail page, correct me if I am wrong.

Thanks.