[PATCH 0/5] Replace is_longterm_pinnable_page()

[PATCH 0/5] Replace is_longterm_pinnable_page()

[Vishal Moola (Oracle)]

This patchset introduces some more helper functions for the folio
conversions, and converts all callers of is_longterm_pinnable_page() to
use folios.

Vishal Moola (Oracle) (5):
  mmzone: Introduce folio_is_zone_movable()
  mmzone: Introduce folio_migratetype()
  mm/gup_test.c: Convert verify_dma_pinned() to us folios
  mm/gup.c: Reorganize try_get_folio()
  mm: Remove is_longterm_pinnable_page() and Reimplement
    folio_is_longterm_pinnable()

 include/linux/mm.h     | 22 +++++------
 include/linux/mmzone.h |  8 ++++
 mm/gup.c               | 88 ++++++++++++++++++++++--------------------
 mm/gup_test.c          | 13 ++++---
 4 files changed, 70 insertions(+), 61 deletions(-)

-- 
2.40.1

[PATCH 1/5] mmzone: Introduce folio_is_zone_movable()

[Vishal Moola (Oracle)]

Introduce folio_is_zone_movable() to act as a folio equivalent for
is_zone_movable_page(). This is to assist in later folio conversions.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 include/linux/mmzone.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index a4889c9d4055..744bf32e48a8 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1116,6 +1116,11 @@ static inline bool is_zone_movable_page(const struct page *page)
 {
 	return page_zonenum(page) == ZONE_MOVABLE;
 }
+
+static inline bool folio_is_zone_movable(const struct folio *folio)
+{
+	return folio_zonenum(folio) == ZONE_MOVABLE;
+}
 #endif
 
 /*
-- 
2.40.1

[PATCH 2/5] mmzone: Introduce folio_migratetype()

[Vishal Moola (Oracle)]

Introduce folio_migratetype() as a folio equivalent for
get_pageblock_migratetype(). This function intends to return the
migratetype the folio is located in, hence the name choice.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 include/linux/mmzone.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 744bf32e48a8..b58c76e68ac7 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -105,6 +105,9 @@ extern int page_group_by_mobility_disabled;
 #define get_pageblock_migratetype(page)					\
 	get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
 
+#define folio_migratetype(folio)				\
+	get_pfnblock_flags_mask(&folio->page, folio_pfn(folio),		\
+			MIGRATETYPE_MASK)
 struct free_area {
 	struct list_head	free_list[MIGRATE_TYPES];
 	unsigned long		nr_free;
-- 
2.40.1

[PATCH 3/5] mm/gup_test.c: Convert verify_dma_pinned() to us folios

[Vishal Moola (Oracle)]

verify_dma_pinned() checks that pages are dma-pinned. We can convert
this to use folios.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/gup_test.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/mm/gup_test.c b/mm/gup_test.c
index 8ae7307a1bb6..860b093b4b3e 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -40,24 +40,25 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
 			      unsigned long nr_pages)
 {
 	unsigned long i;
-	struct page *page;
+	struct folio *folio;
 
 	switch (cmd) {
 	case PIN_FAST_BENCHMARK:
 	case PIN_BASIC_TEST:
 	case PIN_LONGTERM_BENCHMARK:
 		for (i = 0; i < nr_pages; i++) {
-			page = pages[i];
-			if (WARN(!page_maybe_dma_pinned(page),
+			folio = page_folio(pages[i]);
+
+			if (WARN(!folio_maybe_dma_pinned(folio),
 				 "pages[%lu] is NOT dma-pinned\n", i)) {
 
-				dump_page(page, "gup_test failure");
+				dump_page(&folio->page, "gup_test failure");
 				break;
 			} else if (cmd == PIN_LONGTERM_BENCHMARK &&
-				WARN(!is_longterm_pinnable_page(page),
+				WARN(!folio_is_longterm_pinnable(folio),
 				     "pages[%lu] is NOT pinnable but pinned\n",
 				     i)) {
-				dump_page(page, "gup_test failure");
+				dump_page(&folio->page, "gup_test failure");
 				break;
 			}
 		}
-- 
2.40.1

[PATCH 4/5] mm/gup.c: Reorganize try_get_folio()

[Vishal Moola (Oracle)]

try_get_folio() takes in a page, then chooses to do some folio
operations based on the flags (either FOLL_GET or FOLL_PIN).
We can rewrite this function to be more purpose oriented.

After calling try_get_folio(), if FOLL_GET is set we can return the
result and end the function. If FOLL_GET is not set and FOLL_PIN is not
set then it's a bug so we warn and fail. Otherwise we simply proceed to
pin the folio and return that as well.

This change assists with folio conversions, and makes the function more
readable.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/gup.c | 88 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 46 insertions(+), 42 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index bbe416236593..adbd81f888f5 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -123,58 +123,62 @@ static inline struct folio *try_get_folio(struct page *page, int refs)
  */
 struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
 {
+	struct folio *folio;
 	if (unlikely(!(flags & FOLL_PCI_P2PDMA) && is_pci_p2pdma_page(page)))
 		return NULL;
 
+	folio = try_get_folio(page, refs);
+
 	if (flags & FOLL_GET)
-		return try_get_folio(page, refs);
-	else if (flags & FOLL_PIN) {
-		struct folio *folio;
+		return 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
-		 * path.
-		 */
-		if (unlikely((flags & FOLL_LONGTERM) &&
-			     !is_longterm_pinnable_page(page)))
-			return NULL;
+	if (unlikely(!(flags & FOLL_PIN))) {
+		WARN_ON_ONCE(1);
+		return NULL;
+	}
 
-		/*
-		 * CAUTION: Don't use compound_head() on the page before this
-		 * point, the result won't be stable.
-		 */
-		folio = try_get_folio(page, refs);
-		if (!folio)
-			return NULL;
+	/*
+	 * CAUTION: Don't use compound_head() on the page before this
+	 * point, the result won't be stable.
+	 */
+	if (!folio)
+		return NULL;
 
-		/*
-		 * When pinning a large folio, use an exact count to track it.
-		 *
-		 * 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.
-		 */
-		if (folio_test_large(folio))
-			atomic_add(refs, &folio->_pincount);
-		else
-			folio_ref_add(folio,
-					refs * (GUP_PIN_COUNTING_BIAS - 1));
-		/*
-		 * Adjust the pincount before re-checking the PTE for changes.
-		 * This is essentially a smp_mb() and is paired with a memory
-		 * barrier in page_try_share_anon_rmap().
-		 */
-		smp_mb__after_atomic();
+	/*
+	 * 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
+	 * path.
+	 */
+	if (unlikely((flags & FOLL_LONGTERM) &&
+		     !folio_is_longterm_pinnable(folio))) {
+		if (!put_devmap_managed_page_refs(&folio->page, refs))
+			folio_put_refs(folio, refs);
+		return NULL;
+	}
 
-		node_stat_mod_folio(folio, NR_FOLL_PIN_ACQUIRED, refs);
+	/*
+	 * When pinning a large folio, use an exact count to track it.
+	 *
+	 * 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.
+	 */
+	if (folio_test_large(folio))
+		atomic_add(refs, &folio->_pincount);
+	else
+		folio_ref_add(folio,
+				refs * (GUP_PIN_COUNTING_BIAS - 1));
+	/*
+	 * Adjust the pincount before re-checking the PTE for changes.
+	 * This is essentially a smp_mb() and is paired with a memory
+	 * barrier in page_try_share_anon_rmap().
+	 */
+	smp_mb__after_atomic();
 
-		return folio;
-	}
+	node_stat_mod_folio(folio, NR_FOLL_PIN_ACQUIRED, refs);
 
-	WARN_ON_ONCE(1);
-	return NULL;
+	return folio;
 }
 
 static void gup_put_folio(struct folio *folio, int refs, unsigned int flags)
-- 
2.40.1

[PATCH 5/5] mm: Remove is_longterm_pinnable_page() and Reimplement folio_is_longterm_pinnable()

[Vishal Moola (Oracle)]

folio_is_longterm_pinnable() already exists as a wrapper function. Now
that the whole implementation of is_longterm_pinnable_page() can be
implemented using folios, folio_is_longterm_pinnable() can be made its
own standalone function - and we can remove is_longterm_pinnable_page().

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 include/linux/mm.h | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..e2d35e272e07 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1910,39 +1910,35 @@ static inline bool page_needs_cow_for_dma(struct vm_area_struct *vma,
 	return page_maybe_dma_pinned(page);
 }
 
-/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin pages */
+/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin folios */
 #ifdef CONFIG_MIGRATION
-static inline bool is_longterm_pinnable_page(struct page *page)
+static inline bool folio_is_longterm_pinnable(struct folio *folio)
 {
 #ifdef CONFIG_CMA
-	int mt = get_pageblock_migratetype(page);
+	int mt = folio_migratetype(folio);
 
 	if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
 		return false;
 #endif
 	/* The zero page may always be pinned */
-	if (is_zero_pfn(page_to_pfn(page)))
+	if (is_zero_pfn(folio_pfn(folio)))
 		return true;
 
 	/* Coherent device memory must always allow eviction. */
-	if (is_device_coherent_page(page))
+	if (folio_is_device_coherent(folio))
 		return false;
 
-	/* Otherwise, non-movable zone pages can be pinned. */
-	return !is_zone_movable_page(page);
+	/* Otherwise, non-movable zone folios can be pinned. */
+	return !folio_is_zone_movable(folio);
+
 }
 #else
-static inline bool is_longterm_pinnable_page(struct page *page)
+static inline bool folio_is_longterm_pinnable(struct folio *folio)
 {
 	return true;
 }
 #endif
 
