Re: [PATCH RFC v3 4/9] mm: Export alloc_contig_range() / free_contig_range()

From: David Hildenbrand
Date: Wed Oct 16 2019 - 08:31:20 EST

On 16.10.19 13:20, Michal Hocko wrote:
On Thu 19-09-19 16:22:23, David Hildenbrand wrote:
A virtio-mem device wants to allocate memory from the memory region it
manages in order to unplug it in the hypervisor - similar to
a balloon driver. Also, it might want to plug previously unplugged
(allocated) memory and give it back to Linux. alloc_contig_range() /
free_contig_range() seem to be the perfect interface for this task.

In contrast to existing balloon devices, a virtio-mem device operates
on bigger chunks (e.g., 4MB) and only on physical memory it manages. It
tracks which chunks (subblocks) are still plugged, so it can go ahead
and try to alloc_contig_range()+unplug them on unplug request, or
plug+free_contig_range() unplugged chunks on plug requests.

A virtio-mem device will use alloc_contig_range() / free_contig_range()
only on ranges that belong to the same node/zone in at least
MAX(MAX_ORDER - 1, pageblock_order) order granularity - e.g., 4MB on
x86-64. The virtio-mem device added that memory, so the memory
exists and does not contain any holes. virtio-mem will only try to allocate
on ZONE_NORMAL, never on ZONE_MOVABLE, just like when allocating
gigantic pages (we don't put unmovable data into the movable zone).

Is there any real reason to export as GPL rather than generic
EXPORT_SYMBOL? In other words do we need to restrict the usage this
interface only to GPL modules and why if so. All other allocator APIs
are EXPORT_SYMBOL so there should better be a good reason for this one
to differ. I can understand that this one is slightly different by
requesting a specific range of the memory but it is still under a full
control of the core MM to say no.

I thought that we might - at least initially - might want to know all users. If you prefer, I can drop the GPL.

Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: Vlastimil Babka <vbabka@xxxxxxx>
Cc: Oscar Salvador <osalvador@xxxxxxx>
Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
Cc: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
Cc: Pavel Tatashin <pavel.tatashin@xxxxxxxxxxxxx>
Cc: Alexander Potapenko <glider@xxxxxxxxxx>
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>

Other than that, I do not think exporting this function is harmful. It
would be worse to reinvent it and do it wrong.

I usually prefer to add a caller in the same patch, though, because it
makes the usage explicit and clear.

It's the next patch in this series (I prefer to split this from the actual driver):

Acked-by: Michal Hocko <mhocko@xxxxxxxx> # to export contig range allocator API




David / dhildenb