Re: [PATCH 6.1] mm/memory_hotplug: fix hwpoisoned large folio handling in do_migrate_range()

[Александра Дюпина]

05/06/26 21:09, Zi Yan wrote:
> On 5 Jun 2026, at 13:26, Alexandra Diupina wrote:
>
> > From: Jinjiang Tu <tujinjiang@xxxxxxxxxx>
> >
> > commit 397f6d14f9c370e4910e6885294c340f39dedbf5 upstream.
> >
> > In do_migrate_range(), the hwpoisoned folio may be large folio, which
> > can't be handled by unmap_poisoned_folio().
> >
> > I can reproduce this issue in qemu after adding delay in memory_failure()
> >
> > BUG: kernel NULL pointer dereference, address: 0000000000000000
> > Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> > RIP: 0010:try_to_unmap_one+0x16a/0xfc0
> >    <TASK>
> >    rmap_walk_anon+0xda/0x1f0
> >    try_to_unmap+0x78/0x80
> >    ? __pfx_try_to_unmap_one+0x10/0x10
> >    ? __pfx_folio_not_mapped+0x10/0x10
> >    ? __pfx_folio_lock_anon_vma_read+0x10/0x10
> >    unmap_poisoned_folio+0x60/0x140
> >    do_migrate_range+0x4d1/0x600
> >    ? slab_memory_callback+0x6a/0x190
> >    ? notifier_call_chain+0x56/0xb0
> >    offline_pages+0x3e6/0x460
> >    memory_subsys_offline+0x130/0x1f0
> >    device_offline+0xba/0x110
> >    acpi_bus_offline+0xb7/0x130
> >    acpi_scan_hot_remove+0x77/0x290
> >    acpi_device_hotplug+0x1e0/0x240
> >    acpi_hotplug_work_fn+0x1a/0x30
> >    process_one_work+0x186/0x340
> >
> > Besides, do_migrate_range() may be called between memory_failure set
> > hwpoison flag and isolate the folio from lru, so remove WARN_ON(). In other
> > places, unmap_poisoned_folio() is called when the folio is isolated, obey
> > it in do_migrate_range() too.
> >
> > [david@xxxxxxxxxx: don't abort offlining, fixed typo, add comment]
> > Link: https://lkml.kernel.org/r/3c214dff-9649-4015-840f-10de0e03ebe4@xxxxxxxxxx
> > Fixes: b15c87263a69 ("hwpoison, memory_hotplug: allow hwpoisoned pages to be offlined")
> > Signed-off-by: Jinjiang Tu <tujinjiang@xxxxxxxxxx>
> > Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> > Acked-by: Zi Yan <ziy@xxxxxxxxxx>
> > Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> > Cc: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> > Cc: Luis Chamberalin <mcgrof@xxxxxxxxxx>
> > Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > Cc: Michal Hocko <mhocko@xxxxxxxxxx>
> > Cc: Oscar Salvador <osalvador@xxxxxxx>
> > Cc: Pankaj Raghav <kernel@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > [ Alexandra: replace continue with put_folio label ]
> > Signed-off-by: Alexandra Diupina <adiupina@xxxxxxxxxxxxx>
> > ---
> >   mm/memory_hotplug.c | 13 ++++++++++---
> >   1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index c8cc2f63c3ea..013db41f6ce2 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -1654,15 +1654,21 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> >   		 * the unmap as the catch all safety net).
> >   		 */
> >   		if (PageHWPoison(page)) {
> > -			if (WARN_ON(folio_test_lru(folio)))
> > -				folio_isolate_lru(folio);
> > +			/*
> > +			 * unmap_poisoned_folio() cannot handle large folios
> > +			 * in all cases yet.
> > +			 */
> > +			if (folio_test_large(folio) && !folio_test_hugetlb(folio))
> > +				goto put_folio;
> > +			if (folio_test_lru(folio) && !folio_isolate_lru(folio))
> > +				goto put_folio;
> >   			if (folio_mapped(folio)) {
> >   				folio_lock(folio);
> >   				try_to_unmap(folio, TTU_IGNORE_MLOCK);
> >   				folio_unlock(folio);
> >   			}
> >
> > -			continue;
> > +			goto put_folio;
> >   		}
> >
> >   		if (!get_page_unless_zero(page))
> > @@ -1687,6 +1693,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> >   				dump_page(page, "isolation failed");
> >   			}
> >   		}
> > +put_folio:
> >   		put_page(page);
> >   	}
> >   	if (!list_empty(&source)) {
> > --
> > 2.30.2
> I am not sure this is right.
>
> The original patch uses put_folio, because folio_try_get() is called
> before this if block. But for 6.1, get_page_unless_zero() is called
> after this if block, so the page in this if block has no elevated
> refcount compared to the original patch context. You might want to
> replace “goto put_folio” with “continue” in the original patch instead.
>
> Best Regards,
> Yan, Zi
Thanks for the comments,
I'll send the second version of the patch