| Message ID | 20190919142228.5483-5-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-mem: paravirtualized memory | expand |
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. > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Mike Rapoport <rppt@linux.ibm.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com> > Cc: Pavel Tatashin <pavel.tatashin@microsoft.com> > Cc: Alexander Potapenko <glider@google.com> > Signed-off-by: David Hildenbrand <david@redhat.com> 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. Acked-by: Michal Hocko <mhocko@suse.com> # to export contig range allocator API > --- > mm/page_alloc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3334a769eb91..d5d7944954b3 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8469,6 +8469,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, > pfn_max_align_up(end), migratetype); > return ret; > } > +EXPORT_SYMBOL_GPL(alloc_contig_range); > #endif /* CONFIG_CONTIG_ALLOC */ > > void free_contig_range(unsigned long pfn, unsigned int nr_pages) > @@ -8483,6 +8484,7 @@ void free_contig_range(unsigned long pfn, unsigned int nr_pages) > } > WARN(count != 0, "%d pages are still in use!\n", count); > } > +EXPORT_SYMBOL_GPL(free_contig_range); > > #ifdef CONFIG_MEMORY_HOTPLUG > /* > -- > 2.21.0
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@linux-foundation.org> >> Cc: Michal Hocko <mhocko@suse.com> >> Cc: Vlastimil Babka <vbabka@suse.cz> >> Cc: Oscar Salvador <osalvador@suse.de> >> Cc: Mel Gorman <mgorman@techsingularity.net> >> Cc: Mike Rapoport <rppt@linux.ibm.com> >> Cc: Dan Williams <dan.j.williams@intel.com> >> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com> >> Cc: Pavel Tatashin <pavel.tatashin@microsoft.com> >> Cc: Alexander Potapenko <glider@google.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> > > 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): https://lkml.org/lkml/2019/9/19/486 > Acked-by: Michal Hocko <mhocko@suse.com> # to export contig range allocator API Thanks!
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 3334a769eb91..d5d7944954b3 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -8469,6 +8469,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, pfn_max_align_up(end), migratetype); return ret; } +EXPORT_SYMBOL_GPL(alloc_contig_range); #endif /* CONFIG_CONTIG_ALLOC */ void free_contig_range(unsigned long pfn, unsigned int nr_pages) @@ -8483,6 +8484,7 @@ void free_contig_range(unsigned long pfn, unsigned int nr_pages) } WARN(count != 0, "%d pages are still in use!\n", count); } +EXPORT_SYMBOL_GPL(free_contig_range); #ifdef CONFIG_MEMORY_HOTPLUG /*
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). Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Oscar Salvador <osalvador@suse.de> Cc: Mel Gorman <mgorman@techsingularity.net> Cc: Mike Rapoport <rppt@linux.ibm.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com> Cc: Pavel Tatashin <pavel.tatashin@microsoft.com> Cc: Alexander Potapenko <glider@google.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- mm/page_alloc.c | 2 ++ 1 file changed, 2 insertions(+)