On Thu 25-03-21 15:02:26, Oscar Salvador wrote:

On Thu, Mar 25, 2021 at 01:26:34PM +0100, Michal Hocko wrote:

quoted

Yeah, David has raised the contiguous flag for zone already. And to be
completely honest I fail to see why we should shape a design based on an
optimization. If anything we can teach set_zone_contiguous to simply
ignore zone affiliation of vmemmap pages. I would be really curious if
that would pose any harm to the compaction code as they are reserved and
compaction should simply skip them.

No, compaction code is clever enough to skip over those pages as it
already does for any Reserved page.
My comment was more towards having the zone contiguous.

I know it is an optimization, but

 commit 7cf91a98e607c2f935dbcc177d70011e95b8faff
 Author: Joonsoo Kim [off-list ref]
 Date:   Tue Mar 15 14:57:51 2016 -0700
 
 mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous

talks about 30% of improvment. I am not sure if those numbers would
still hold nowawadys, but it feels wrong to drop it to the ground when
we can do better there, and IMHO, it does not overly complicate things.

Again, do not shape design around an optimization. If this turns out a
real problem then it can be handled on top.

quoted

THere is nothing like a proper zone.

I guess not, but for me it makes sense that vmemmap pages stay within
the same zone as the pages they describe.

This is not the case for normal hotplug so why this should be any
different.

Of course, this is a matter of opinions/taste.

quoted

Not sure what you are referring to but if you have prior to f1dd2cd13c4b
("mm, memory_hotplug: do not associate hotadded memory to zones until
online") then this was entirely a different story. Users do care where
they memory goes because that depends on the usecase but do they care
about vmemmap?

As I said, that is not what I am worried about.
Users do not really care where those pages end up, that is transparent
to them (wrt. vmemmap pages), but we (internally) kind of do.

So, as I said, I see advantatges of using your way, but I see downsides
as:

- I would like to consider zone, and for that we would have to pull
  some of the functions that check for the zone at an aearly stage, and
  the mere thought sounds ugly.

This is impossible and whatever kind of heuristic you come up with might
be wrong.

- Section containing vmemmap can remain offline and would have to come
  up to sort that out

Yes, this is a problem indeed and as I've said in other email this would
be a problem for your initial implementation as well if the memory block
is still offline. I suspect we need to treat these Vmemmap pages as
online (via pfn_to_online_page).
-- 
Michal Hocko
SUSE Labs