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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux