Convert GUP to folios

This macro doesn't simplify the users; it's easier to just call compound_range_next() inside the loop.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

This macro doesn't simplify the users; it's easier to just call compound_range_next() inside the loop.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 12 ++----------
  1 file changed, 2 insertions(+), 10 deletions(-)

mm/gup.c +0 -3

index 2c51e9748a6a..7a07e0c00bf5 100644
@@ -236,9 +236,6 @@ static inline void compound_range_next(unsigned long i, unsigned long npages,
  	struct page *next, *page;
  	unsigned int nr = 1;
 
-	if (i >= npages)
-		return;
-
  	next = *list + i;
  	page = compound_head(next);
  	if (PageCompound(page) && compound_order(page) >= 1)

This macro doesn't simplify the users; it's easier to just call compound_next() inside a standard loop.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---

  mm/gup.c | 16 +++++-----------
  1 file changed, 5 insertions(+), 11 deletions(-)

mm/gup.c +5 -11

index 7a07e0c00bf5..86f8d843de72 100644
@@ -253,9 +253,6 @@ static inline void compound_next(unsigned long i, unsigned long npages,
  	struct page *page;
  	unsigned int nr;
 
-	if (i >= npages)
-		return;
-
  	page = compound_head(list[i]);
  	for (nr = i + 1; nr < npages; nr++) {
  		if (compound_head(list[nr]) != page)

@@ -266,12 +263,6 @@ static inline void compound_next(unsigned long i, unsigned long npages,
  	*ntails = nr - i;
  }
 
-#define for_each_compound_head(__i, __list, __npages, __head, __ntails) \
-	for (__i = 0, \
-	     compound_next(__i, __npages, __list, &(__head), &(__ntails)); \
-	     __i < __npages; __i += __ntails, \
-	     compound_next(__i, __npages, __list, &(__head), &(__ntails)))
-
  /**
   * unpin_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
   * @pages:  array of pages to be maybe marked dirty, and definitely released.

@@ -306,7 +297,8 @@ void unpin_user_pages_dirty_lock(struct page **pages, unsigned long npages,
  		return;
  	}
 
-	for_each_compound_head(index, pages, npages, head, ntails) {
+	for (index = 0; index < npages; index += ntails) {
+		compound_next(index, npages, pages, &head, &ntails);
  		/*
  		 * Checking PageDirty at this point may race with
  		 * clear_page_dirty_for_io(), but that's OK. Two key

@@ -394,8 +386,10 @@ void unpin_user_pages(struct page **pages, unsigned long npages)
  	if (WARN_ON(IS_ERR_VALUE(npages)))
  		return;
 
-	for_each_compound_head(index, pages, npages, head, ntails)
+	for (index = 0; index < npages; index += ntails) {
+		compound_next(index, npages, pages, &head, &ntails);
  		put_compound_head(head, ntails, FOLL_PIN);
+	}
  }
  EXPORT_SYMBOL(unpin_user_pages);
 

Return the head page instead of storing it to a passed parameter. Pass the start page directly instead of passing a pointer to it.

Return the head page instead of storing it to a passed parameter. Pass the start page directly instead of passing a pointer to it.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---

  mm/gup.c | 11 +++++------
  1 file changed, 5 insertions(+), 6 deletions(-)

mm/gup.c +5 -6

index 86f8d843de72..3c93d2fdf4da 100644
@@ -229,21 +229,20 @@ void unpin_user_page(struct page *page)
  }
  EXPORT_SYMBOL(unpin_user_page);
 
-static inline void compound_range_next(unsigned long i, unsigned long npages,
-				       struct page **list, struct page **head,
-				       unsigned int *ntails)
+static inline struct page *compound_range_next(unsigned long i,
+		unsigned long npages, struct page *start, unsigned int *ntails)
  {
  	struct page *next, *page;
  	unsigned int nr = 1;
 
-	next = *list + i;
+	next = start + i;
  	page = compound_head(next);
  	if (PageCompound(page) && compound_order(page) >= 1)
  		nr = min_t(unsigned int,
  			   page + compound_nr(page) - next, npages - i);
 
-	*head = page;
  	*ntails = nr;
+	return page;
  }
 
  static inline void compound_next(unsigned long i, unsigned long npages,

@@ -355,7 +354,7 @@ void unpin_user_page_range_dirty_lock(struct page *page, unsigned long npages,
  	unsigned int ntails;
 
  	for (index = 0; index < npages; index += ntails) {
-		compound_range_next(index, npages, &page, &head, &ntails);
+		head = compound_range_next(index, npages, page, &ntails);
  		if (make_dirty && !PageDirty(head))
  			set_page_dirty_lock(head);
  		put_compound_head(head, ntails, FOLL_PIN);

By definition, a compound page has an order >= 1, so the second half of the test was redundant. Also, this cannot be a tail page since it's the result of calling compound_head(), so use PageHead() instead of PageCompound().

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

By definition, a compound page has an order >= 1, so the second half of the test was redundant. Also, this cannot be a tail page since it's the result of calling compound_head(), so use PageHead() instead of PageCompound().

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---

  mm/gup.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

mm/gup.c +1 -1

index 3c93d2fdf4da..6eedca605b3d 100644
@@ -237,7 +237,7 @@ static inline struct page *compound_range_next(unsigned long i,
 
  	next = start + i;
  	page = compound_head(next);
-	if (PageCompound(page) && compound_order(page) >= 1)
+	if (PageHead(page))
  		nr = min_t(unsigned int,
  			   page + compound_nr(page) - next, npages - i);
 

Return the head page instead of storing it to a passed parameter.

Quoted 1 line · Matthew Wilcox (Oracle)

Return the head page instead of storing it to a passed parameter.

Looks good, but same comment about maybe passing list and npages first.

Return the head page instead of storing it to a passed parameter.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 11 +++++------
  1 file changed, 5 insertions(+), 6 deletions(-)

mm/gup.c +5 -6

index 6eedca605b3d..8a0ea220ced1 100644
@@ -245,9 +245,8 @@ static inline struct page *compound_range_next(unsigned long i,
  	return page;
  }
 
-static inline void compound_next(unsigned long i, unsigned long npages,
-				 struct page **list, struct page **head,
-				 unsigned int *ntails)
+static inline struct page *compound_next(unsigned long i,
+		unsigned long npages, struct page **list, unsigned int *ntails)
  {
  	struct page *page;
  	unsigned int nr;

@@ -258,8 +257,8 @@ static inline void compound_next(unsigned long i, unsigned long npages,
  			break;
  	}
 
-	*head = page;
  	*ntails = nr - i;
+	return page;
  }
 
  /**

@@ -297,7 +296,7 @@ void unpin_user_pages_dirty_lock(struct page **pages, unsigned long npages,
  	}
 
  	for (index = 0; index < npages; index += ntails) {
-		compound_next(index, npages, pages, &head, &ntails);
+		head = compound_next(index, npages, pages, &ntails);
  		/*
  		 * Checking PageDirty at this point may race with
  		 * clear_page_dirty_for_io(), but that's OK. Two key

@@ -386,7 +385,7 @@ void unpin_user_pages(struct page **pages, unsigned long npages)
  		return;
 
  	for (index = 0; index < npages; index += ntails) {
-		compound_next(index, npages, pages, &head, &ntails);
+		head = compound_next(index, npages, pages, &ntails);
  		put_compound_head(head, ntails, FOLL_PIN);
  	}
  }

Several functions in gup.c assume that a compound page has virtually contiguous page structs. This isn't true for SPARSEMEM configs unless SPARSEMEM_VMEMMAP is also set. Fix them by using nth_page() instead of plain pointer arithmetic.

Quoted 4 lines · Matthew Wilcox (Oracle)

Several functions in gup.c assume that a compound page has virtually contiguous page structs. This isn't true for SPARSEMEM configs unless SPARSEMEM_VMEMMAP is also set. Fix them by using nth_page() instead of plain pointer arithmetic.

So is this an actualy bug that need a Fixes tag, or do all architectures that support THP and sparsemem use SPARSEMEM_VMEMMAP?

mm/gup.c static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr, · line 2469

Quoted 1 line

mm/gup.c +1 -0

+	page = nth_page(head, (addr & (sz-1)) >> PAGE_SHIFT);

Would be nice to fix the indeation for sz - 1 while you're at it.

Quoted 8 lines · Christoph Hellwig

Quoted 4 lines · Matthew Wilcox (Oracle)

Several functions in gup.c assume that a compound page has virtually contiguous page structs. This isn't true for SPARSEMEM configs unless SPARSEMEM_VMEMMAP is also set. Fix them by using nth_page() instead of plain pointer arithmetic.

So is this an actualy bug that need a Fixes tag, or do all architectures that support THP and sparsemem use SPARSEMEM_VMEMMAP?

As far as I can tell (and I am by no means an expert in this area), this problem only affects pages of order MAX_ORDER or higher. That is, somebody using regular 2MB hugepages on x86 won't see a problem, whether they're using VMEMMAP or not. It only starts to become a problem for 1GB hugepages.

Since THPs are (currently) only allocated from the page allocator, it's never a problem for THPs, only hugetlbfs. Correcting the places which can't see a 1GB page is just defense against copy-and-paste programming.

So I'll defer to Mike -- does this ever affect real systems and thus warrant a backport? I know this doesn't affect UEK because we enable SPARSEMEM_VMEMMAP.

Several functions in gup.c assume that a compound page has virtually contiguous page structs. This isn't true for SPARSEMEM configs unless SPARSEMEM_VMEMMAP is also set. Fix them by using nth_page() instead of plain pointer arithmetic.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

mm/gup.c +7 -7

index 8a0ea220ced1..9c0a702a4e03 100644
@@ -235,7 +235,7 @@ static inline struct page *compound_range_next(unsigned long i,
  	struct page *next, *page;
  	unsigned int nr = 1;
 
-	next = start + i;
+	next = nth_page(start, i);
  	page = compound_head(next);
  	if (PageHead(page))
  		nr = min_t(unsigned int,

@@ -2430,8 +2430,8 @@ static int record_subpages(struct page *page, unsigned long addr,
  {
  	int nr;
 
-	for (nr = 0; addr != end; addr += PAGE_SIZE)
-		pages[nr++] = page++;
+	for (nr = 0; addr != end; nr++, addr += PAGE_SIZE)
+		pages[nr] = nth_page(page, nr);
 
  	return nr;
  }

@@ -2466,7 +2466,7 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
  	VM_BUG_ON(!pfn_valid(pte_pfn(pte)));
 
  	head = pte_page(pte);
-	page = head + ((addr & (sz-1)) >> PAGE_SHIFT);
+	page = nth_page(head, (addr & (sz-1)) >> PAGE_SHIFT);
  	refs = record_subpages(page, addr, end, pages + *nr);
 
  	head = try_grab_compound_head(head, refs, flags);

@@ -2526,7 +2526,7 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
  					     pages, nr);
  	}
 
-	page = pmd_page(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
+	page = nth_page(pmd_page(orig), (addr & ~PMD_MASK) >> PAGE_SHIFT);
  	refs = record_subpages(page, addr, end, pages + *nr);
 
  	head = try_grab_compound_head(pmd_page(orig), refs, flags);

@@ -2560,7 +2560,7 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
  					     pages, nr);
  	}
 
-	page = pud_page(orig) + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
+	page = nth_page(pud_page(orig), (addr & ~PUD_MASK) >> PAGE_SHIFT);
  	refs = record_subpages(page, addr, end, pages + *nr);
 
  	head = try_grab_compound_head(pud_page(orig), refs, flags);

@@ -2589,7 +2589,7 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
 
  	BUILD_BUG_ON(pgd_devmap(orig));
 
-	page = pgd_page(orig) + ((addr & ~PGDIR_MASK) >> PAGE_SHIFT);
+	page = nth_page(pgd_page(orig), (addr & ~PGDIR_MASK) >> PAGE_SHIFT);
  	refs = record_subpages(page, addr, end, pages + *nr);
 
  	head = try_grab_compound_head(pgd_page(orig), refs, flags);

This assumption needs the inverse of nth_page(), which I've temporarily named page_nth() until someone comes up with a better name.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  include/linux/mm.h | 2 ++
  mm/gup.c           | 4 ++--
  2 files changed, 4 insertions(+), 2 deletions(-)

include/linux/mm.h +2 -0

index d8b7d7ed14dd..f2f3400665a4 100644
@@ -216,8 +216,10 @@ int overcommit_policy_handler(struct ctl_table *, int, void *, size_t *,
 
  #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
  #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n))
+#define page_nth(head, tail)	(page_to_pfn(tail) - page_to_pfn(head))
  #else
  #define nth_page(page,n) ((page) + (n))
+#define page_nth(head, tail)	((tail) - (head))
  #endif
 
  /* to align the pointer to the (next) page boundary */

mm/gup.c +2 -2

index 9c0a702a4e03..afb638a30e44 100644
@@ -238,8 +238,8 @@ static inline struct page *compound_range_next(unsigned long i,
  	next = nth_page(start, i);
  	page = compound_head(next);
  	if (PageHead(page))
-		nr = min_t(unsigned int,
-			   page + compound_nr(page) - next, npages - i);
+		nr = min_t(unsigned int, npages - i,
+			   compound_nr(page) - page_nth(page, next));
 
  	*ntails = nr;
  	return page;

If we hit the page split race, the current code returns NULL which will presumably trigger a retry under the mmap_lock. This isn't necessary; we can just retry the compound_head() lookup. This is a very minor optimisation of an unlikely path, but conceptually it matches (eg) the page cache RCU-protected lookup.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

If we hit the page split race, the current code returns NULL which will presumably trigger a retry under the mmap_lock. This isn't necessary; we can just retry the compound_head() lookup. This is a very minor optimisation of an unlikely path, but conceptually it matches (eg) the page cache RCU-protected lookup.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

mm/gup.c +5 -2

index afb638a30e44..dbb1b54d0def 100644
@@ -68,7 +68,10 @@ static void put_page_refs(struct page *page, int refs)
   */
  static inline struct page *try_get_compound_head(struct page *page, int refs)
  {
-	struct page *head = compound_head(page);
+	struct page *head;
+
+retry:
+	head = compound_head(page);
 
  	if (WARN_ON_ONCE(page_ref_count(head) < 0))
  		return NULL;

@@ -86,7 +89,7 @@ static inline struct page *try_get_compound_head(struct page *page, int refs)
  	 */
  	if (unlikely(compound_head(page) != head)) {
  		put_page_refs(head, refs);
-		return NULL;
+		goto retry;
  	}
 
  	return head;

Simplify try_grab_compound_head() and remove an unnecessary VM_BUG_ON by handling pages both with and without a pincount field in page_pincount_add().

Simplify try_grab_compound_head() and remove an unnecessary VM_BUG_ON by handling pages both with and without a pincount field in page_pincount_add().

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 33 +++++++++++++++------------------
  1 file changed, 15 insertions(+), 18 deletions(-)

mm/gup.c +15 -18

index dbb1b54d0def..3ed9907f3c8d 100644
@@ -29,12 +29,23 @@ struct follow_page_context {
  	unsigned int page_mask;
  };
 
-static void hpage_pincount_add(struct page *page, int refs)
+/*
+ * When pinning a compound page of order > 1 (which is what
+ * hpage_pincount_available() checks for), use an exact count to track
+ * it, via page_pincount_add/_sub().
+ *
+ * However, be sure to *also* increment the normal page refcount field
+ * at least once, so that the page really is pinned.  That's why the
+ * refcount from the earlier try_get_compound_head() is left intact.
+ */
+static void page_pincount_add(struct page *page, int refs)
  {
-	VM_BUG_ON_PAGE(!hpage_pincount_available(page), page);
  	VM_BUG_ON_PAGE(page != compound_head(page), page);
 
-	atomic_add(refs, compound_pincount_ptr(page));
+	if (hpage_pincount_available(page))
+		atomic_add(refs, compound_pincount_ptr(page));
+	else
+		page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
  }
 
  static void hpage_pincount_sub(struct page *page, int refs)

@@ -150,21 +161,7 @@ struct page *try_grab_compound_head(struct page *page,
  		if (!page)
  			return NULL;
 
-		/*
-		 * When pinning a compound page of order > 1 (which is what
-		 * hpage_pincount_available() checks for), use an exact count to
-		 * track it, via hpage_pincount_add/_sub().
-		 *
-		 * However, be sure to *also* increment the normal page refcount
-		 * field at least once, so that the page really is pinned.
-		 * That's why the refcount from the earlier
-		 * try_get_compound_head() is left intact.
-		 */
-		if (hpage_pincount_available(page))
-			hpage_pincount_add(page, refs);
-		else
-			page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
-
+		page_pincount_add(page, refs);
  		mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_ACQUIRED,
  				    refs);
 

mm/gup.c +0 -0

index dbb1b54d0def..3ed9907f3c8d 100644
@@ -29,12 +29,23 @@ struct follow_page_context {
  	unsigned int page_mask;
  };
 

mm/gup.c +5 -2

+static void page_pincount_add(struct page *page, int refs)
  {
-	VM_BUG_ON_PAGE(!hpage_pincount_available(page), page);
  	VM_BUG_ON_PAGE(page != compound_head(page), page);
 
-	atomic_add(refs, compound_pincount_ptr(page));
+	if (hpage_pincount_available(page))
+		atomic_add(refs, compound_pincount_ptr(page));
+	else
+		page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
  }
 
  static void hpage_pincount_sub(struct page *page, int refs)

@@ -150,21 +161,7 @@ struct page *try_grab_compound_head(struct page *page,
  		if (!page)
  			return NULL;
 

On 1/9/22 20:23, Matthew Wilcox (Oracle) wrote:
...

Quoted 14 lines

mm/gup.c +6 -1

index dbb1b54d0def..3ed9907f3c8d 100644
@@ -29,12 +29,23 @@ struct follow_page_context {
  	unsigned int page_mask;
  };
-static void hpage_pincount_add(struct page *page, int refs)
+/*
+ * When pinning a compound page of order > 1 (which is what
+ * hpage_pincount_available() checks for), use an exact count to track
+ * it, via page_pincount_add/_sub().
+ *
+ * However, be sure to *also* increment the normal page refcount field

mm/gup.c struct follow_page_context { · line 40

Quoted 3 lines

mm/gup.c +3 -0

+ * at least once, so that the page really is pinned.  That's why the
+ * refcount from the earlier try_get_compound_head() is left intact.
+ */

I just realized, after looking at this again in a later patch, that the last paragraph, above, is now misplaced. It refers to the behavior of the caller, not to this routine. So it needs to be airlifted back to the caller.

Quoted 14 lines

mm/gup.c +5 -2

+static void page_pincount_add(struct page *page, int refs)
  {
-	VM_BUG_ON_PAGE(!hpage_pincount_available(page), page);
  	VM_BUG_ON_PAGE(page != compound_head(page), page);
-	atomic_add(refs, compound_pincount_ptr(page));
+	if (hpage_pincount_available(page))
+		atomic_add(refs, compound_pincount_ptr(page));
+	else
+		page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
  }
  static void hpage_pincount_sub(struct page *page, int refs)

@@ -150,21 +161,7 @@ struct page *try_grab_compound_head(struct page *page,
  		if (!page)
  			return NULL;

mm/gup.c struct page *try_grab_compound_head(struct page *page, · line 161

Quoted 10 lines

mm/gup.c +0 -10

-		/*
-		 * When pinning a compound page of order > 1 (which is what
-		 * hpage_pincount_available() checks for), use an exact count to
-		 * track it, via hpage_pincount_add/_sub().
-		 *
-		 * However, be sure to *also* increment the normal page refcount
-		 * field at least once, so that the page really is pinned.
-		 * That's why the refcount from the earlier
-		 * try_get_compound_head() is left intact.
-		 */

...here.

mm/gup.c struct page *try_grab_compound_head(struct page *page, · line 166

Quoted 8 lines

mm/gup.c +1 -5

-		if (hpage_pincount_available(page))
-			hpage_pincount_add(page, refs);
-		else
-			page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
-
+		page_pincount_add(page, refs);
  		mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_ACQUIRED,
  				    refs);

thanks,

Signature · 2 lines

John Hubbard
NVIDIA

Remove an unnecessary VM_BUG_ON by handling pages both with and without a pincount field in page_pincount_sub().

Remove an unnecessary VM_BUG_ON by handling pages both with and without a pincount field in page_pincount_sub().

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 15 +++++++--------
  1 file changed, 7 insertions(+), 8 deletions(-)

mm/gup.c +0 -0

index 3ed9907f3c8d..aed48de3912e 100644

mm/gup.c +7 -7

+static int page_pincount_sub(struct page *page, int refs)
  {
-	VM_BUG_ON_PAGE(!hpage_pincount_available(page), page);
  	VM_BUG_ON_PAGE(page != compound_head(page), page);
 
-	atomic_sub(refs, compound_pincount_ptr(page));
+	if (hpage_pincount_available(page))
+		atomic_sub(refs, compound_pincount_ptr(page));
+	else
+		refs *= GUP_PIN_COUNTING_BIAS;
+	return refs;
  }
 
  /* Equivalent to calling put_page() @refs times. */

@@ -177,11 +180,7 @@ static void put_compound_head(struct page *page, int refs, unsigned int flags)
  	if (flags & FOLL_PIN) {
  		mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_RELEASED,
  				    refs);
-
-		if (hpage_pincount_available(page))
-			hpage_pincount_sub(page, refs);
-		else
-			refs *= GUP_PIN_COUNTING_BIAS;
+		refs = page_pincount_sub(page, refs);
  	}
 
  	put_page_refs(page, refs);

Move compound_pincount from the third page to the second page, which means it's available for all compound pages. That lets us delete hpage_pincount_available().

include/linux/mm.h +2 -0

@@ -955,7 +944,9 @@ static inline int compound_pincount(struct page *page)
  static inline void set_compound_order(struct page *page, unsigned int order)
  {
  	page[1].compound_order = order;
+#ifdef CONFIG_64BIT
  	page[1].compound_nr = 1U << order;
+#endif
  }
 
  /* Returns the number of pages in this potentially compound page. */

mm/gup.c +1 -1

@@ -42,7 +41,7 @@ static void page_pincount_add(struct page *page, int refs)
  {
  	VM_BUG_ON_PAGE(page != compound_head(page), page);
 
-	if (hpage_pincount_available(page))
+	if (PageHead(page))
  		atomic_add(refs, compound_pincount_ptr(page));
  	else
  		page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));

include/linux/mm.h static inline unsigned long compound_nr(struct page *page) · line 961

Quoted 5 lines

include/linux/mm.h +4 -0

+#ifdef CONFIG_64BIT
  	return page[1].compound_nr;
+#else
+	return 1UL << compound_order(page);
+#endif

Now that you are highlighting this, I have this persistent feeling (not yet confirmed by any testing) that compound_nr is a micro-optimization that is actually invisible at runtime--but is now slicing up our code with ifdefs, and using space in a fairly valuable location.

Not for this patch or series, but maybe a separate patch or series should just remove the compound_nr field entirely, yes? It is surprising to carry around both compound_order and (1 << compound_order), right next to each other. It would be different if this were an expensive calculation, but it's just a shift.

Maybe testing would prove that that's a bad idea, and maybe someone has already looked into it, but I wanted to point it out.

mm/gup.c static int page_pincount_sub(struct page *page, int refs) · line 53

Quoted 5 lines

mm/gup.c +1 -1

@@ -52,7 +51,7 @@ static int page_pincount_sub(struct page *page, int refs)
  {
  	VM_BUG_ON_PAGE(page != compound_head(page), page);
-	if (hpage_pincount_available(page))
+	if (PageHead(page))

OK, so we just verified (via VM_BUG_ON_PAGE(), which is not always active) that this is not a tail page. And so PageHead() effectively means PageCompound().

I wonder if it would be better to just use PageCompound() here and in similar cases. Because that's what is logically being checked, after all. It seems slightly more accurate.

Quoted 17 lines · John Hubbard

include/linux/mm.h static inline unsigned long compound_nr(struct page *page) · line 961

Quoted 5 lines

include/linux/mm.h +4 -0

+#ifdef CONFIG_64BIT
   	return page[1].compound_nr;
+#else
+	return 1UL << compound_order(page);
+#endif

Now that you are highlighting this, I have this persistent feeling (not yet confirmed by any testing) that compound_nr is a micro-optimization that is actually invisible at runtime--but is now slicing up our code with ifdefs, and using space in a fairly valuable location.

Not for this patch or series, but maybe a separate patch or series should just remove the compound_nr field entirely, yes? It is surprising to carry around both compound_order and (1 << compound_order), right next to each other. It would be different if this were an expensive calculation, but it's just a shift.

Quoted 2 lines

Maybe testing would prove that that's a bad idea, and maybe someone has already looked into it, but I wanted to point it out.

It' probably worth looking at the patch which added it ... 1378a5ee451a in August 2020. I didn't provide any performance numbers, but code size definitely went down.

Quoted 10 lines

mm/gup.c static int page_pincount_sub(struct page *page, int refs) · line 53

Quoted 6 lines

mm/gup.c +1 -1

@@ -52,7 +51,7 @@ static int page_pincount_sub(struct page *page, int refs)
   {
   	VM_BUG_ON_PAGE(page != compound_head(page), page);
-	if (hpage_pincount_available(page))
+	if (PageHead(page))

OK, so we just verified (via VM_BUG_ON_PAGE(), which is not always active) that this is not a tail page. And so PageHead() effectively means PageCompound().

Quoted 3 lines

I wonder if it would be better to just use PageCompound() here and in similar cases. Because that's what is logically being checked, after all. It seems slightly more accurate.

Well PageCompound() is defined as PageHead() || PageTail(). I don't think the intent was for people to always ask "Is this a compound page", more "This is a good shorthand to replace PageHead() || PageTail()". It's kind of moot anyway because this gets replaced with folio_test_large() further down the patch series.

This is like folio_put(), but puts N references at once instead of just one. It's like put_page_refs(), but does one atomic operation instead of two, and is available to more than just gup.c.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

This is like folio_put(), but puts N references at once instead of just one. It's like put_page_refs(), but does one atomic operation instead of two, and is available to more than just gup.c.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  include/linux/mm.h | 20 ++++++++++++++++++++
  1 file changed, 20 insertions(+)

include/linux/mm.h +0 -0

index 598be27d4d2e..bf9624ca61c3 100644

include/linux/mm.h +9 -0

+ *
+ * If the folio's reference count reaches zero, the memory will be
+ * released back to the page allocator and may be used by another
+ * allocation immediately.  Do not access the memory or the struct folio
+ * after calling folio_put_refs() unless you can be sure that these weren't
+ * the last references.
+ *
+ * Context: May be called in process or interrupt context, but not in NMI
+ * context.  May be called while holding a spinlock.

This is the folio equivalent of compound_pincount_ptr().

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  include/linux/mm_types.h | 6 ++++++
  1 file changed, 6 insertions(+)

include/linux/mm_types.h +0 -0

index 60e4595eaf63..34c7114ea9e9 100644

include/linux/mm_types.h +0 -0

  static inline atomic_t *compound_pincount_ptr(struct page *page)
  {
  	return &page[1].compound_pincount;

Replace three calls to compound_head() with one. This removes the last user of compound_pincount(), so remove that helper too.

Replace three calls to compound_head() with one. This removes the last user of compound_pincount(), so remove that helper too.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  include/linux/mm.h | 21 ++++++++-------------
  1 file changed, 8 insertions(+), 13 deletions(-)

include/linux/mm.h +8 -13

index bf9624ca61c3..d3769897c8ac 100644
@@ -934,13 +934,6 @@ static inline int head_compound_pincount(struct page *head)
  	return atomic_read(compound_pincount_ptr(head));
  }
 
-static inline int compound_pincount(struct page *page)
-{
-	VM_BUG_ON_PAGE(!PageCompound(page), page);
-	page = compound_head(page);
-	return head_compound_pincount(page);
-}
-
  static inline void set_compound_order(struct page *page, unsigned int order)
  {
  	page[1].compound_order = order;

@@ -1340,18 +1333,20 @@ void unpin_user_pages(struct page **pages, unsigned long npages);
   */
  static inline bool page_maybe_dma_pinned(struct page *page)
  {
-	if (PageCompound(page))
-		return compound_pincount(page) > 0;
+	struct folio *folio = page_folio(page);
+
+	if (folio_test_large(folio))
+		return atomic_read(folio_pincount_ptr(folio)) > 0;
 
  	/*
-	 * page_ref_count() is signed. If that refcount overflows, then
-	 * page_ref_count() returns a negative value, and callers will avoid
+	 * folio_ref_count() is signed. If that refcount overflows, then
+	 * folio_ref_count() returns a negative value, and callers will avoid
  	 * further incrementing the refcount.
  	 *
-	 * Here, for that overflow case, use the signed bit to count a little
+	 * Here, for that overflow case, use the sign bit to count a little
  	 * bit higher via unsigned math, and thus still get an accurate result.
  	 */
-	return ((unsigned int)page_ref_count(compound_head(page))) >=
+	return ((unsigned int)folio_ref_count(folio)) >=
  		GUP_PIN_COUNTING_BIAS;
  }
 

Convert try_get_compound_head() into try_get_folio() and convert try_grab_compound_head() into try_grab_folio(). Also convert hpage_pincount_add() to folio_pincount_add(). Add a temporary try_grab_compound_head() wrapper around try_grab_folio() to let us convert callers individually.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c      | 104 +++++++++++++++++++++++++-------------------------
  mm/internal.h |   5 +++
  2 files changed, 56 insertions(+), 53 deletions(-)

mm/gup.c +51 -53

index 1282d29357b7..9e581201d679 100644
@@ -30,21 +30,19 @@ struct follow_page_context {
  };
 
  /*
- * When pinning a compound page, use an exact count to track it, via
- * page_pincount_add/_sub().
+ * When pinning a large folio, use an exact count to track it.
   *
- * However, be sure to *also* increment the normal page refcount field
- * at least once, so that the page really is pinned.  That's why the
- * refcount from the earlier try_get_compound_head() is left intact.
+ * However, be sure to *also* increment the normal folio refcount
+ * field at least once, so that the folio really is pinned.
+ * That's why the refcount from the earlier
+ * try_get_folio() is left intact.
   */
-static void page_pincount_add(struct page *page, int refs)
+static void folio_pincount_add(struct folio *folio, int refs)
  {
-	VM_BUG_ON_PAGE(page != compound_head(page), page);
-
-	if (PageHead(page))
-		atomic_add(refs, compound_pincount_ptr(page));
+	if (folio_test_large(folio))
+		atomic_add(refs, folio_pincount_ptr(folio));
  	else
-		page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
+		folio_ref_add(folio, refs * (GUP_PIN_COUNTING_BIAS - 1));
  }
 
  static int page_pincount_sub(struct page *page, int refs)

@@ -76,75 +74,70 @@ static void put_page_refs(struct page *page, int refs)
  }
 
  /*
- * Return the compound head page with ref appropriately incremented,
+ * Return the folio with ref appropriately incremented,
   * or NULL if that failed.
   */
-static inline struct page *try_get_compound_head(struct page *page, int refs)
+static inline struct folio *try_get_folio(struct page *page, int refs)
  {
-	struct page *head;
+	struct folio *folio;
 
  retry:
-	head = compound_head(page);
-
-	if (WARN_ON_ONCE(page_ref_count(head) < 0))
+	folio = page_folio(page);
+	if (WARN_ON_ONCE(folio_ref_count(folio) < 0))
  		return NULL;
-	if (unlikely(!page_cache_add_speculative(head, refs)))
+	if (unlikely(!folio_ref_try_add_rcu(folio, refs)))
  		return NULL;
 
  	/*
-	 * At this point we have a stable reference to the head page; but it
-	 * could be that between the compound_head() lookup and the refcount
-	 * increment, the compound page was split, in which case we'd end up
-	 * holding a reference on a page that has nothing to do with the page
+	 * At this point we have a stable reference to the folio; but it
+	 * could be that between calling page_folio() and the refcount
+	 * increment, the folio was split, in which case we'd end up
+	 * holding a reference on a folio that has nothing to do with the page
  	 * we were given anymore.
-	 * So now that the head page is stable, recheck that the pages still
-	 * belong together.
+	 * So now that the folio is stable, recheck that the page still
+	 * belongs to this folio.
  	 */
-	if (unlikely(compound_head(page) != head)) {
-		put_page_refs(head, refs);
+	if (unlikely(page_folio(page) != folio)) {
+		folio_put_refs(folio, refs);
  		goto retry;
  	}
 
-	return head;
+	return folio;
  }
 
  /**
- * try_grab_compound_head() - attempt to elevate a page's refcount, by a
- * flags-dependent amount.
- *
- * Even though the name includes "compound_head", this function is still
- * appropriate for callers that have a non-compound @page to get.
- *
+ * try_grab_folio() - Attempt to get or pin a folio.
   * @page:  pointer to page to be grabbed
- * @refs:  the value to (effectively) add to the page's refcount
+ * @refs:  the value to (effectively) add to the folio's refcount
   * @flags: gup flags: these are the FOLL_* flag values.
   *
   * "grab" names in this file mean, "look at flags to decide whether to use
- * FOLL_PIN or FOLL_GET behavior, when incrementing the page's refcount.
+ * FOLL_PIN or FOLL_GET behavior, when incrementing the folio's refcount.
   *
   * Either FOLL_PIN or FOLL_GET (or neither) must be set, but not both at the
   * same time. (That's true throughout the get_user_pages*() and
   * pin_user_pages*() APIs.) Cases:
   *
- *    FOLL_GET: page's refcount will be incremented by @refs.
+ *    FOLL_GET: folio's refcount will be incremented by @refs.
   *
- *    FOLL_PIN on compound pages: page's refcount will be incremented by
- *    @refs, and page[1].compound_pincount will be incremented by @refs.
+ *    FOLL_PIN on large folios: folio's refcount will be incremented by
+ *    @refs, and its compound_pincount will be incremented by @refs.
   *
- *    FOLL_PIN on normal pages: page's refcount will be incremented by
+ *    FOLL_PIN on single-page folios: folio's refcount will be incremented by
   *    @refs * GUP_PIN_COUNTING_BIAS.
   *
- * Return: head page (with refcount appropriately incremented) for success, or
- * NULL upon failure. If neither FOLL_GET nor FOLL_PIN was set, that's
- * considered failure, and furthermore, a likely bug in the caller, so a warning
- * is also emitted.
+ * Return: The folio containing @page (with refcount appropriately
+ * incremented) for success, or NULL upon failure. If neither FOLL_GET
+ * nor FOLL_PIN was set, that's considered failure, and furthermore,
+ * a likely bug in the caller, so a warning is also emitted.
   */
-struct page *try_grab_compound_head(struct page *page,
-				    int refs, unsigned int flags)
+struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
  {
  	if (flags & FOLL_GET)
-		return try_get_compound_head(page, refs);
+		return try_get_folio(page, refs);
  	else if (flags & FOLL_PIN) {
+		struct folio *folio;
+
  		/*
  		 * Can't do FOLL_LONGTERM + FOLL_PIN gup fast path if not in a
  		 * right zone, so fail and let the caller fall back to the slow

@@ -158,21 +151,26 @@ struct page *try_grab_compound_head(struct page *page,
  		 * CAUTION: Don't use compound_head() on the page before this
  		 * point, the result won't be stable.
  		 */
-		page = try_get_compound_head(page, refs);
-		if (!page)
+		folio = try_get_folio(page, refs);
+		if (!folio)
  			return NULL;
 
-		page_pincount_add(page, refs);
-		mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_ACQUIRED,
-				    refs);
+		folio_pincount_add(folio, refs);
+		node_stat_mod_folio(folio, NR_FOLL_PIN_ACQUIRED, refs);
 
-		return page;
+		return folio;
  	}
 
  	WARN_ON_ONCE(1);
  	return NULL;
  }
 
+struct page *try_grab_compound_head(struct page *page,
+		int refs, unsigned int flags)
+{
+	return &try_grab_folio(page, refs, flags)->page;
+}
+
  static void put_compound_head(struct page *page, int refs, unsigned int flags)
  {
  	if (flags & FOLL_PIN) {

@@ -196,7 +194,7 @@ static void put_compound_head(struct page *page, int refs, unsigned int flags)
   * @flags:   gup flags: these are the FOLL_* flag values.
   *
   * Either FOLL_PIN or FOLL_GET (or neither) may be set, but not both at the same
- * time. Cases: please see the try_grab_compound_head() documentation, with
+ * time. Cases: please see the try_grab_folio() documentation, with
   * "refs=1".
   *
   * Return: true for success, or if no action was required (if neither FOLL_PIN

mm/internal.h +5 -0

index 26af8a5a5be3..9a72d1ecdab4 100644
@@ -723,4 +723,9 @@ void vunmap_range_noflush(unsigned long start, unsigned long end);
  int numa_migrate_prep(struct page *page, struct vm_area_struct *vma,
  		      unsigned long addr, int page_nid, int *flags);
 
+/*
+ * mm/gup.c
+ */
+struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
+
  #endif	/* __MM_INTERNAL_H */

These wrappers have no more callers, so delete them.

These wrappers have no more callers, so delete them.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  include/linux/mm.h      |  7 +++----
  include/linux/pagemap.h | 11 -----------
  2 files changed, 3 insertions(+), 15 deletions(-)

include/linux/mm.h +3 -4

index d3769897c8ac..b249156f7cf1 100644
@@ -1291,10 +1291,9 @@ static inline void put_page(struct page *page)
   * applications that don't have huge page reference counts, this won't be an
   * issue.
   *
- * Locking: the lockless algorithm described in page_cache_get_speculative()
- * and page_cache_gup_pin_speculative() provides safe operation for
- * get_user_pages and page_mkclean and other calls that race to set up page
- * table entries.
+ * Locking: the lockless algorithm described in folio_try_get_rcu()
+ * provides safe operation for get_user_pages(), page_mkclean() and
+ * other calls that race to set up page table entries.
   */
  #define GUP_PIN_COUNTING_BIAS (1U << 10)
 

include/linux/pagemap.h +0 -11

index 704cb1b4b15d..4a63176b6417 100644
@@ -283,17 +283,6 @@ static inline struct inode *folio_inode(struct folio *folio)
  	return folio->mapping->host;
  }
 
-static inline bool page_cache_add_speculative(struct page *page, int count)
-{
-	VM_BUG_ON_PAGE(PageTail(page), page);
-	return folio_ref_try_add_rcu((struct folio *)page, count);
-}
-
-static inline bool page_cache_get_speculative(struct page *page)
-{
-	return page_cache_add_speculative(page, 1);
-}
-
  /**
   * folio_attach_private - Attach private data to a folio.
   * @folio: Folio to attach data to.

Convert put_compound_head() to gup_put_folio() and hpage_pincount_sub() to folio_pincount_sub(). This removes the last call to put_page_refs(), so delete it. Add a temporary put_compound_head() wrapper which will be deleted by the end of this series.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 42 ++++++++++++++----------------------------
  1 file changed, 14 insertions(+), 28 deletions(-)

mm/gup.c +0 -0

index 9e581201d679..719252fa0402 100644

mm/gup.c +13 -27

  {
-	VM_BUG_ON_PAGE(page != compound_head(page), page);
-
-	if (PageHead(page))
-		atomic_sub(refs, compound_pincount_ptr(page));
+	if (folio_test_large(folio))
+		atomic_sub(refs, folio_pincount_ptr(folio));
  	else
  		refs *= GUP_PIN_COUNTING_BIAS;
  	return refs;
  }
 
-/* Equivalent to calling put_page() @refs times. */
-static void put_page_refs(struct page *page, int refs)
-{
-#ifdef CONFIG_DEBUG_VM
-	if (VM_WARN_ON_ONCE_PAGE(page_ref_count(page) < refs, page))
-		return;
-#endif
-
-	/*
-	 * Calling put_page() for each ref is unnecessarily slow. Only the last
-	 * ref needs a put_page().
-	 */
-	if (refs > 1)
-		page_ref_sub(page, refs - 1);
-	put_page(page);
-}
-
  /*
   * Return the folio with ref appropriately incremented,
   * or NULL if that failed.

@@ -171,15 +152,20 @@ struct page *try_grab_compound_head(struct page *page,
  	return &try_grab_folio(page, refs, flags)->page;
  }
 
-static void put_compound_head(struct page *page, int refs, unsigned int flags)
+static void gup_put_folio(struct folio *folio, int refs, unsigned int flags)
  {
  	if (flags & FOLL_PIN) {
-		mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_RELEASED,
-				    refs);
-		refs = page_pincount_sub(page, refs);
+		node_stat_mod_folio(folio, NR_FOLL_PIN_RELEASED, refs);
+		refs = folio_pincount_sub(folio, refs);
  	}
 
-	put_page_refs(page, refs);
+	folio_put_refs(folio, refs);
+}
+
+static void put_compound_head(struct page *page, int refs, unsigned int flags)
+{
+	VM_BUG_ON_PAGE(PageTail(page), page);
+	gup_put_folio((struct folio *)page, refs, flags);
  }
 
  /**

@@ -220,7 +206,7 @@ bool __must_check try_grab_page(struct page *page, unsigned int flags)
   */
  void unpin_user_page(struct page *page)
  {
-	put_compound_head(compound_head(page), 1, FOLL_PIN);
+	gup_put_folio(page_folio(page), 1, FOLL_PIN);
  }
  EXPORT_SYMBOL(unpin_user_page);
 

follow_hugetlb_page() only cares about success or failure, so it doesn't need to know the type of the returned pointer, only whether it's NULL or not.

follow_hugetlb_page() only cares about success or failure, so it doesn't need to know the type of the returned pointer, only whether it's NULL or not.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  include/linux/mm.h | 3 ---
  mm/gup.c           | 2 +-
  mm/hugetlb.c       | 7 +++----
  3 files changed, 4 insertions(+), 8 deletions(-)

include/linux/mm.h +0 -3

index b249156f7cf1..c103c6401ecd 100644
@@ -1195,9 +1195,6 @@ static inline void get_page(struct page *page)
  }
 
  bool __must_check try_grab_page(struct page *page, unsigned int flags);
-struct page *try_grab_compound_head(struct page *page, int refs,
-				    unsigned int flags);
-
 
  static inline __must_check bool try_get_page(struct page *page)
  {

mm/gup.c +1 -1

index 719252fa0402..20703de2f107 100644
@@ -146,7 +146,7 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
  	return NULL;
  }
 
-struct page *try_grab_compound_head(struct page *page,
+static inline struct page *try_grab_compound_head(struct page *page,
  		int refs, unsigned int flags)
  {
  	return &try_grab_folio(page, refs, flags)->page;

mm/hugetlb.c +3 -4

index abcd1785c629..ab67b13c4a71 100644
@@ -6072,7 +6072,7 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
 
  		if (pages) {
  			/*
-			 * try_grab_compound_head() should always succeed here,
+			 * try_grab_folio() should always succeed here,
  			 * because: a) we hold the ptl lock, and b) we've just
  			 * checked that the huge page is present in the page
  			 * tables. If the huge page is present, then the tail

@@ -6081,9 +6081,8 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
  			 * any way. So this page must be available at this
  			 * point, unless the page refcount overflowed:
  			 */
-			if (WARN_ON_ONCE(!try_grab_compound_head(pages[i],
-								 refs,
-								 flags))) {
+			if (WARN_ON_ONCE(!try_grab_folio(pages[i], refs,
+							 flags))) {
  				spin_unlock(ptl);
  				remainder = 0;
  				err = -ENOMEM;

try_grab_page() only cares about success or failure, not about the type returned.

try_grab_page() only cares about success or failure, not about the type returned.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

mm/gup.c +1 -1

index 20703de2f107..c3e514172eaf 100644
@@ -192,7 +192,7 @@ bool __must_check try_grab_page(struct page *page, unsigned int flags)
  	if (!(flags & (FOLL_GET | FOLL_PIN)))
  		return true;
 
-	return try_grab_compound_head(page, 1, flags);
+	return try_grab_folio(page, 1, flags);
  }
 
  /**

We still call try_grab_folio() once per PTE; a future patch could optimise to just adjust the reference count for each page within the folio.

We still call try_grab_folio() once per PTE; a future patch could optimise to just adjust the reference count for each page within the folio.

Signed-off-by Matthew Wilcox (Oracle) <[email protected]>

---
  mm/gup.c | 16 +++++++---------
  1 file changed, 7 insertions(+), 9 deletions(-)