Re: [PATCH] mm/migrate: Putback split folios when numa hint migration fails

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2024/7/9 05:55, Peter Xu wrote:
> This issue is not from any report yet, but by code observation only.
> 
> This is yet another fix besides Hugh's patch [1] but on relevant code path,
> where eager split of folio can happen if the folio is already on deferred
> list during a folio migration.
> 
> Here the issue is NUMA path (migrate_misplaced_folio()) may start to
> encounter such folio split now even with MR_NUMA_MISPLACED hint applied.
> Then when migrate_pages() didn't migrate all the folios, it's possible the
> split small folios be put onto the list instead of the original folio.
> Then putting back only the head page won't be enough.
> 
> Fix it by putting back all the folios on the list.
> 
> [1] https://lore.kernel.org/all/46c948b4-4dd8-6e03-4c7b-ce4e81cfa536@google.com/
> 
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Yang Shi <shy828301@gmail.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: David Hildenbrand <david@redhat.com>
> Fixes: 7262f208ca68 ("mm/migrate: split source folio if it is on deferred split list")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---

Good catch. With fixing the building issue pointed by Andrew, please 
feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> 
> Don't need to copy stable if this can still hit 6.10..  Only smoke tested.
> ---
>   mm/migrate.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index e10d2445fbd8..20da2595527a 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2615,14 +2615,8 @@ int migrate_misplaced_folio(struct folio *folio, struct vm_area_struct *vma,
>   	nr_remaining = migrate_pages(&migratepages, alloc_misplaced_dst_folio,
>   				     NULL, node, MIGRATE_ASYNC,
>   				     MR_NUMA_MISPLACED, &nr_succeeded);
> -	if (nr_remaining) {
> -		if (!list_empty(&migratepages)) {
> -			list_del(&folio->lru);
> -			node_stat_mod_folio(folio, NR_ISOLATED_ANON +
> -					folio_is_file_lru(folio), -nr_pages);
> -			folio_putback_lru(folio);
> -		}
> -	}
> +	if (nr_remaining && !list_empty(&migratepages))

Nit: you can drop the '!list_empty(&migratepages)' validation, since 
putback_movable_pages() can handle this unusual case.

> +		putback_movable_pages(&migratepages);
>   	if (nr_succeeded) {
>   		count_vm_numa_events(NUMA_PAGE_MIGRATE, nr_succeeded);
>   		if (!node_is_toptier(folio_nid(folio)) && node_is_toptier(node))