Re: [PATCH v5 1/5] mm,memory_hotplug: Allocate memmap from the added memory range

[David Hildenbrand]

On 25.03.21 17:36, Michal Hocko wrote:
> On Thu 25-03-21 17:20:23, David Hildenbrand wrote:
> > On 25.03.21 17:07, Michal Hocko wrote:
> > > On Thu 25-03-21 16:35:58, Michal Hocko wrote:
> > > [...]
> > > > So there is indeed a difference. One way around that would be to mark
> > > > vmemmap pages (e.g. PageReserved && magic value stored somewhere in the
> > > > struct page - resembling bootmem vmemmaps) or mark section fully backing
> > > > vmemmaps as online (ugly).
> > >
> > > I am not yet ready to give up on this. Here is a quick stab at the
> > > pfn_to_online_page approach. It is not great but it is not really
> > > terrible either. I think we can do better and skip
> >
> > We both seem to have a different taste, to phrase it in a nice way :) ; but
> > well, you seem to have set your mind (just like I seem to have set mine when
> > trying to find a nice and somewhat-clean way to handle this when discussing
> > it in the past).
>
> I definitely do not want to fight for a certain solution just for the
> sake of it. I really dislike how the lifetime of the reserved space and
> its accounting are completely detached. But hey, I do understand that
> a worse solution from the design perspective can be better due to
> practical reasons or constrains.
>
> I haven't seen the hibernation problem before and I do recognize it is
> a nasty one. If all it takes is to make pfn_to_online_page work (and my
> previous attempt is incorrect because it should consult block rather
> than section pfn range) and there are no other downsides then I would
> still prefer to go with my proposal.  If there are still other things to
> plug then, well, practicality is going to win.
>
> So before I give up on the "proper" design card, are there more
> subtleties to watch for? You have certainly given this much more thought
> than I have.

"Just one more thing" :)

With the pfn_to_online_page() change, I think what remains is


1. The contiguous zone thingy, which we discussed is not a deal breaker, 
although sub-optimal and most probably not to be optimized in the future.

2. There corner cases issue with /dev/mem use case with offline memory 
blocks I mentioned. Existing setups (!memmap_on_memory) are not 
affected, so I guess we're fine.

3. valid_zones_show() has to be taught to only look at the !vmemmap 
part, otherwise we'll no longer indicate "movable" after onlining to the 
movable zone. Should be fairly easy.


We'll have pfn_to_online_section() succeed without SECTION_IS_ONLINE. I 
think I/we removed all such code that purely relied on that flag for 
optimizations like

if (!online_section(s))
	continue;


I can give it some more thought, it could fly. At least zone shrinking 
and hibernation should continue working as expected, which is a relief.

--
Thanks,

David / dhildenb