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

On Mon, Aug 11, 2025 at 10:47 PM David Hildenbrand <[email protected]> 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 ;)

>                         rc = migrate_folio_unmap(get_new_folio, put_new_folio,
>                                         private, folio, &dst, mode, reason,
>                                         ret_folios);

Anyway, just a small thought. Feel free to add:
Reviewed-by: Lance Yang <[email protected]>

Thanks,
Lance