Re: [PATCH v2 2/5] mm,memory_hotplug: Introduce MHP_VMEMMAP_FLAGS

From: osalvador
Date: Wed Jul 24 2019 - 17:36:55 EST


On 2019-07-24 22:11, Dan Williams wrote:
On Tue, Jun 25, 2019 at 12:53 AM Oscar Salvador <osalvador@xxxxxxx> wrote:

This patch introduces MHP_MEMMAP_DEVICE and MHP_MEMMAP_MEMBLOCK flags,
and prepares the callers that add memory to take a "flags" parameter.
This "flags" parameter will be evaluated later on in Patch#3
to init mhp_restrictions struct.

The callers are:

add_memory
__add_memory
add_memory_resource

Unfortunately, we do not have a single entry point to add memory, as depending
on the requisites of the caller, they want to hook up in different places,
(e.g: Xen reserve_additional_memory()), so we have to spread the parameter
in the three callers.

The flags are either MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK, and only differ
in the way they allocate vmemmap pages within the memory blocks.

MHP_MEMMAP_MEMBLOCK:
- With this flag, we will allocate vmemmap pages in each memory block.
This means that if we hot-add a range that spans multiple memory blocks,
we will use the beginning of each memory block for the vmemmap pages.
This strategy is good for cases where the caller wants the flexiblity
to hot-remove memory in a different granularity than when it was added.

E.g:
We allocate a range (x,y], that spans 3 memory blocks, and given
memory block size = 128MB.
[memblock#0 ]
[0 - 511 pfns ] - vmemmaps for section#0
[512 - 32767 pfns ] - normal memory

[memblock#1 ]
[32768 - 33279 pfns] - vmemmaps for section#1
[33280 - 65535 pfns] - normal memory

[memblock#2 ]
[65536 - 66047 pfns] - vmemmap for section#2
[66048 - 98304 pfns] - normal memory

MHP_MEMMAP_DEVICE:
- With this flag, we will store all vmemmap pages at the beginning of
hot-added memory.

E.g:
We allocate a range (x,y], that spans 3 memory blocks, and given
memory block size = 128MB.
[memblock #0 ]
[0 - 1533 pfns ] - vmemmap for section#{0-2}
[1534 - 98304 pfns] - normal memory

When using larger memory blocks (1GB or 2GB), the principle is the same.

Of course, MHP_MEMMAP_DEVICE is nicer when it comes to have a large contigous
area, while MHP_MEMMAP_MEMBLOCK allows us to have flexibility when removing the
memory.

Concept and patch looks good to me, but I don't quite like the
proliferation of the _DEVICE naming, in theory it need not necessarily
be ZONE_DEVICE that is the only user of that flag. I also think it
might be useful to assign a flag for the default 'allocate from RAM'
case, just so the code is explicit. So, how about:

MHP_MEMMAP_PAGE_ALLOC
MHP_MEMMAP_MEMBLOCK
MHP_MEMMAP_RESERVED

...for the 3 cases?

Other than that, feel free to add:

Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
HI Dan,

I'll be sending V3 tomorrow, with some major rewrites (more simplified).

Thanks