... and factor the complete handling of movable_ops pages out.
Convert it similar to isolate_movable_ops_page().

While at it, convert the VM_BUG_ON_FOLIO() into a VM_WARN_ON_PAGE().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 mm/migrate.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 6bbb455f8b593..32e77898f7d6c 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -133,12 +133,30 @@ bool isolate_movable_ops_page(struct page *page, isolate_mode_t mode)
 	return false;
 }
 
-static void putback_movable_folio(struct folio *folio)
+/**
+ * putback_movable_ops_page - putback an isolated movable_ops page
+ * @page: The isolated page.
+ *
+ * Putback an isolated movable_ops page.
+ *
+ * After the page was putback, it might get freed instantly.
+ */
+static void putback_movable_ops_page(struct page *page)
 {
-	const struct movable_operations *mops = folio_movable_ops(folio);
-
-	mops->putback_page(&folio->page);
-	folio_clear_isolated(folio);
+	/*
+	 * TODO: these pages will not be folios in the future. All
+	 * folio dependencies will have to be removed.
+	 */
+	struct folio *folio = page_folio(page);
+
+	VM_WARN_ON_ONCE_PAGE(!PageIsolated(page), page);
+	folio_lock(folio);
+	/* If the page was released by it's owner, there is nothing to do. */
+	if (PageMovable(page))
+		page_movable_ops(page)->putback_page(page);
+	ClearPageIsolated(page);
+	folio_unlock(folio);
+	folio_put(folio);
 }
 
 /*
@@ -166,14 +184,7 @@ void putback_movable_pages(struct list_head *l)
 		 * have PAGE_MAPPING_MOVABLE.
 		 */
 		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
-				folio_clear_isolated(folio);
-			folio_unlock(folio);
-			folio_put(folio);
+			putback_movable_ops_page(&folio->page);
 		} else {
 			node_stat_mod_folio(folio, NR_ISOLATED_ANON +
 					folio_is_file_lru(folio), -folio_nr_pages(folio));
-- 
2.49.0

On 18 Jun 2025, at 13:39, David Hildenbrand wrote:

> ... and factor the complete handling of movable_ops pages out.
> Convert it similar to isolate_movable_ops_page().
>
> While at it, convert the VM_BUG_ON_FOLIO() into a VM_WARN_ON_PAGE().
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  mm/migrate.c | 37 ++++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 6bbb455f8b593..32e77898f7d6c 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -133,12 +133,30 @@ bool isolate_movable_ops_page(struct page *page, isolate_mode_t mode)
>  	return false;
>  }
>
> -static void putback_movable_folio(struct folio *folio)
> +/**
> + * putback_movable_ops_page - putback an isolated movable_ops page
> + * @page: The isolated page.
> + *
> + * Putback an isolated movable_ops page.
> + *
> + * After the page was putback, it might get freed instantly.
> + */
> +static void putback_movable_ops_page(struct page *page)
>  {
> -	const struct movable_operations *mops = folio_movable_ops(folio);
> -
> -	mops->putback_page(&folio->page);
> -	folio_clear_isolated(folio);
> +	/*
> +	 * TODO: these pages will not be folios in the future. All
> +	 * folio dependencies will have to be removed.
> +	 */
> +	struct folio *folio = page_folio(page);
> +
> +	VM_WARN_ON_ONCE_PAGE(!PageIsolated(page), page);
> +	folio_lock(folio);
> +	/* If the page was released by it's owner, there is nothing to do. */
> +	if (PageMovable(page))
> +		page_movable_ops(page)->putback_page(page);
> +	ClearPageIsolated(page);
> +	folio_unlock(folio);
> +	folio_put(folio);

Why not use page version of lock, unlock, and put? Especially you are
thinking about not using folio for these pages. Just a question,
I am OK with current patch.

>  }
>
>  /*
> @@ -166,14 +184,7 @@ void putback_movable_pages(struct list_head *l)
>  		 * have PAGE_MAPPING_MOVABLE.
>  		 */
>  		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
> -				folio_clear_isolated(folio);
> -			folio_unlock(folio);
> -			folio_put(folio);
> +			putback_movable_ops_page(&folio->page);
>  		} else {
>  			node_stat_mod_folio(folio, NR_ISOLATED_ANON +
>  					folio_is_file_lru(folio), -folio_nr_pages(folio));
> -- 
> 2.49.0

Acked-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

On Wed, Jun 18, 2025 at 03:10:10PM -0400, Zi Yan wrote:
> On 18 Jun 2025, at 13:39, David Hildenbrand wrote:
> > +	/*
> > +	 * TODO: these pages will not be folios in the future. All
> > +	 * folio dependencies will have to be removed.
> > +	 */
> > +	struct folio *folio = page_folio(page);
> > +
> > +	VM_WARN_ON_ONCE_PAGE(!PageIsolated(page), page);
> > +	folio_lock(folio);
> > +	/* If the page was released by it's owner, there is nothing to do. */
> > +	if (PageMovable(page))
> > +		page_movable_ops(page)->putback_page(page);
> > +	ClearPageIsolated(page);
> > +	folio_unlock(folio);
> > +	folio_put(folio);
> 
> Why not use page version of lock, unlock, and put? Especially you are
> thinking about not using folio for these pages. Just a question,
> I am OK with current patch.

That would reintroduce unnecessary calls to compound_head().

On 18 Jun 2025, at 15:18, Matthew Wilcox wrote:

> On Wed, Jun 18, 2025 at 03:10:10PM -0400, Zi Yan wrote:
>> On 18 Jun 2025, at 13:39, David Hildenbrand wrote:
>>> +	/*
>>> +	 * TODO: these pages will not be folios in the future. All
>>> +	 * folio dependencies will have to be removed.
>>> +	 */
>>> +	struct folio *folio = page_folio(page);
>>> +
>>> +	VM_WARN_ON_ONCE_PAGE(!PageIsolated(page), page);
>>> +	folio_lock(folio);
>>> +	/* If the page was released by it's owner, there is nothing to do. */
>>> +	if (PageMovable(page))
>>> +		page_movable_ops(page)->putback_page(page);
>>> +	ClearPageIsolated(page);
>>> +	folio_unlock(folio);
>>> +	folio_put(folio);
>>
>> Why not use page version of lock, unlock, and put? Especially you are
>> thinking about not using folio for these pages. Just a question,
>> I am OK with current patch.
>
> That would reintroduce unnecessary calls to compound_head().

Got it. But here page is not folio, so it cannot be a compound page.
Then, we will need page versions without compound_head() for
non compound pages. Could that happen in the future when only folio
can be compound and page is only order-0?

Best Regards,
Yan, Zi

On Wed, Jun 18, 2025 at 03:25:46PM -0400, Zi Yan wrote:
> On 18 Jun 2025, at 15:18, Matthew Wilcox wrote:
> >> Why not use page version of lock, unlock, and put? Especially you are
> >> thinking about not using folio for these pages. Just a question,
> >> I am OK with current patch.
> >
> > That would reintroduce unnecessary calls to compound_head().
> 
> Got it. But here page is not folio, so it cannot be a compound page.
> Then, we will need page versions without compound_head() for
> non compound pages. Could that happen in the future when only folio
> can be compound and page is only order-0?

I think the assumption that we'll only see compound pages as part of
folios is untrue.  For example, slabs will still allocate multiple
pages (though slabs aren't migratable at this point).  The sketch at
https://kernelnewbies.org/MatthewWilcox/Memdescs supports "misc pages"
with an order stored in bits 12-17 of the memdesc.  I don't know
how useful that will turn out to be; maybe we'll never implement that.

On 18.06.25 22:04, Matthew Wilcox wrote:
> On Wed, Jun 18, 2025 at 03:25:46PM -0400, Zi Yan wrote:
>> On 18 Jun 2025, at 15:18, Matthew Wilcox wrote:
>>>> Why not use page version of lock, unlock, and put? Especially you are
>>>> thinking about not using folio for these pages. Just a question,
>>>> I am OK with current patch.
>>>
>>> That would reintroduce unnecessary calls to compound_head().

Right. And I want to make it clear that everything that uses "folio" 
here must be reworked: not necessarily switching to the "page" variant, 
but actually by implementing it entirely differently.

(e.g., store them in an array instead of a list, get rid of the lock 
bit, try getting rid of the refcount as well)

>>
>> Got it. But here page is not folio, so it cannot be a compound page.
>> Then, we will need page versions without compound_head() for
>> non compound pages. Could that happen in the future when only folio
>> can be compound and page is only order-0?
> 
> I think the assumption that we'll only see compound pages as part of
> folios is untrue.  For example, slabs will still allocate multiple
> pages (though slabs aren't migratable at this point).  The sketch at
> https://kernelnewbies.org/MatthewWilcox/Memdescs supports "misc pages"
> with an order stored in bits 12-17 of the memdesc.  I don't know
> how useful that will turn out to be; maybe we'll never implement that.

Once we have to support that for movable_ops, it will be an interesting 
question how to handle that. Most certainly, these things will not be 
folios. :)

-- 
Cheers,

David / dhildenb