Re: [PATCH v2 1/1] mm/memory_hotplug: PG_offline_skippable for offlining memory blocks with PageOffline pages

[David Hildenbrand <david@redhat.com>]

>>
>> Acked-by: Zi Yan <ziy@nvidia.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> Hi David, sorry for jumping in late

Hi,

not late at all :)

> 
>> @@ -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 is a more generic concept also e.g., used by memory 
balloning. virtio-mem uses that concept, yes.

> - 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

PageOffline pages currently always have a refcount > 0 (except 
temporarily in this special case we are removing here). In the future, 
it won't have a refcount at all.

> - 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)

No, the code is confusing.

All pages are PageOffline. Only the ones that have not been onlined are 
*in addition* PageDirty.

The relevant bit is documented in page-flags.h:

"When a memory block gets onlined, all pages are initialized with a 
refcount of 1 and PageOffline(). generic_online_page() will take care of 
clearing PageOffline()."

> - If we want to release logically-offline pages belonging to an onlined memory-block,
>    we ClearDirty them and be done

PageOffline gets cleared in both cases: See the comment in 
virtio_mem_clear_fake_offline()

"/* generic_online_page() will clear PageOffline(). */"

I'll note that I am planning on removing that PageDirty() handling 
completely, and also letting handling PageOffline() clearing be always 
performed by memory freeing core (the latter is easier to achieve).

> - If we want to release logically-offlined pages belonging we allocated
>    via alloc_contig_range, we clear PageOffline and be done

Yes, in the future that clearing will be done by the core (so the plan).

 > - PageOfflineSkipabble are unmovable PageOffline pages, which cannot 
be migrated?

Yes, but they can simply be skipped -- consider them memory holes we 
(the driver) can rediscover when re-onlining the memory. See below.

> - So for a PageOffline to be able to be migrated away must be Movable or
>    marked PageOfflineSkipabble, making do_migrate_range ignore it

Yes.

> - PageOfflineSkipabble will be marked PageOffline upon re-onlining? Will
>    still be marked as PageOfflineSkipabble?

When re-onlining, the core will set them all PageOffline, and virtio-mem 
will intercept page onlining using the page_online_cb.

virtio-mem will then online the actually plugged parts (-> 
generic_online_page(), which clears PageOffline and exposes them to the 
buddy) and set the unplugged/hole parts as PageOfflineSkipabble again.

That logic resides in virtio_mem_online_page_cb().

> 
>> +
>> +		/*
>> +		 * 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.

Definitely. Thanks!

-- 
Cheers,

David / dhildenb