Re: [PATCH v1 1/2] mm/migrate: remove MIGRATEPAGE_UNMAP

[David Hildenbrand <david@redhat.com>]

On 13.08.25 07:05, Lance Yang wrote:
> On Mon, Aug 11, 2025 at 10:47 PM David Hildenbrand <david@redhat.com> wrote:
>>
> [...]
>> +++ b/mm/migrate.c
>> @@ -1176,16 +1176,6 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>          bool locked = false;
>>          bool dst_locked = false;
>>
>> -       if (folio_ref_count(src) == 1) {
>> -               /* Folio was freed from under us. So we are done. */
>> -               folio_clear_active(src);
>> -               folio_clear_unevictable(src);
>> -               /* free_pages_prepare() will clear PG_isolated. */
>> -               list_del(&src->lru);
>> -               migrate_folio_done(src, reason);
>> -               return MIGRATEPAGE_SUCCESS;
>> -       }
>> -
>>          dst = get_new_folio(src, private);
>>          if (!dst)
>>                  return -ENOMEM;
>> @@ -1275,7 +1265,7 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>
>>          if (unlikely(page_has_movable_ops(&src->page))) {
>>                  __migrate_folio_record(dst, old_page_state, anon_vma);
>> -               return MIGRATEPAGE_UNMAP;
>> +               return 0;
>>          }
>>
>>          /*
>> @@ -1305,7 +1295,7 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>
>>          if (!folio_mapped(src)) {
>>                  __migrate_folio_record(dst, old_page_state, anon_vma);
>> -               return MIGRATEPAGE_UNMAP;
>> +               return 0;
>>          }
>>
>>   out:
>> @@ -1848,14 +1838,28 @@ static int migrate_pages_batch(struct list_head *from,
>>                                  continue;
>>                          }
>>
>> +                       /*
>> +                        * If we are holding the last folio reference, the folio
>> +                        * was freed from under us, so just drop our reference.
>> +                        */
>> +                       if (likely(!page_has_movable_ops(&folio->page)) &&
>> +                           folio_ref_count(folio) == 1) {
>> +                               folio_clear_active(folio);
>> +                               folio_clear_unevictable(folio);
>> +                               list_del(&folio->lru);
>> +                               migrate_folio_done(folio, reason);
>> +                               stats->nr_succeeded += nr_pages;
>> +                               stats->nr_thp_succeeded += is_thp;
>> +                               continue;
>> +                       }
>> +
> 
> It seems the reason parameter is no longer used within migrate_folio_unmap()
> after this patch.
> 
> Perhaps it could be removed from the function's signature ;)

Thanks, well spotted, @Andrew can you squash the following?


 From 40938bb0de20e03250c813d5abc7286aea69d835 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Mon, 18 Aug 2025 13:26:05 +0200
Subject: [PATCH] fixup: mm/migrate: remove MIGRATEPAGE_UNMAP

No need to pass "reason" to migrate_folio_unmap().

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

diff --git a/mm/migrate.c b/mm/migrate.c
index 2db4974178e6a..aabc736eec022 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1166,7 +1166,7 @@ static void migrate_folio_done(struct folio *src,
  static int migrate_folio_unmap(new_folio_t get_new_folio,
  		free_folio_t put_new_folio, unsigned long private,
  		struct folio *src, struct folio **dstp, enum migrate_mode mode,
-		enum migrate_reason reason, struct list_head *ret)
+		struct list_head *ret)
  {
  	struct folio *dst;
  	int rc = -EAGAIN;
@@ -1852,8 +1852,7 @@ static int migrate_pages_batch(struct list_head *from,
  			}
  
  			rc = migrate_folio_unmap(get_new_folio, put_new_folio,
-					private, folio, &dst, mode, reason,
-					ret_folios);
+					private, folio, &dst, mode, ret_folios);
  			/*
  			 * The rules are:
  			 *	0: folio will be put on unmap_folios list,
-- 
2.50.1


-- 
Cheers

David / dhildenb