Re: [PATCH 2/4] mm: not __SetPageReserved on initializing hot-plugged memory

[David Hildenbrand <david@redhat.com>]

On 29.06.24 10:32, Wei Yang wrote:
> On Sat, Jun 29, 2024 at 08:19:49AM +0200, David Hildenbrand wrote:
>> On 29.06.24 03:33, Wei Yang wrote:
>>> Initialize all pages reserved is an ancient behavior.
>>>
>>> Since commit 92923ca3aace ("mm: meminit: only set page reserved in the
>>> memblock region"), SetPageReserved is removed from
>>> __init_single_page(). Only those reserved pages are marked PG_reserved.
>>>
>>> But we still set PG_reserved on offline and check it on online.
>>>
>>> Following two commits removed both of them:
>>>
>>> * Commit 0ee5f4f31d36 ("mm/page_alloc.c: don't set pages PageReserved()
>>>     when offlining") removed the set on offline.
>>> * Commit 5ecae6359e3a ("mm/memory_hotplug: drop PageReserved() check in
>>>     online_pages_range()") removed the check on online.
>>>
>>> This means we set PG_reserved for hot-plugged memory at initialization
>>> is not helpful and a little different from bootmem initialization path.
>>> Now we can remove it.
>>
>> It's not that easy for ZONE_DEVICE.
>>
>> Also, see mm/mm-stable
>>
>> commit 3dadec1babf9eee0c67c967df931d6f0cb124a04
>> Author: David Hildenbrand <david@redhat.com>
>> Date:   Fri Jun 7 11:09:36 2024 +0200
>>
>>     mm: pass meminit_context to __free_pages_core()
>>
>>     Patch series "mm/memory_hotplug: use PageOffline() instead of
>>     PageReserved() for !ZONE_DEVICE".
>>
>>
>> commit b873faaa609ab44c223b2327f55d2b6a2ba4ca9c
>> Author: David Hildenbrand <david@redhat.com>
>> Date:   Fri Jun 7 11:09:37 2024 +0200
>>
>>     mm/memory_hotplug: initialize memmap of !ZONE_DEVICE with PageOffline()
>> instead of PageReserved()
>>
> 
> Let me try to understand this.
> 
> You also tries to get rid of PG_reserved but you want PG_offline instead,
> because this benefit virtio-mem, right?

We now make proper use of PG_offline. All hotplugged pages start out 
PG_offline once we turn the section online. Only the ones that actually 
get exposed to the buddy -- actually get onlined -- get PG_offline 
cleared. A side effect of that is less hacks for virtio-mem, and more 
natural handling for the other ballooning drivers that hotplug memory.

In the future, I'm planning on moving more fake-offlining code from 
virtio-mem the core, making use of more PG_offline in memory.

For now, it's stops the PG_reserved use while maintaining the same 
semantics as before: the page content and "struct page" is not to be 
touched by anybody except the "owner".

> 
> But I don't get why PG_offline is wrong for ZONE_DEVICE. I may miss some
> knowledge for it.

I suggest you take a look at the PG_offline documentation. ZONE_DEVICE 
are certainly not logically offline pages. They will never be considered 
online as part of online sections. But they will never be handed to the 
buddy.

Maybe we want a dedicate page type for them in the future, not sure. We 
can right now identify them reliably using the zone idx.

Using a page type right now is very likely not possible, because we 
might be using the page->_mapcount in rmap code when mapping some of 
them to user space.

If we want to get rid of the PG_reserved for them right now, we'll have 
to make sure all existing PageReserved checks won't be degraded. For
example, drivers/vfio/vfio_iommu_type1.c might need some work (no sure).

The KVM one in kvm_pfn_to_refcounted_page() should already be fine, 
because they really want to refcount them.

A lot of other ones like can_gather_numa_stats(), already refuse 
is_zone_device_page() manually, and maybe we want to factor both checks 
out into a separate function like "is_special_reserved_page()" or sth 
like that.

-- 
Cheers,

David / dhildenb