diff mbox series

[v4,08/25] mm: Handle per-folio private data

Message ID 20210305041901.2396498-9-willy@infradead.org (mailing list archive)
State New, archived
Headers show
Series Page folios | expand

Commit Message

Matthew Wilcox March 5, 2021, 4:18 a.m. UTC
Add folio_private() and set_folio_private() which mirror page_private()
and set_page_private() -- ie folio private data is the same as page
private data.

Turn attach_page_private() into attach_folio_private() and reimplement
attach_page_private() as a wrapper.  No filesystem which uses page private
data currently supports compound pages, so we're free to define the rules.
attach_page_private() may only be called on a head page; if you want
to add private data to a tail page, you can call set_page_private()
directly (and shouldn't increment the page refcount!  That should be
done when adding private data to the head page / folio).

This saves 886 bytes of text with the distro-derived config that I'm
testing due to removing the calls to compound_head() in get_page()
& put_page().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/mm_types.h | 16 ++++++++++++++
 include/linux/pagemap.h  | 48 ++++++++++++++++++++++++----------------
 2 files changed, 45 insertions(+), 19 deletions(-)

Comments

Christoph Hellwig March 17, 2021, 5:20 p.m. UTC | #1
> +static inline void attach_page_private(struct page *page, void *data)
> +{
> +	attach_folio_private((struct folio *)page, data);
> +}
> +
> +static inline void *detach_page_private(struct page *page)
> +{
> +	return detach_folio_private((struct folio *)page);
> +}

I hate these open code casts.  Can't we have a single central
page_to_folio helper, which could also grow a debug check (maybe
under a new config option) to check that it really is called on a
head page?
Matthew Wilcox March 18, 2021, 5:57 p.m. UTC | #2
On Wed, Mar 17, 2021 at 06:20:32PM +0100, Christoph Hellwig wrote:
> > +static inline void attach_page_private(struct page *page, void *data)
> > +{
> > +	attach_folio_private((struct folio *)page, data);
> > +}
> > +
> > +static inline void *detach_page_private(struct page *page)
> > +{
> > +	return detach_folio_private((struct folio *)page);
> > +}
> 
> I hate these open code casts.  Can't we have a single central
> page_to_folio helper, which could also grow a debug check (maybe
> under a new config option) to check that it really is called on a
> head page?

Some of that is already there.  We have page_folio() which is the
page_to_folio() helper you're asking for.  And folio_flags() (which is
called *all the time*) contains
        VM_BUG_ON_PGFLAGS(PageTail(page), page);
Someone passing around a tail pointer cast to a folio is not going to
get very far, assuming CONFIG_DEBUG_VM_PGFLAGS is enabled (most distros
don't, but I do when I'm testing anything THPish).

These helpers aren't going to live for very long ... I expect to have
all filesystems which use attach/detach page private converted to folios
pretty soon.  Certainly before any of them _use_ multi-page folios.

Anyway, the simple thing to do is just to use page_folio() here and eat
the cost of calling compound_head() on something we're certain is an
order-0 page.  It only defers the win of removing the compound_head()
call; it doesn't preclude it.  And it means we're not setting a bad
example here (there really shouldn't be any casts from pages to folios,
except in the folio allocator, which uses the page allocator and then
casts what _must be_ a non-tail page to a folio).
diff mbox series

Patch

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a311cb48526f..b650423fcba6 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -258,6 +258,12 @@  static inline atomic_t *compound_pincount_ptr(struct page *page)
 #define PAGE_FRAG_CACHE_MAX_SIZE	__ALIGN_MASK(32768, ~PAGE_MASK)
 #define PAGE_FRAG_CACHE_MAX_ORDER	get_order(PAGE_FRAG_CACHE_MAX_SIZE)
 
+/*
+ * page_private can be used on tail pages.  However, PagePrivate is only
+ * checked by the VM on the head page.  So page_private on the tail pages
+ * should be used for data that's ancillary to the head page (eg attaching
+ * buffer heads to tail pages after attaching buffer heads to the head page)
+ */
 #define page_private(page)		((page)->private)
 
 static inline void set_page_private(struct page *page, unsigned long private)
@@ -265,6 +271,16 @@  static inline void set_page_private(struct page *page, unsigned long private)
 	page->private = private;
 }
 
+static inline unsigned long folio_private(struct folio *folio)
+{
+	return folio->page.private;
+}
+
+static inline void set_folio_private(struct folio *folio, unsigned long v)
+{
+	folio->page.private = v;
+}
+
 struct page_frag_cache {
 	void * va;
 #if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 20225b067583..f07c03da83f6 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -245,42 +245,52 @@  static inline int page_cache_add_speculative(struct page *page, int count)
 }
 
 /**
- * attach_page_private - Attach private data to a page.
- * @page: Page to attach data to.
- * @data: Data to attach to page.
+ * attach_folio_private - Attach private data to a folio.
+ * @folio: Folio to attach data to.
+ * @data: Data to attach to folio.
  *
- * Attaching private data to a page increments the page's reference count.
- * The data must be detached before the page will be freed.
+ * Attaching private data to a folio increments the page's reference count.
+ * The data must be detached before the folio will be freed.
  */
-static inline void attach_page_private(struct page *page, void *data)
+static inline void attach_folio_private(struct folio *folio, void *data)
 {
-	get_page(page);
-	set_page_private(page, (unsigned long)data);
-	SetPagePrivate(page);
+	get_folio(folio);
+	set_folio_private(folio, (unsigned long)data);
+	SetFolioPrivate(folio);
 }
 
 /**
- * detach_page_private - Detach private data from a page.
- * @page: Page to detach data from.
+ * detach_folio_private - Detach private data from a folio.
+ * @folio: Folio to detach data from.
  *
- * Removes the data that was previously attached to the page and decrements
+ * Removes the data that was previously attached to the folio and decrements
  * the refcount on the page.
  *
- * Return: Data that was attached to the page.
+ * Return: Data that was attached to the folio.
  */
-static inline void *detach_page_private(struct page *page)
+static inline void *detach_folio_private(struct folio *folio)
 {
-	void *data = (void *)page_private(page);
+	void *data = (void *)folio_private(folio);
 
-	if (!PagePrivate(page))
+	if (!FolioPrivate(folio))
 		return NULL;
-	ClearPagePrivate(page);
-	set_page_private(page, 0);
-	put_page(page);
+	ClearFolioPrivate(folio);
+	set_folio_private(folio, 0);
+	put_folio(folio);
 
 	return data;
 }
 
+static inline void attach_page_private(struct page *page, void *data)
+{
+	attach_folio_private((struct folio *)page, data);
+}
+
+static inline void *detach_page_private(struct page *page)
+{
+	return detach_folio_private((struct folio *)page);
+}
+
 #ifdef CONFIG_NUMA
 extern struct page *__page_cache_alloc(gfp_t gfp);
 #else