Re: [PATCH v1 2/3] mm/memory_hotplug: initialize memmap of !ZONE_DEVICE with PageOffline() instead of PageReserved()

[David Hildenbrand <david@xxxxxxxxxx>]

On 11.06.24 09:45, Oscar Salvador wrote:
> On Mon, Jun 10, 2024 at 10:56:02AM +0200, David Hildenbrand wrote:
> > There are fortunately not that many left.
> >
> > I'd even say marking them (vmemmap) reserved is more wrong than right: note
> > that ordinary vmemmap pages after memory hotplug are not reserved! Only
> > bootmem should be reserved.
> Ok, that is a very good point that I missed.
> I thought that hotplugged-vmemmap pages (not selfhosted) were marked as
> Reserved, that is why I thought this would be inconsistent.
> But then, if that is the case, I think we are safe as kernel can already
> encounter vmemmap pages that are not reserved and it deals with them
> somehow.
>
> > Let's take at the relevant core-mm ones (arch stuff is mostly just for MMIO
> > remapping)
> ...
> > Any PageReserved user that I am missing, or why we should handle these
> > vmemmap pages differently than the ones allocated during ordinary memory
> > hotplug?
> No, I cannot think of a reason why normal vmemmap pages should behave
> different than self-hosted.
>
> I was also confused because I thought that after this change
> pfn_to_online_page() would be different for self-hosted vmemmap pages,
> because I thought that somehow we relied on PageOffline(), but it is not
> the case.
Fortunately not :) PageFakeOffline() or PageLogicallyOffline()  might be 
clearer, but I don't quite like these names. If you have a good idea, 
please let me know.
> > In the future, we might want to consider using a dedicated page type for
> > them, so we can stop using a bit that doesn't allow to reliably identify
> > them. (we should mark all vmemmap with that type then)
> Yes, a all-vmemmap pages type would be a good thing, so we do not have
> to special case.
>
> Just one last thing.
> Now self-hosted vmemmap pages will have the PageOffline cleared, and that
> will still remain after the memory-block they belong to has gone
> offline, which is ok because those vmemmap pages lay around until the
> chunk of memory gets removed.
Yes, and that memmap might even get poisoned in debug kernels to catch 
any wrong access.
> Ok, just wanted to convince myself that there will no be surprises.
>
> Thanks David for claryfing.
Thanks for the review and raising that. I'll add more details to the 
patch description!
--
Cheers,

David / dhildenb