diff mbox series

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

Message ID 20210511214735.1836149-13-willy@infradead.org (mailing list archive)
State New, archived
Headers show
Series Memory folios | expand

Commit Message

Matthew Wilcox (Oracle) May 11, 2021, 9:47 p.m. UTC 
folio_index() is the equivalent of page_index() for folios.
folio_file_page() is the equivalent of find_subpage().
folio_contains() is the equivalent of thp_contains().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Jeff Layton <jlayton@kernel.org>
---
 include/linux/pagemap.h | 53 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

Comments

Vlastimil Babka May 14, 2021, 3:55 p.m. UTC | #1 
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?

> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jeff Layton <jlayton@kernel.org>
> ---
>  include/linux/pagemap.h | 53 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index bc5fa3d7204e..8eaeffccfd38 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -386,6 +386,59 @@ static inline bool thp_contains(struct page *head, pgoff_t index)
>  	return page_index(head) == (index & ~(thp_nr_pages(head) - 1UL));
>  }
>  
> +#define swapcache_index(folio)	__page_file_index(&(folio)->page)
> +
> +/**
> + * folio_index - File index of a folio.
> + * @folio: The folio.
> + *
> + * For a folio which is either in the page cache or the swap cache,
> + * return its index within the address_space it belongs to.  If you know
> + * the page is definitely in the page cache, you can look at the folio's
> + * index directly.
> + *
> + * Return: The index (offset in units of pages) of a folio in its file.
> + */
> +static inline pgoff_t folio_index(struct folio *folio)
> +{
> +        if (unlikely(folio_swapcache(folio)))
> +                return swapcache_index(folio);
> +        return folio->index;
> +}
> +
> +/**
> + * folio_file_page - The page for a particular index.
> + * @folio: The folio which contains this index.
> + * @index: The index we want to look up.
> + *
> + * Sometimes after looking up a folio in the page cache, we need to
> + * obtain the specific page for an index (eg a page fault).
> + *
> + * Return: The page containing the file data for this index.
> + */
> +static inline struct page *folio_file_page(struct folio *folio, pgoff_t index)
> +{
> +	return folio_page(folio, index & (folio_nr_pages(folio) - 1));
> +}
> +
> +/**
> + * folio_contains - Does this folio contain this index?
> + * @folio: The folio.
> + * @index: The page index within the file.
> + *
> + * Context: The caller should have the page locked in order to prevent
> + * (eg) shmem from moving the page between the page cache and swap cache
> + * and changing its index in the middle of the operation.
> + * Return: true or false.
> + */
> +static inline bool folio_contains(struct folio *folio, pgoff_t index)
> +{
> +	/* HugeTLBfs indexes the page cache in units of hpage_size */
> +	if (folio_hugetlb(folio))
> +		return folio->index == index;
> +	return index - folio_index(folio) < folio_nr_pages(folio);
> +}
> +
>  /*
>   * Given the page we found in the page cache, return the page corresponding
>   * to this index in the file
>
Matthew Wilcox (Oracle) May 15, 2021, 3:51 p.m. UTC | #2 
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.
diff mbox series

Patch

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index bc5fa3d7204e..8eaeffccfd38 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -386,6 +386,59 @@  static inline bool thp_contains(struct page *head, pgoff_t index)
 	return page_index(head) == (index & ~(thp_nr_pages(head) - 1UL));
 }
 
+#define swapcache_index(folio)	__page_file_index(&(folio)->page)
+
+/**
+ * folio_index - File index of a folio.
+ * @folio: The folio.
+ *
+ * For a folio which is either in the page cache or the swap cache,
+ * return its index within the address_space it belongs to.  If you know
+ * the page is definitely in the page cache, you can look at the folio's
+ * index directly.
+ *
+ * Return: The index (offset in units of pages) of a folio in its file.
+ */
+static inline pgoff_t folio_index(struct folio *folio)
+{
+        if (unlikely(folio_swapcache(folio)))
+                return swapcache_index(folio);
+        return folio->index;
+}
+
+/**
+ * folio_file_page - The page for a particular index.
+ * @folio: The folio which contains this index.
+ * @index: The index we want to look up.
+ *
+ * Sometimes after looking up a folio in the page cache, we need to
+ * obtain the specific page for an index (eg a page fault).
+ *
+ * Return: The page containing the file data for this index.
+ */
+static inline struct page *folio_file_page(struct folio *folio, pgoff_t index)
+{
+	return folio_page(folio, index & (folio_nr_pages(folio) - 1));
+}
+
+/**
+ * folio_contains - Does this folio contain this index?
+ * @folio: The folio.
+ * @index: The page index within the file.
+ *
+ * Context: The caller should have the page locked in order to prevent
+ * (eg) shmem from moving the page between the page cache and swap cache
+ * and changing its index in the middle of the operation.
+ * Return: true or false.
+ */
+static inline bool folio_contains(struct folio *folio, pgoff_t index)
+{
+	/* HugeTLBfs indexes the page cache in units of hpage_size */
+	if (folio_hugetlb(folio))
+		return folio->index == index;
+	return index - folio_index(folio) < folio_nr_pages(folio);
+}
+
 /*
  * Given the page we found in the page cache, return the page corresponding
  * to this index in the file