Message-ID: <aC3V1Sr7rGqqgLzW@localhost.localdomain>
Date: Wed, 21 May 2025 15:32:05 +0200
From: Oscar Salvador <osalvador@...e.de>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, 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>,
	Vlastimil Babka <vbabka@...e.cz>,
	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 Tue, May 20, 2025 at 06:42:11PM +0200, 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
> 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.
> 
> Note that we didn't see the existing WARN_ON so far, because nobody
> should ever be referencing such pages.
> 
> 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>

Hi David, sorry for jumping in late

> @@ -1157,6 +1083,7 @@ static void virtio_mem_set_fake_offline(unsigned long pfn,
>  			SetPageDirty(page);
>  		else
>  			__SetPageOffline(page);
> +		__SetPageOfflineSkippable(page);
>  		VM_WARN_ON_ONCE(!PageOffline(page));

I think I am having some issues understanding this, let me see if I get
it.

- virtio-mem defines PageOffline pages, which are logically-offlined
  pages within an onlined memory-block
- PageOffline pages have a refcount of '0' once they are properly
  initialized, meaning that refcount > 0 implies somebody is holding
  a refcount and that should not really happen
- logically-offline pages belonging to onlined memory-blocks are marked PageDirty,
  while logically-offlined pages we allocated via alloc_contig_range are marked
  PageOffline (I am getting a bit lost between fake-online, fake-offline, my fault)
- If we want to release logically-offline pages belonging to an onlined memory-block,
  we ClearDirty them and be done
- If we want to release logically-offlined pages belonging we allocated
  via alloc_contig_range, we clear PageOffline and be done
- PageOfflineSkipabble are unmovable PageOffline pages, which cannot be
  migrated? 
- So for a PageOffline to be able to be migrated away must be Movable or
  marked PageOfflineSkipabble, making do_migrate_range ignore it
- PageOfflineSkipabble will be marked PageOffline upon re-onlining? Will
  still be marked as PageOfflineSkipabble?

> +
> +		/*
> +		 * Only PageOffline() pages that are marked "skippable" cannot
> +		 * be migrated but can be skipped when offlining. See

It is probably me, and nevermind the comment but I somehow find
"PageOfflineSkipabble are not migrated but skipped when offlining" a bit
easier.

 

-- 
Oscar Salvador
SUSE Labs

Powered by blists - more mailing lists