[PATCH 0/3] Convert a couple migrate functions to use folios

[PATCH 0/3] Convert a couple migrate functions to use folios

[Vishal Moola (Oracle)]

This patch set introduces folio_movable_ops() and converts 3 functions
in mm/migrate.c to use folios.

---
This can go under Sid's hugetlb fault function folio conversions as
suggested by Matthew.

Link: https://lore.kernel.org/linux-mm/Y8orMisdXzaUI0t8@casper.infradead.org/

Vishal Moola (Oracle) (3):
  mm: Add folio_movable_ops()
  mm/migrate: Convert isolate_movable_page() to use folios
  mm/migrate: Convert putback_movable_pages() to use folios

 include/linux/migrate.h |  9 +++++
 mm/migrate.c            | 87 +++++++++++++++++++++--------------------
 2 files changed, 53 insertions(+), 43 deletions(-)

-- 
2.38.1

[PATCH 1/3] mm: Add folio_movable_ops()

[Vishal Moola (Oracle)]

folio_movable_ops() does the same as page_movable_ops() except uses
folios instead of pages. This function will help make folio conversions
in mm/migrate.c more readable.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 include/linux/migrate.h | 9 +++++++++
 mm/migrate.c            | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 3ef77f52a4f0..bdff950a8bb4 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -122,6 +122,15 @@ static inline bool folio_test_movable(struct folio *folio)
 	return PageMovable(&folio->page);
 }
 
+static inline
+const struct movable_operations *folio_movable_ops(struct folio *folio)
+{
+	VM_BUG_ON(!__folio_test_movable(folio));
+
+	return (const struct movable_operations *)
+		((unsigned long)folio->mapping - PAGE_MAPPING_MOVABLE);
+}
+
 static inline
 const struct movable_operations *page_movable_ops(struct page *page)
 {
diff --git a/mm/migrate.c b/mm/migrate.c
index a4d3fc65085f..4c1776445c74 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -973,7 +973,7 @@ static int move_to_new_folio(struct folio *dst, struct folio *src,
 			goto out;
 		}
 
-		mops = page_movable_ops(&src->page);
+		mops = folio_movable_ops(src);
 		rc = mops->migrate_page(&dst->page, &src->page, mode);
 		WARN_ON_ONCE(rc == MIGRATEPAGE_SUCCESS &&
 				!folio_test_isolated(src));
-- 
2.38.1

[PATCH 2/3] mm/migrate: Convert isolate_movable_page() to use folios

[Vishal Moola (Oracle)]

Removes 6 calls to compound_head() and prepares the function to take in a
folio instead of page argument.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/migrate.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 4c1776445c74..bcde3cbbc8c9 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -60,6 +60,7 @@
 
 int isolate_movable_page(struct page *page, isolate_mode_t mode)
 {
+	struct folio *folio = page_folio(page);
 	const struct movable_operations *mops;
 
 	/*
@@ -71,11 +72,11 @@ int isolate_movable_page(struct page *page, isolate_mode_t mode)
 	 * the put_page() at the end of this block will take care of
 	 * release this page, thus avoiding a nasty leakage.
 	 */
-	if (unlikely(!get_page_unless_zero(page)))
+	if (unlikely(!folio_try_get(folio)))
 		goto out;
 
-	if (unlikely(PageSlab(page)))
-		goto out_putpage;
+	if (unlikely(folio_test_slab(folio)))
+		goto out_putfolio;
 	/* Pairs with smp_wmb() in slab freeing, e.g. SLUB's __free_slab() */
 	smp_rmb();
 	/*
@@ -83,12 +84,12 @@ int isolate_movable_page(struct page *page, isolate_mode_t mode)
 	 * we use non-atomic bitops on newly allocated page flags so
 	 * unconditionally grabbing the lock ruins page's owner side.
 	 */
-	if (unlikely(!__PageMovable(page)))
-		goto out_putpage;
+	if (unlikely(!__folio_test_movable(folio)))
+		goto out_putfolio;
 	/* Pairs with smp_wmb() in slab allocation, e.g. SLUB's alloc_slab_page() */
 	smp_rmb();
-	if (unlikely(PageSlab(page)))
-		goto out_putpage;
+	if (unlikely(folio_test_slab(folio)))
+		goto out_putfolio;
 
 	/*
 	 * As movable pages are not isolated from LRU lists, concurrent
@@ -101,29 +102,29 @@ int isolate_movable_page(struct page *page, isolate_mode_t mode)
 	 * lets be sure we have the page lock
 	 * before proceeding with the movable page isolation steps.
 	 */
-	if (unlikely(!trylock_page(page)))
-		goto out_putpage;
+	if (unlikely(!folio_trylock(folio)))
+		goto out_putfolio;
 
-	if (!PageMovable(page) || PageIsolated(page))
+	if (!folio_test_movable(folio) || folio_test_isolated(folio))
 		goto out_no_isolated;
 
-	mops = page_movable_ops(page);
-	VM_BUG_ON_PAGE(!mops, page);
+	mops = folio_movable_ops(folio);
+	VM_BUG_ON_FOLIO(!mops, folio);
 
-	if (!mops->isolate_page(page, mode))
+	if (!mops->isolate_page(&folio->page, mode))
 		goto out_no_isolated;
 
 	/* Driver shouldn't use PG_isolated bit of page->flags */
-	WARN_ON_ONCE(PageIsolated(page));
-	SetPageIsolated(page);
-	unlock_page(page);
+	WARN_ON_ONCE(folio_test_isolated(folio));
+	folio_set_isolated(folio);
+	folio_unlock(folio);
 
 	return 0;
 
 out_no_isolated:
-	unlock_page(page);
-out_putpage:
-	put_page(page);
+	folio_unlock(folio);
+out_putfolio:
+	folio_put(folio);
 out:
 	return -EBUSY;
 }
-- 
2.38.1

[PATCH 3/3] mm/migrate: Convert putback_movable_pages() to use folios

[Vishal Moola (Oracle)]

Removes 6 calls to compound_head(), and replaces putback_movable_page()
with putback_movable_folio() as well.

Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/migrate.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index bcde3cbbc8c9..de65f7146ee6 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -129,12 +129,12 @@ int isolate_movable_page(struct page *page, isolate_mode_t mode)
 	return -EBUSY;
 }
 
-static void putback_movable_page(struct page *page)
+static void putback_movable_folio(struct folio *folio)
 {
-	const struct movable_operations *mops = page_movable_ops(page);
+	const struct movable_operations *mops = folio_movable_ops(folio);
 
-	mops->putback_page(page);
-	ClearPageIsolated(page);
+	mops->putback_page(&folio->page);
+	folio_clear_isolated(folio);
 }
 
 /*
@@ -147,33 +147,33 @@ static void putback_movable_page(struct page *page)
  */
 void putback_movable_pages(struct list_head *l)
 {
-	struct page *page;
-	struct page *page2;
+	struct folio *folio;
+	struct folio *folio2;
 
-	list_for_each_entry_safe(page, page2, l, lru) {
-		if (unlikely(PageHuge(page))) {
-			putback_active_hugepage(page);
+	list_for_each_entry_safe(folio, folio2, l, lru) {
+		if (unlikely(folio_test_hugetlb(folio))) {
+			putback_active_hugepage(&folio->page);
 			continue;
 		}
-		list_del(&page->lru);
+		list_del(&folio->lru);
 		/*
-		 * We isolated non-lru movable page so here we can use
-		 * __PageMovable because LRU page's mapping cannot have
+		 * We isolated non-lru movable folio so here we can use
+		 * __PageMovable because LRU folio's mapping cannot have
 		 * PAGE_MAPPING_MOVABLE.
 		 */
-		if (unlikely(__PageMovable(page))) {
-			VM_BUG_ON_PAGE(!PageIsolated(page), page);
-			lock_page(page);
-			if (PageMovable(page))
-				putback_movable_page(page);
+		if (unlikely(__folio_test_movable(folio))) {
+			VM_BUG_ON_FOLIO(!folio_test_isolated(folio), folio);
+			folio_lock(folio);
+			if (folio_test_movable(folio))
+				putback_movable_folio(folio);
 			else
-				ClearPageIsolated(page);
-			unlock_page(page);
-			put_page(page);
+				folio_clear_isolated(folio);
+			folio_unlock(folio);
+			folio_put(folio);
 		} else {
-			mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
-					page_is_file_lru(page), -thp_nr_pages(page));
-			putback_lru_page(page);
+			node_stat_mod_folio(folio, NR_ISOLATED_ANON +
+					folio_is_file_lru(folio), -folio_nr_pages(folio));
+			folio_putback_lru(folio);
 		}
 	}
 }
-- 
2.38.1

Re: [PATCH 2/3] mm/migrate: Convert isolate_movable_page() to use folios

[Matthew Wilcox]

On Fri, Jan 20, 2023 at 04:56:21PM -0800, Vishal Moola (Oracle) wrote:
>  int isolate_movable_page(struct page *page, isolate_mode_t mode)
>  {
> +	struct folio *folio = page_folio(page);
>  	const struct movable_operations *mops;
>  
>  	/*
> @@ -71,11 +72,11 @@ int isolate_movable_page(struct page *page, isolate_mode_t mode)
>  	 * the put_page() at the end of this block will take care of
>  	 * release this page, thus avoiding a nasty leakage.
>  	 */
> -	if (unlikely(!get_page_unless_zero(page)))
> +	if (unlikely(!folio_try_get(folio)))

This changes behaviour.  Previously when called on a tail page, the
call failed.  Now it succeeds, getting a ref on something that at
least was the folio head at some point.

If you're going to do this, you need to recheck that the page is still
part of the folio after getting the ref (see gup.c for an example).
But I think we should probably maintain the behaviour of failing on
tail pages.

Maybe something like ...

	if (unlikely(!get_page_unless_zero(page)))
		goto out;
	/* Refcount is zero on tail pages, so we must have a head */
	folio = (struct folio *)page;

Re: [PATCH 2/3] mm/migrate: Convert isolate_movable_page() to use folios

[Matthew Wilcox]

On Sun, Jan 22, 2023 at 12:46:34PM +0000, Matthew Wilcox wrote:
> On Fri, Jan 20, 2023 at 04:56:21PM -0800, Vishal Moola (Oracle) wrote:
> >  int isolate_movable_page(struct page *page, isolate_mode_t mode)
> >  {
> > +	struct folio *folio = page_folio(page);
> >  	const struct movable_operations *mops;
> >  
> >  	/*
> > @@ -71,11 +72,11 @@ int isolate_movable_page(struct page *page, isolate_mode_t mode)
> >  	 * the put_page() at the end of this block will take care of
> >  	 * release this page, thus avoiding a nasty leakage.
> >  	 */
> > -	if (unlikely(!get_page_unless_zero(page)))
> > +	if (unlikely(!folio_try_get(folio)))
> 
> This changes behaviour.  Previously when called on a tail page, the
> call failed.  Now it succeeds, getting a ref on something that at
> least was the folio head at some point.
> 
> If you're going to do this, you need to recheck that the page is still
> part of the folio after getting the ref (see gup.c for an example).
> But I think we should probably maintain the behaviour of failing on
> tail pages.
> 
> Maybe something like ...
> 
> 	if (unlikely(!get_page_unless_zero(page)))
> 		goto out;
> 	/* Refcount is zero on tail pages, so we must have a head */
> 	folio = (struct folio *)page;

I've been thinking about this some more as I don't like doing these
kinds of casts (except in the helper functions).  What do you think
to adding:

struct folio *folio_get_nontail_page(struct page *)
{
	if unlikely(!get_page_unless_zero(page))
		return NULL;
	return (struct folio *)page;
}

and then isolate_movable_page() looks like:

	struct folio *folio;
[...]

	folio = folio_get_nontail_page(page);
	if (!folio)
		goto out;

I keep thinking about how this is all going to work when we get to
one-pointer-per-page.  Telling tail pages from head pages becomes hard.
This probably becomes an out-of-line function that looks something like ..

	struct memdesc *memdesc = READ_ONCE(page->memdesc);
	struct folio *folio;

	if (!memdesc_is_folio(memdesc))
		return NULL;
	folio = memdesc_folio(memdesc);
	if (!folio_try_get(folio))
		return NULL;
	if (READ_ONCE(page->memdesc) != memdesc ||
	    folio->pfn != page_pfn(page)) {
		folio_put(folio);
		return NULL;
	}

	return folio;

(note: We need to check that page->memdesc still points to this folio after 
getting the refcount on it.  we could loop around if it fails, but
failing the entire get is OK; if memdesc changed then either before this
function was called, after this function was called or while this
function was called, the refcount on the memdesc it was pointing to
was zero)