Re: [PATCH v10 12/33] mm/filemap: Add folio_index, folio_file_page and folio_contains

From: Matthew Wilcox
Date: Sat May 15 2021 - 11:52:13 EST


On Fri, May 14, 2021 at 05:55:46PM +0200, Vlastimil Babka wrote:
> On 5/11/21 11:47 PM, Matthew Wilcox (Oracle) wrote:
> > folio_index() is the equivalent of page_index() for folios.
> > folio_file_page() is the equivalent of find_subpage().
>
> find_subpage() special cases hugetlbfs, folio_file_page() doesn't.
>
> > folio_contains() is the equivalent of thp_contains().
>
> Yet here, both thp_contains() and folio_contains() does.
>
> This patch doesn't add users so maybe it becomes obvious later, but perhaps
> worth explaining in the changelog or comment?

No, you're right, this is a bug.

I originally had it in my mind that hugetlbfs wouldn't need to do this
any more because it can just use the folio interfaces and never try to
find the subpage.

But I don't understand all the cases well enough to be sure that
they're all gone, and they certainly don't all go as part of this
patch series. So I think I need to reintroduce the check-for-hugetlb
to folio_file_page() and we can look at removing it later once we're
sure that nobody is using the interfaces that return pages from the page
cache any more. Or we convert hugetlbfs to use the page cache the same
way as every other filesystem ;-)

Thanks for spotting that.