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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zo1PsXQIA9Jr5k0x@x1n>
Date: Tue, 9 Jul 2024 10:56:49 -0400
From: Peter Xu <peterx@...hat.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>, Zi Yan <ziy@...dia.com>,
	Yang Shi <shy828301@...il.com>, Hugh Dickins <hughd@...gle.com>,
	Huang Ying <ying.huang@...el.com>,
	David Hildenbrand <david@...hat.com>
Subject: Re: [PATCH] mm/migrate: Putback split folios when numa hint
 migration fails

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@google.com/
> > 
> > Cc: Zi Yan <ziy@...dia.com>
> > Cc: Yang Shi <shy828301@...il.com>
> > Cc: Hugh Dickins <hughd@...gle.com>
> > Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
> > Cc: Huang Ying <ying.huang@...el.com>
> > Cc: David Hildenbrand <david@...hat.com>
> > Fixes: 7262f208ca68 ("mm/migrate: split source folio if it is on deferred split list")
> > Signed-off-by: Peter Xu <peterx@...hat.com>
> > ---
> 
> Good catch. With fixing the building issue pointed by Andrew, please feel
> free to add:
> Reviewed-by: Baolin Wang <baolin.wang@...ux.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.

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