-static inline bool folio_is_longterm_pinnable(struct folio *folio)
-{
-	return is_longterm_pinnable_page(&folio->page);
-}
-
 static inline void set_page_zone(struct page *page, enum zone_type zone)
 {
 	page->flags &= ~(ZONES_MASK << ZONES_PGSHIFT);
-- 
2.40.1

Re: [PATCH 4/5] mm/gup.c: Reorganize try_get_folio()

[Matthew Wilcox]

On Tue, Jun 13, 2023 at 01:18:26PM -0700, Vishal Moola (Oracle) wrote:
>  struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
>  {
> +	struct folio *folio;

checkpatch will whinge about there not being a blank line here, and in
this case, I think it's correct.

>  	if (unlikely(!(flags & FOLL_PCI_P2PDMA) && is_pci_p2pdma_page(page)))
>  		return NULL;
>  
> +	folio = try_get_folio(page, refs);
> +
>  	if (flags & FOLL_GET)
> -		return try_get_folio(page, refs);
> -	else if (flags & FOLL_PIN) {
> -		struct folio *folio;
> +		return 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
> -		 * path.
> -		 */
> -		if (unlikely((flags & FOLL_LONGTERM) &&
> -			     !is_longterm_pinnable_page(page)))
> -			return NULL;
> +	if (unlikely(!(flags & FOLL_PIN))) {
> +		WARN_ON_ONCE(1);
> +		return NULL;

Don't we need to folio_put_refs() in this case?  Or rather, I think the

	if (WARN_ON_ONCE(flags & (FOLL_PIN|FOLL_GET) == 0) {

test should be first.

> +	/*
> +	 * CAUTION: Don't use compound_head() on the page before this
> +	 * point, the result won't be stable.
> +	 */

I think we can lose the comment at this point?

> +	if (!folio)
> +		return NULL;

Re: [PATCH 5/5] mm: Remove is_longterm_pinnable_page() and Reimplement folio_is_longterm_pinnable()

[Matthew Wilcox]

On Tue, Jun 13, 2023 at 01:18:27PM -0700, Vishal Moola (Oracle) wrote:
> folio_is_longterm_pinnable() already exists as a wrapper function. Now
> that the whole implementation of is_longterm_pinnable_page() can be
> implemented using folios, folio_is_longterm_pinnable() can be made its
> own standalone function - and we can remove is_longterm_pinnable_page().

Yay, another wrapper function bites the dust.

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 1/5] mmzone: Introduce folio_is_zone_movable()

[Matthew Wilcox]

On Tue, Jun 13, 2023 at 01:18:23PM -0700, Vishal Moola (Oracle) wrote:
> Introduce folio_is_zone_movable() to act as a folio equivalent for
> is_zone_movable_page(). This is to assist in later folio conversions.

Honestly, I thought we already had this one.

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 2/5] mmzone: Introduce folio_migratetype()

[Matthew Wilcox]

On Tue, Jun 13, 2023 at 01:18:24PM -0700, Vishal Moola (Oracle) wrote:
> Introduce folio_migratetype() as a folio equivalent for
> get_pageblock_migratetype(). This function intends to return the
> migratetype the folio is located in, hence the name choice.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 3/5] mm/gup_test.c: Convert verify_dma_pinned() to us folios

[Matthew Wilcox]

On Tue, Jun 13, 2023 at 01:18:25PM -0700, Vishal Moola (Oracle) wrote:
> verify_dma_pinned() checks that pages are dma-pinned. We can convert
> this to use folios.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 3/5] mm/gup_test.c: Convert verify_dma_pinned() to us folios

[Lorenzo Stoakes]

On Tue, 13 Jun 2023 at 21:18, Vishal Moola (Oracle)
<vishal.moola@gmail.com> wrote:
>
> verify_dma_pinned() checks that pages are dma-pinned. We can convert
> this to use folios.
>
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
>  mm/gup_test.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/mm/gup_test.c b/mm/gup_test.c
> index 8ae7307a1bb6..860b093b4b3e 100644
> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -40,24 +40,25 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
>                               unsigned long nr_pages)
>  {
>         unsigned long i;
> -       struct page *page;
> +       struct folio *folio;
>
>         switch (cmd) {
>         case PIN_FAST_BENCHMARK:
>         case PIN_BASIC_TEST:
>         case PIN_LONGTERM_BENCHMARK:
>                 for (i = 0; i < nr_pages; i++) {
> -                       page = pages[i];
> -                       if (WARN(!page_maybe_dma_pinned(page),
> +                       folio = page_folio(pages[i]);
> +
> +                       if (WARN(!folio_maybe_dma_pinned(folio),
>                                  "pages[%lu] is NOT dma-pinned\n", i)) {
>
> -                               dump_page(page, "gup_test failure");
> +                               dump_page(&folio->page, "gup_test failure");
>                                 break;
>                         } else if (cmd == PIN_LONGTERM_BENCHMARK &&
> -                               WARN(!is_longterm_pinnable_page(page),
> +                               WARN(!folio_is_longterm_pinnable(folio),
>                                      "pages[%lu] is NOT pinnable but pinned\n",
>                                      i)) {
> -                               dump_page(page, "gup_test failure");
> +                               dump_page(&folio->page, "gup_test failure");
>                                 break;
>                         }
>                 }
> --
> 2.40.1
>
>

(Hope you don't mind me taking a look, GUP is of interest to me :)

Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com>