[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <acc04c23-d7d6-4ecd-bfeb-6cccfd80e6d9@redhat.com>
Date: Mon, 2 Jun 2025 22:55:52 +0200
From: David Hildenbrand <david@...hat.com>
To: Vlastimil Babka <vbabka@...e.cz>, linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org, virtualization@...ts.linux.dev,
"Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
<eperezma@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>,
Oscar Salvador <osalvador@...e.de>, Suren Baghdasaryan <surenb@...gle.com>,
Michal Hocko <mhocko@...e.com>, Brendan Jackman <jackmanb@...gle.com>,
Johannes Weiner <hannes@...xchg.org>, Zi Yan <ziy@...dia.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>
Subject: Re: [PATCH v2 1/1] mm/memory_hotplug: PG_offline_skippable for
offlining memory blocks with PageOffline pages
On 30.05.25 18:54, Vlastimil Babka wrote:
> On 5/20/25 18:42, David Hildenbrand wrote:
>> A long-term goal is supporting frozen PageOffline pages, and later
>> PageOffline pages that don't have a refcount at all. Some more work for
>
> Looking forward to that :)
:) It's definitely ... challenging.
>
>> that is needed -- in particular around non-folio page migration and
>> memory ballooning drivers -- but let's start by handling PageOffline pages
>> that can be skipped during memory offlining differently.
>>
>> Note that PageOffline is used to mark pages that are logically offline
>> in an otherwise online memory block (e.g., 128 MiB). If a memory
>> block is offline, the memmap is considered compeltely uninitialized
>> and stale (see pfn_to_online_page()).
>>
>> Let's introduce a PageOffline specific page flag (PG_offline_skippable)
>> that for now reuses PG_owner_2. In the memdesc future, it will be one of
>> a small number of per-memdesc flags stored alongside the type.
>>
>> By setting PG_offline_skippable, a driver indicates that it can
>> restore the PageOffline state of these specific pages when re-onlining a
>> memory block: it knows that these pages are supposed to be PageOffline()
>> without the information in the vmemmap, so it can filter them out and
>> not expose them to the buddy -> they stay PageOffline().
>>
>> While PG_offline_offlineable might be clearer, it is also super
>> confusing. Alternatives (PG_offline_sticky?) also don't quite feel right.
>> So let's use "skippable" for now.
>>
>> The flag is not supposed to be used for movable PageOffline pages as
>> used for balloon compaction; movable PageOffline() pages can simply be
>> migrated during the memory offlining stage, turning the migration
>> destination page PageOffline() and turning the migration source page
>> into a free buddy page.
>>
>> Let's convert the single user from our MEM_GOING_OFFLINE approach
>> to the new PG_offline_skippable approach: virtio-mem. Fortunately,
>> this simplifies the code quite a lot. The only corner case we have to
>> take care of is when force-unloading the virtio-mem driver: we have to
>> prevent partially-plugged memory blocks from getting offlined by
>> clearing PG_offline_skippable again.
>>
>> What if someone decides to grab a reference on these pages although they
>> really shouldn't? After all, we'll now keep the refcount at 1 (until we
>> can properly stop using the refcount completely).
>>
>> Well, less worse things will happen than would currently: currently,
>> if someone would grab a reference to these pages, in MEM_GOING_OFFLINE
>> we would run into the
>> if (WARN_ON(!page_ref_dec_and_test(page)))
>> dump_page(page, "fake-offline page referenced");
>>
>> And once that unexpected reference would get dropped, we would end up
>> freeing that page to the buddy: ouch.
>>
>> Now, we'll allow for offlining that memory, and when that unexpected
>> reference would get dropped, we would not end up freeing that page to
>> the buddy. Once we have frozen PageOffline() pages, it will all get a
>> lot cleaner.
>
> Hmm, a question on that later in the code (assuming I identified the right
> place).
>
>> Note that we didn't see the existing WARN_ON so far, because nobody
>> should ever be referencing such pages.
>
> It's mostly a speculative refcount increase from a pfn walker, such as
> compaction scanner, that can happen due to its inherent raciness.
PFN walkers are in general careful to only try grabbing a reference if
the target page is a possible candidate.
E.g., compaction checks for LRU or movable non-folio pages before
calling folio_get_nontail_page().
KSM uses MEM_GOING_OFFLINE to pause scanning (and thereby
folio_try_get()) while memory is getting offlined, so it won't try
accessing any page in the trees that might now be stale.
>
>> An alternative might be to have another callback chain from memory hotplug
>> code, where a driver that owns that page could agree to skip the
>> PageOffline() page. However, we would have to repeatedly issue these
>> callbacks for individual PageOffline() pages, which does not sound
>> compelling. As we have spare bits, let's use this simpler approach for
>> now.
>>
>> Acked-by: Zi Yan <ziy@...dia.com>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
>
> Acked-by: Vlastimil Babka <vbabka@...e.cz> # page allocator
>
> I'll leave hotplug to the experts :)
>
> <snip>
>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index f6482223e28a2..7e4c41e46a911 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7023,12 +7023,12 @@ unsigned long __offline_isolated_pages(unsigned long start_pfn,
>> continue;
>> }
>> /*
>> - * At this point all remaining PageOffline() pages have a
>> - * reference count of 0 and can simply be skipped.
>> + * At this point all remaining PageOffline() pages must be
>> + * "skippable" and have exactly one reference.
>> */
>> if (PageOffline(page)) {
>> - BUG_ON(page_count(page));
>> - BUG_ON(PageBuddy(page));
>> + WARN_ON_ONCE(!PageOfflineSkippable(page));
>> + WARN_ON_ONCE(page_count(page) != 1);
>
> So is this the part where an unexpected speculative refcount might be
> detected? Should be harmless then as it will then decrease the refcount from
> e.g. 2 to 1 and nothing will happen right.
Yes, unless it would be taken for a longer time, which is not what
usually happens: speculative references are immediately dropped if the
target page is of no use.
> That's assuming that once we pass __offline_isolated_pages(), the following
> actions wont modify the refcount or the struct page won't be zeroed, or
> removed completely (vmemmap). Probably something already prevents that...
Right, we'd have similar problems also if the refcount would be 0 and
someone would do a folio_try_get(), while we are tearing down or
poisoning the vmemmap.
The idea here is that these folios were PageOffline() already for a
while (before even starting memory offlining), so any speculative
references from pre-PageOffline times should long be gone.
It's still racy, but not more racy than our existing
pfn_to_online_page() race, or the race we already had in the old code.
Thanks!
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists