On Tue, Jul 09, 2024 at 09:48:54AM +0800, Baolin Wang wrote: > > > 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@xxxxxxxxxx/ > > > > Cc: Zi Yan <ziy@xxxxxxxxxx> > > Cc: Yang Shi <shy828301@xxxxxxxxx> > > Cc: Hugh Dickins <hughd@xxxxxxxxxx> > > Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx> > > Cc: Huang Ying <ying.huang@xxxxxxxxx> > > Cc: David Hildenbrand <david@xxxxxxxxxx> > > Fixes: 7262f208ca68 ("mm/migrate: split source folio if it is on deferred split list") > > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx> > > --- > > Good catch. With fixing the building issue pointed by Andrew, please feel > free to add: > Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx> > > > > > 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. Sure, considering that it should normally be !empty when the first check passed. Though to make this simple for now, I assume we can keep what has been queued in Andrew's tree. It isn't so bad either to double check the list to avoid a function call if possible, I think. Thanks for the comment, -- Peter Xu
