On Mon, Aug 11, 2025 at 10:47 PM David Hildenbrand <da...@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 ;)
> 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 <lance.y...@linux.dev>
Thanks,
Lance
