On 7/22/26 14:08, David Hildenbrand (Arm) wrote:
> On 7/22/26 09:48, Aboorva Devarajan wrote:
>> Memory offlining can loop forever when a memory block holds a page that
>> can never be migrated or freed. This series adds an opt-in retry limit
>> that lets offline_pages() bail out with -EBUSY instead of retrying
>> forever. The default (0) preserves today's behaviour exactly.
>>
>> Patch 1 is the fix; patch 2 is an in-tree selftest that reproduces the
>> hang from an unsignalable kworker and verifies the limit recovers it.
>>
>> 1. The problem
>> ==============
>>
>> offline_pages() repeats two nested steps until the whole range is
>> isolated:
>>
>> 1. Inner loop: find and migrate movable pages out of the range
>> (scan_movable_pages() + do_migrate_range()).
>> 2. Outer loop: re-check isolation (test_pages_isolated()); if not
>> fully isolated, go back to step 1.
>>
>> Neither loop has a termination condition. When a page can never be
>> migrated, scan_movable_pages() keeps returning the same pfn,
>> do_migrate_range() keeps failing on it, and control never even reaches
>> the outer isolation re-check - the inner loop spins forever. The admin
>> guide acknowledges this:
>>
>> "Further, memory offlining might retry for a long time (or even
>> forever), until aborted by the user."
>>
>> The single escape in the loop is signal_pending(current):
>>
>> offline_pages(pfn, end_pfn):
>> do { /* outer: isolation */
>> do { /* inner: migration */
>> if (signal_pending(current)) /* <-- ONLY escape */
>> goto failed_removal;
>> pfn = scan_movable_pages(pfn, end_pfn);
>> do_migrate_range(pfn, end_pfn); /* may never succeed */
>> } while (pfn < end_pfn);
>> } while (test_pages_isolated(...));
>>
>>
>> 2. Why the kernel itself should be able to bail
>> ===============================================
>>
>> We keep hitting this during memory hot-remove operations: a single
>> stuck page can block the whole operation. This was also reported in
>> [4] earlier; one example, where offlining kept failing to migrate a
>> busy block-device page-cache page (aops:def_blk_aops) in a normal zone:
>>
>> [10880.889199] page dumped because: migration failure
>> [10880.889232] migrating pfn 2a87b failed ret:1
>> [10880.889235] page: refcount:3 mapcount:0 mapping:00000000718ec5a6
>> index:0x857 pfn:0x2a87b
>> [10880.889241] aops:def_blk_aops ino:800003 dentry name(?):""
>> [10880.889245] flags:
>> 0x33ffffe00004104(referenced|active|private|node=3|zone=0|lastcpupid=0x1fffff)
>> ...
>> [10880.889291] migrating pfn 2a87b failed ret:1
>
> Hi,
>
> the text reads AI generated. If you did use AI, please disclose it properly.
>
> Quick feedback:
>
> 1) Using passes is not really what we want in many cases (e.g., ZONE_MOVABLE
> where we might need a couple of seconds/minutes to complete offlining with a
> lot
> of concurrent activity). An actual timeout is also problematic (see below).
>
> 2) Having a toggle for all offline_pages() users is questionable. In
> particular,
> user-triggered offlining or offlinig triggered on ZONE_MOVABLE etc should not
> obey such timeouts.
>
> I proposed a more restricted approach only for offline_and_remove_memory()
> previously [1]
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> Michal back then commented "I really hate having timeouts back. They just
> proven
> to be hard to get right and it is essentially a policy implemented in the
> kernel. They simply do not belong to the kernel space IMHO."
>
> And I agree.
>
> I assume you run into such issues with DIMMs in VMs? virtio-mem handles that
> much nicer nowadays, by essentially doing the hard part that should fail
> easily
> through alloc_contig_range().
>
> offline_pages() is just designed to retry forever.
>
One thing we can definitely do is to just fail faster if we find an unmovable
page in !ZONE_MOVABLE.
Usually that happens when we race offlining (has_unmovable_pages()) with page
allocation, and failing on unmovable pages is actually perfectly fine.
But for ZONE_MOVABLE we should keep retrying, because some pages might only look
temporarily unmovable.
So that would be the low hanging fruit: on !ZONE_MOVABLE, fail faster.
--
Cheers,
David