Re: [RFC PATCH] mm/memory_hotplug: use public nodes for migration targets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Restricting the migration target nodemask to N_MEMORY_PUBLIC prevents HugeTLB folios from migrating between private nodes.
- [Low] N_MEMORY_PUBLIC is undeclared, causing a build failure.

Pre-existing issues:
- [High] Infinite loop during memory offlining when encountering a hwpoisoned large folio.
- [High] HWPoisoned folios are permanently leaked from the LRU and their refcounts are permanently elevated during memory hotplug offlining.
--

commit a396d55b92bfae38e85ebc68194e34af029a9db7
Author: Zimo.Lu <louis2665573906@gmail.com>

mm/memory_hotplug: use public nodes for migration targets

This commit modifies do_migrate_range() to use N_MEMORY_PUBLIC for the
migration target nodemask instead of N_MEMORY. This aims to prevent
private nodes from blocking fallback to public nodes when allocating
migration targets, which also impacts HugeTLB migration targets during
memory offlining.

> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index d7a59167bec43..683925183df6b 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1928,7 +1928,7 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)

[Severity: High]
This is a pre-existing issue, but does skipping poisoned large folios earlier
in do_migrate_range() cause a livelock?

During memory offlining, skipping the folio without splitting it means
isolation fails and the folio is never handled:

		if (folio_contain_hwpoisoned_page(folio)) {
			/*
			 * unmap_poisoned_folio() cannot handle large folios
			 * in all cases yet.
			 */
			if (folio_test_large(folio) && !folio_test_hugetlb(folio))
				goto put_folio;

This causes test_pages_isolated() to fail, which forces the outer do-while
loop in offline_pages() to retry indefinitely, holding global hotplug locks
and causing a system-wide denial of service.

Can this be fixed by splitting the large folio before evaluating
unmap_poisoned_folio()?

[Severity: High]
This is also a pre-existing issue, but does this leak the isolation reference
and permanently elevate the folio refcount for hwpoisoned folios?

When folio_isolate_lru() succeeds, it removes the folio from the LRU and
takes an isolation reference:

			if (folio_test_lru(folio) && !folio_isolate_lru(folio))
				goto put_folio;
			if (folio_mapped(folio)) {
				folio_lock(folio);
				unmap_poisoned_folio(folio, pfn, false);
				folio_unlock(folio);
			}

			goto put_folio;
		}

By jumping to put_folio, the code drops only the initial reference from
folio_try_get():

>  		folio_put(folio);

This leaves the isolation reference permanently leaked and the folio
permanently off the LRU list, preventing the memory block from being offlined.

>  	}
>  	if (!list_empty(&source)) {
> -		nodemask_t nmask = node_states[N_MEMORY];
> +		nodemask_t nmask = node_states[N_MEMORY_PUBLIC];

[Severity: Low]
Does this cause a build failure? N_MEMORY_PUBLIC is undeclared in the kernel
source, causing compilation to fail.

[Severity: Medium]
Does restricting this nodemask to N_MEMORY_PUBLIC break HugeTLB migration
between private nodes?

Since alloc_hugetlb_folio_nodemask() strictly iterates the provided nmask,
initializing it to N_MEMORY_PUBLIC strips out all private nodes as potential
targets.

If a private node with HugeTLB pages is offlined, and public nodes lack
sufficient HugeTLB pages, offlining will fail even if other private nodes
could accept them.

>  		struct migration_target_control mtc = {
>  			.nmask = &nmask,
>  			.gfp_mask = GFP_KERNEL | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914074323.3567923-1-louis2665573906@gmail.com?part=1