On Thu 25-03-21 15:02:26, Oscar Salvador wrote:
> On Thu, Mar 25, 2021 at 01:26:34PM +0100, Michal Hocko wrote:
> > 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 <iamjoonsoo.kim@xxxxxxx>
> 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.
> > 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.
>
> > 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
