David Hildenbrand
2020-Aug-21 10:34 UTC
[PATCH v1 0/5] mm/memory_hotplug: selective merging of system ram resources
This is the follow-up of "[PATCH RFCv1 0/5] mm/memory_hotplug: selective merging of memory resources" [1] Some add_memory*() users add memory in small, contiguous memory blocks. Examples include virtio-mem, hyper-v balloon, and the XEN balloon. This can quickly result in a lot of memory resources, whereby the actual resource boundaries are not of interest (e.g., it might be relevant for DIMMs, exposed via /proc/iomem to user space). We really want to merge added resources in this scenario where possible. Resources are effectively stored in a list-based tree. Having a lot of resources not only wastes memory, it also makes traversing that tree more expensive, and makes /proc/iomem explode in size (e.g., requiring kexec-tools to manually merge resources when creating a kdump header. The current kexec-tools resource count limit does not allow for more than ~100GB of memory with a memory block size of 128MB on x86-64). Let's allow to selectively merge system ram resources directly below a specific parent resource. Patch #3 contains a /proc/iomem example. Only tested with virtio-mem. Note: This gets the job done and is comparably simple. More complicated approaches would require introducing IORESOURCE_MERGEABLE and extending our add_memory*() interfaces with a flag, specifying that merging after adding succeeded is acceptable. I'd like to avoid that complexity and code churn for now. [1] https://lkml.kernel.org/r/20200731091838.7490-1-david at redhat.com RFC -> v1: - Switch from rather generic "merge_child_mem_resources()" where a resource name has to be specified to "merge_system_ram_resources(). - Smaller comment/documentation/patch description changes/fixes David Hildenbrand (5): kernel/resource: make release_mem_region_adjustable() never fail kernel/resource: merge_system_ram_resources() to merge resources after hotplug virtio-mem: try to merge system ram resources xen/balloon: try to merge system ram resources hv_balloon: try to merge system ram resources drivers/hv/hv_balloon.c | 3 ++ drivers/virtio/virtio_mem.c | 14 ++++- drivers/xen/balloon.c | 4 ++ include/linux/ioport.h | 7 ++- kernel/resource.c | 101 ++++++++++++++++++++++++++++-------- mm/memory_hotplug.c | 22 +------- 6 files changed, 105 insertions(+), 46 deletions(-) -- 2.26.2
David Hildenbrand
2020-Aug-21 10:34 UTC
[PATCH v1 1/5] kernel/resource: make release_mem_region_adjustable() never fail
Let's make sure splitting a resource on memory hotunplug will never fail. This will become more relevant once we merge selected system ram resources - then, we'll trigger that case more frequently on memory hotunplug. In general, this function is already unlikely to fail. When we remove memory, we free up quite a lot of metadata (memmap, page tables, memory block device, etc.). The only way it could really fail currently is when injecting allocation errors. All other error cases inside release_mem_region_adjustable() seem to be sanity checks if the function would be abused in different context - let's add WARN_ON_ONCE(). Cc: Andrew Morton <akpm at linux-foundation.org> Cc: Michal Hocko <mhocko at suse.com> Cc: Dan Williams <dan.j.williams at intel.com> Cc: Jason Gunthorpe <jgg at ziepe.ca> Cc: Kees Cook <keescook at chromium.org> Cc: Ard Biesheuvel <ardb at kernel.org> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> Cc: Baoquan He <bhe at redhat.com> Cc: Wei Yang <richardw.yang at linux.intel.com> Signed-off-by: David Hildenbrand <david at redhat.com> --- include/linux/ioport.h | 4 ++-- kernel/resource.c | 49 ++++++++++++++++++++++++------------------ mm/memory_hotplug.c | 22 +------------------ 3 files changed, 31 insertions(+), 44 deletions(-) diff --git a/include/linux/ioport.h b/include/linux/ioport.h index 6c2b06fe8beb7..52a91f5fa1a36 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h @@ -248,8 +248,8 @@ extern struct resource * __request_region(struct resource *, extern void __release_region(struct resource *, resource_size_t, resource_size_t); #ifdef CONFIG_MEMORY_HOTREMOVE -extern int release_mem_region_adjustable(struct resource *, resource_size_t, - resource_size_t); +extern void release_mem_region_adjustable(struct resource *, resource_size_t, + resource_size_t); #endif /* Wrappers for managed devices */ diff --git a/kernel/resource.c b/kernel/resource.c index 841737bbda9e5..1dcef5d53d76e 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -1255,21 +1255,28 @@ EXPORT_SYMBOL(__release_region); * assumes that all children remain in the lower address entry for * simplicity. Enhance this logic when necessary. */ -int release_mem_region_adjustable(struct resource *parent, - resource_size_t start, resource_size_t size) +void release_mem_region_adjustable(struct resource *parent, + resource_size_t start, resource_size_t size) { + struct resource *new_res = NULL; + bool alloc_nofail = false; struct resource **p; struct resource *res; - struct resource *new_res; resource_size_t end; - int ret = -EINVAL; end = start + size - 1; - if ((start < parent->start) || (end > parent->end)) - return ret; + if (WARN_ON_ONCE((start < parent->start) || (end > parent->end))) + return; - /* The alloc_resource() result gets checked later */ - new_res = alloc_resource(GFP_KERNEL); + /* + * We free up quite a lot of memory on memory hotunplug (esp., memap), + * just before releasing the region. This is highly unlikely to + * fail - let's play save and make it never fail as the caller cannot + * perform any error handling (e.g., trying to re-add memory will fail + * similarly). + */ +retry: + new_res = alloc_resource(GFP_KERNEL | alloc_nofail ? __GFP_NOFAIL : 0); p = &parent->child; write_lock(&resource_lock); @@ -1295,7 +1302,6 @@ int release_mem_region_adjustable(struct resource *parent, * so if we are dealing with them, let us just back off here. */ if (!(res->flags & IORESOURCE_SYSRAM)) { - ret = 0; break; } @@ -1312,20 +1318,23 @@ int release_mem_region_adjustable(struct resource *parent, /* free the whole entry */ *p = res->sibling; free_resource(res); - ret = 0; } else if (res->start == start && res->end != end) { /* adjust the start */ - ret = __adjust_resource(res, end + 1, - res->end - end); + WARN_ON_ONCE(__adjust_resource(res, end + 1, + res->end - end)); } else if (res->start != start && res->end == end) { /* adjust the end */ - ret = __adjust_resource(res, res->start, - start - res->start); + WARN_ON_ONCE(__adjust_resource(res, res->start, + start - res->start)); } else { - /* split into two entries */ + /* split into two entries - we need a new resource */ if (!new_res) { - ret = -ENOMEM; - break; + new_res = alloc_resource(GFP_ATOMIC); + if (!new_res) { + alloc_nofail = true; + write_unlock(&resource_lock); + goto retry; + } } new_res->name = res->name; new_res->start = end + 1; @@ -1336,9 +1345,8 @@ int release_mem_region_adjustable(struct resource *parent, new_res->sibling = res->sibling; new_res->child = NULL; - ret = __adjust_resource(res, res->start, - start - res->start); - if (ret) + if (WARN_ON_ONCE(__adjust_resource(res, res->start, + start - res->start))) break; res->sibling = new_res; new_res = NULL; @@ -1349,7 +1357,6 @@ int release_mem_region_adjustable(struct resource *parent, write_unlock(&resource_lock); free_resource(new_res); - return ret; } #endif /* CONFIG_MEMORY_HOTREMOVE */ diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 35d56cbd3e45b..f77fd1810e9f5 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1700,26 +1700,6 @@ void try_offline_node(int nid) } EXPORT_SYMBOL(try_offline_node); -static void __release_memory_resource(resource_size_t start, - resource_size_t size) -{ - int ret; - - /* - * When removing memory in the same granularity as it was added, - * this function never fails. It might only fail if resources - * have to be adjusted or split. We'll ignore the error, as - * removing of memory cannot fail. - */ - ret = release_mem_region_adjustable(&iomem_resource, start, size); - if (ret) { - resource_size_t endres = start + size - 1; - - pr_warn("Unable to release resource <%pa-%pa> (%d)\n", - &start, &endres, ret); - } -} - static int __ref try_remove_memory(int nid, u64 start, u64 size) { int rc = 0; @@ -1753,7 +1733,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) memblock_remove(start, size); } - __release_memory_resource(start, size); + release_mem_region_adjustable(&iomem_resource, start, size); try_offline_node(nid); -- 2.26.2
David Hildenbrand
2020-Aug-21 10:34 UTC
[PATCH v1 2/5] kernel/resource: merge_system_ram_resources() to merge resources after hotplug
Some add_memory*() users add memory in small, contiguous memory blocks. Examples include virtio-mem, hyper-v balloon, and the XEN balloon. This can quickly result in a lot of memory resources, whereby the actual resource boundaries are not of interest (e.g., it might be relevant for DIMMs, exposed via /proc/iomem to user space). We really want to merge added resources in this scenario where possible. Let's provide an interface to trigger merging of applicable child resources. It will be, for example, used by virtio-mem to trigger merging of system ram resources it added to its resource container, but also by XEN and Hyper-V to trigger merging of system ram resources in iomem_resource. Note: We really want to merge after the whole operation succeeded, not directly when adding a resource to the resource tree (it would break add_memory_resource() and require splitting resources again when the operation failed - e.g., due to -ENOMEM). Cc: Andrew Morton <akpm at linux-foundation.org> Cc: Michal Hocko <mhocko at suse.com> Cc: Dan Williams <dan.j.williams at intel.com> Cc: Jason Gunthorpe <jgg at ziepe.ca> Cc: Kees Cook <keescook at chromium.org> Cc: Ard Biesheuvel <ardb at kernel.org> Cc: Thomas Gleixner <tglx at linutronix.de> Cc: "K. Y. Srinivasan" <kys at microsoft.com> Cc: Haiyang Zhang <haiyangz at microsoft.com> Cc: Stephen Hemminger <sthemmin at microsoft.com> Cc: Wei Liu <wei.liu at kernel.org> Cc: Boris Ostrovsky <boris.ostrovsky at oracle.com> Cc: Juergen Gross <jgross at suse.com> Cc: Stefano Stabellini <sstabellini at kernel.org> Cc: Roger Pau Monn? <roger.pau at citrix.com> Cc: Julien Grall <julien at xen.org> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> Cc: Baoquan He <bhe at redhat.com> Cc: Wei Yang <richardw.yang at linux.intel.com> Signed-off-by: David Hildenbrand <david at redhat.com> --- include/linux/ioport.h | 3 +++ kernel/resource.c | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/include/linux/ioport.h b/include/linux/ioport.h index 52a91f5fa1a36..3bb0020cd6ddc 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h @@ -251,6 +251,9 @@ extern void __release_region(struct resource *, resource_size_t, extern void release_mem_region_adjustable(struct resource *, resource_size_t, resource_size_t); #endif +#ifdef CONFIG_MEMORY_HOTPLUG +extern void merge_system_ram_resources(struct resource *res); +#endif /* Wrappers for managed devices */ struct device; diff --git a/kernel/resource.c b/kernel/resource.c index 1dcef5d53d76e..b4e0963edadd2 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -1360,6 +1360,58 @@ void release_mem_region_adjustable(struct resource *parent, } #endif /* CONFIG_MEMORY_HOTREMOVE */ +#ifdef CONFIG_MEMORY_HOTPLUG +static bool system_ram_resources_mergeable(struct resource *r1, + struct resource *r2) +{ + return r1->flags == r2->flags && r1->end + 1 == r2->start && + r1->name == r2->name && r1->desc == r2->desc && + !r1->child && !r2->child; +} + +/* + * merge_system_ram_resources - try to merge contiguous system ram resources + * @parent: parent resource descriptor + * + * This interface is intended for memory hotplug, whereby lots of contiguous + * system ram resources are added (e.g., via add_memory*()) by a driver, and + * the actual resource boundaries are not of interest (e.g., it might be + * relevant for DIMMs). Only immediate child resources that are busy and + * don't have any children are considered. All applicable child resources + * must be immutable during the request. + * + * Note: + * - The caller has to make sure that no pointers to resources that might + * get merged are held anymore. Callers should only trigger merging of child + * resources when they are the only one adding system ram resources to the + * parent (besides during boot). + * - release_mem_region_adjustable() will split on demand on memory hotunplug + */ +void merge_system_ram_resources(struct resource *parent) +{ + const unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + struct resource *cur, *next; + + write_lock(&resource_lock); + + cur = parent->child; + while (cur && cur->sibling) { + next = cur->sibling; + if ((cur->flags & flags) == flags && + system_ram_resources_mergeable(cur, next)) { + cur->end = next->end; + cur->sibling = next->sibling; + free_resource(next); + next = cur->sibling; + } + cur = next; + } + + write_unlock(&resource_lock); +} +EXPORT_SYMBOL(merge_system_ram_resources); +#endif /* CONFIG_MEMORY_HOTPLUG */ + /* * Managed region resource */ -- 2.26.2
David Hildenbrand
2020-Aug-21 10:34 UTC
[PATCH v1 3/5] virtio-mem: try to merge system ram resources
virtio-mem adds memory in memory block granularity, to be able to remove it in the same granularity again later, and to grow slowly on demand. This, however, results in quite a lot of resources when adding a lot of memory. Resources are effectively stored in a list-based tree. Having a lot of resources not only wastes memory, it also makes traversing that tree more expensive, and makes /proc/iomem explode in size (e.g., requiring kexec-tools to manually merge resources later when e.g., trying to create a kdump header). Before this patch, we get (/proc/iomem) when hotplugging 2G via virtio-mem on x86-64: [...] 100000000-13fffffff : System RAM 140000000-33fffffff : virtio0 140000000-147ffffff : System RAM (virtio_mem) 148000000-14fffffff : System RAM (virtio_mem) 150000000-157ffffff : System RAM (virtio_mem) 158000000-15fffffff : System RAM (virtio_mem) 160000000-167ffffff : System RAM (virtio_mem) 168000000-16fffffff : System RAM (virtio_mem) 170000000-177ffffff : System RAM (virtio_mem) 178000000-17fffffff : System RAM (virtio_mem) 180000000-187ffffff : System RAM (virtio_mem) 188000000-18fffffff : System RAM (virtio_mem) 190000000-197ffffff : System RAM (virtio_mem) 198000000-19fffffff : System RAM (virtio_mem) 1a0000000-1a7ffffff : System RAM (virtio_mem) 1a8000000-1afffffff : System RAM (virtio_mem) 1b0000000-1b7ffffff : System RAM (virtio_mem) 1b8000000-1bfffffff : System RAM (virtio_mem) 3280000000-32ffffffff : PCI Bus 0000:00 With this patch, we get (/proc/iomem): [...] fffc0000-ffffffff : Reserved 100000000-13fffffff : System RAM 140000000-33fffffff : virtio0 140000000-1bfffffff : System RAM (virtio_mem) 3280000000-32ffffffff : PCI Bus 0000:00 Of course, with more hotplugged memory, it gets worse. When unplugging memory blocks again, try_remove_memory() (via offline_and_remove_memory()) will properly split the resource up again. Cc: Andrew Morton <akpm at linux-foundation.org> Cc: Michal Hocko <mhocko at suse.com> Cc: Dan Williams <dan.j.williams at intel.com> Cc: Michael S. Tsirkin <mst at redhat.com> Cc: Jason Wang <jasowang at redhat.com> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> Cc: Baoquan He <bhe at redhat.com> Cc: Wei Yang <richardw.yang at linux.intel.com> Signed-off-by: David Hildenbrand <david at redhat.com> --- drivers/virtio/virtio_mem.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c index 834b7c13ef3dc..3aae0f87073a8 100644 --- a/drivers/virtio/virtio_mem.c +++ b/drivers/virtio/virtio_mem.c @@ -407,6 +407,7 @@ static int virtio_mem_mb_add(struct virtio_mem *vm, unsigned long mb_id) { const uint64_t addr = virtio_mem_mb_id_to_phys(mb_id); int nid = vm->nid; + int rc; if (nid == NUMA_NO_NODE) nid = memory_add_physaddr_to_nid(addr); @@ -423,8 +424,17 @@ static int virtio_mem_mb_add(struct virtio_mem *vm, unsigned long mb_id) } dev_dbg(&vm->vdev->dev, "adding memory block: %lu\n", mb_id); - return add_memory_driver_managed(nid, addr, memory_block_size_bytes(), - vm->resource_name); + rc = add_memory_driver_managed(nid, addr, memory_block_size_bytes(), + vm->resource_name); + if (!rc) { + /* + * Try to reduce the number of system ram resources in our + * resource container. The memory removal path will properly + * split them up again. + */ + merge_system_ram_resources(vm->parent_resource); + } + return rc; } /* -- 2.26.2
David Hildenbrand
2020-Aug-21 10:34 UTC
[PATCH v1 4/5] xen/balloon: try to merge system ram resources
Let's reuse the new mechanism to merge system ram resources below the root. We are the only one hotplugging system ram (e.g., DIMMs don't apply), so this is safe to be used. Cc: Andrew Morton <akpm at linux-foundation.org> Cc: Michal Hocko <mhocko at suse.com> Cc: Boris Ostrovsky <boris.ostrovsky at oracle.com> Cc: Juergen Gross <jgross at suse.com> Cc: Stefano Stabellini <sstabellini at kernel.org> Cc: Roger Pau Monn? <roger.pau at citrix.com> Cc: Julien Grall <julien at xen.org> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> Cc: Baoquan He <bhe at redhat.com> Cc: Wei Yang <richardw.yang at linux.intel.com> Signed-off-by: David Hildenbrand <david at redhat.com> --- drivers/xen/balloon.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 37ffccda8bb87..5ec73f752b8a7 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -338,6 +338,10 @@ static enum bp_state reserve_additional_memory(void) if (rc) { pr_warn("Cannot add additional memory (%i)\n", rc); goto err; + } else { + resource = NULL; + /* Try to reduce the number of system ram resources. */ + merge_system_ram_resources(&iomem_resource); } balloon_stats.total_pages += balloon_hotplug; -- 2.26.2
David Hildenbrand
2020-Aug-21 10:34 UTC
[PATCH v1 5/5] hv_balloon: try to merge system ram resources
Let's use the new mechanism to merge system ram resources below the root. We are the only one hotplugging system ram, e.g., DIMMs don't apply, so this is safe to be used. Cc: Andrew Morton <akpm at linux-foundation.org> Cc: Michal Hocko <mhocko at suse.com> Cc: "K. Y. Srinivasan" <kys at microsoft.com> Cc: Haiyang Zhang <haiyangz at microsoft.com> Cc: Stephen Hemminger <sthemmin at microsoft.com> Cc: Wei Liu <wei.liu at kernel.org> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> Cc: Baoquan He <bhe at redhat.com> Cc: Wei Yang <richardw.yang at linux.intel.com> Signed-off-by: David Hildenbrand <david at redhat.com> --- drivers/hv/hv_balloon.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 32e3bc0aa665a..49a6305f0fb73 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -745,6 +745,9 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, has->covered_end_pfn -= processed_pfn; spin_unlock_irqrestore(&dm_device.ha_lock, flags); break; + } else { + /* Try to reduce the number of system ram resources. */ + merge_system_ram_resources(&iomem_resource); } /* -- 2.26.2
Jürgen Groß
2020-Sep-02 10:15 UTC
[PATCH v1 4/5] xen/balloon: try to merge system ram resources
On 21.08.20 12:34, David Hildenbrand wrote:> Let's reuse the new mechanism to merge system ram resources below the > root. We are the only one hotplugging system ram (e.g., DIMMs don't apply), > so this is safe to be used. > > Cc: Andrew Morton <akpm at linux-foundation.org> > Cc: Michal Hocko <mhocko at suse.com> > Cc: Boris Ostrovsky <boris.ostrovsky at oracle.com> > Cc: Juergen Gross <jgross at suse.com> > Cc: Stefano Stabellini <sstabellini at kernel.org> > Cc: Roger Pau Monn? <roger.pau at citrix.com> > Cc: Julien Grall <julien at xen.org> > Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> > Cc: Baoquan He <bhe at redhat.com> > Cc: Wei Yang <richardw.yang at linux.intel.com> > Signed-off-by: David Hildenbrand <david at redhat.com> > --- > drivers/xen/balloon.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 37ffccda8bb87..5ec73f752b8a7 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -338,6 +338,10 @@ static enum bp_state reserve_additional_memory(void) > if (rc) { > pr_warn("Cannot add additional memory (%i)\n", rc); > goto err; > + } else { > + resource = NULL; > + /* Try to reduce the number of system ram resources. */ > + merge_system_ram_resources(&iomem_resource); > }I don't see the need for setting resource to NULL and to use an "else" clause here. Juergen
David Hildenbrand
2020-Sep-08 10:26 UTC
[PATCH v1 2/5] kernel/resource: merge_system_ram_resources() to merge resources after hotplug
On 31.08.20 11:35, Pankaj Gupta wrote:>> Some add_memory*() users add memory in small, contiguous memory blocks. >> Examples include virtio-mem, hyper-v balloon, and the XEN balloon. >> >> This can quickly result in a lot of memory resources, whereby the actual >> resource boundaries are not of interest (e.g., it might be relevant for >> DIMMs, exposed via /proc/iomem to user space). We really want to merge >> added resources in this scenario where possible. >> >> Let's provide an interface to trigger merging of applicable child >> resources. It will be, for example, used by virtio-mem to trigger >> merging of system ram resources it added to its resource container, but >> also by XEN and Hyper-V to trigger merging of system ram resources in >> iomem_resource. >> >> Note: We really want to merge after the whole operation succeeded, not >> directly when adding a resource to the resource tree (it would break >> add_memory_resource() and require splitting resources again when the >> operation failed - e.g., due to -ENOMEM). >> >> Cc: Andrew Morton <akpm at linux-foundation.org> >> Cc: Michal Hocko <mhocko at suse.com> >> Cc: Dan Williams <dan.j.williams at intel.com> >> Cc: Jason Gunthorpe <jgg at ziepe.ca> >> Cc: Kees Cook <keescook at chromium.org> >> Cc: Ard Biesheuvel <ardb at kernel.org> >> Cc: Thomas Gleixner <tglx at linutronix.de> >> Cc: "K. Y. Srinivasan" <kys at microsoft.com> >> Cc: Haiyang Zhang <haiyangz at microsoft.com> >> Cc: Stephen Hemminger <sthemmin at microsoft.com> >> Cc: Wei Liu <wei.liu at kernel.org> >> Cc: Boris Ostrovsky <boris.ostrovsky at oracle.com> >> Cc: Juergen Gross <jgross at suse.com> >> Cc: Stefano Stabellini <sstabellini at kernel.org> >> Cc: Roger Pau Monn? <roger.pau at citrix.com> >> Cc: Julien Grall <julien at xen.org> >> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> >> Cc: Baoquan He <bhe at redhat.com> >> Cc: Wei Yang <richardw.yang at linux.intel.com> >> Signed-off-by: David Hildenbrand <david at redhat.com> >> --- >> include/linux/ioport.h | 3 +++ >> kernel/resource.c | 52 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 55 insertions(+) >> >> diff --git a/include/linux/ioport.h b/include/linux/ioport.h >> index 52a91f5fa1a36..3bb0020cd6ddc 100644 >> --- a/include/linux/ioport.h >> +++ b/include/linux/ioport.h >> @@ -251,6 +251,9 @@ extern void __release_region(struct resource *, resource_size_t, >> extern void release_mem_region_adjustable(struct resource *, resource_size_t, >> resource_size_t); >> #endif >> +#ifdef CONFIG_MEMORY_HOTPLUG >> +extern void merge_system_ram_resources(struct resource *res); >> +#endif >> >> /* Wrappers for managed devices */ >> struct device; >> diff --git a/kernel/resource.c b/kernel/resource.c >> index 1dcef5d53d76e..b4e0963edadd2 100644 >> --- a/kernel/resource.c >> +++ b/kernel/resource.c >> @@ -1360,6 +1360,58 @@ void release_mem_region_adjustable(struct resource *parent, >> } >> #endif /* CONFIG_MEMORY_HOTREMOVE */ >> >> +#ifdef CONFIG_MEMORY_HOTPLUG >> +static bool system_ram_resources_mergeable(struct resource *r1, >> + struct resource *r2) >> +{ >> + return r1->flags == r2->flags && r1->end + 1 == r2->start && >> + r1->name == r2->name && r1->desc == r2->desc && >> + !r1->child && !r2->child; >> +} >> + >> +/* >> + * merge_system_ram_resources - try to merge contiguous system ram resources >> + * @parent: parent resource descriptor >> + * >> + * This interface is intended for memory hotplug, whereby lots of contiguous >> + * system ram resources are added (e.g., via add_memory*()) by a driver, and >> + * the actual resource boundaries are not of interest (e.g., it might be >> + * relevant for DIMMs). Only immediate child resources that are busy and >> + * don't have any children are considered. All applicable child resources >> + * must be immutable during the request. >> + * >> + * Note: >> + * - The caller has to make sure that no pointers to resources that might >> + * get merged are held anymore. Callers should only trigger merging of child >> + * resources when they are the only one adding system ram resources to the >> + * parent (besides during boot). >> + * - release_mem_region_adjustable() will split on demand on memory hotunplug >> + */ >> +void merge_system_ram_resources(struct resource *parent) >> +{ >> + const unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> + struct resource *cur, *next; >> + >> + write_lock(&resource_lock); >> + >> + cur = parent->child; >> + while (cur && cur->sibling) { >> + next = cur->sibling; >> + if ((cur->flags & flags) == flags && > > Maybe this can be changed to: > !(cur->flags & ~flags)That would be different I think. (cur->flags & flags) == flags checks that all "flags" are set (additional ones might be set). !(cur->flags & ~flags) checks that no other flags besides "flags" are set (and "flags" are not required to be set). We use the same handling in find_next_iomem_res(), e.g., called via walk_system_ram_range also with IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY. Thanks for having a look! -- Thanks, David / dhildenb
David Hildenbrand
2020-Sep-08 10:26 UTC
[PATCH v1 5/5] hv_balloon: try to merge system ram resources
On 04.09.20 19:30, Wei Liu wrote:> On Fri, Aug 21, 2020 at 12:34:31PM +0200, David Hildenbrand wrote: >> Let's use the new mechanism to merge system ram resources below the >> root. We are the only one hotplugging system ram, e.g., DIMMs don't apply, >> so this is safe to be used. >> >> Cc: Andrew Morton <akpm at linux-foundation.org> >> Cc: Michal Hocko <mhocko at suse.com> >> Cc: "K. Y. Srinivasan" <kys at microsoft.com> >> Cc: Haiyang Zhang <haiyangz at microsoft.com> >> Cc: Stephen Hemminger <sthemmin at microsoft.com> >> Cc: Wei Liu <wei.liu at kernel.org> >> Cc: Pankaj Gupta <pankaj.gupta.linux at gmail.com> >> Cc: Baoquan He <bhe at redhat.com> >> Cc: Wei Yang <richardw.yang at linux.intel.com> >> Signed-off-by: David Hildenbrand <david at redhat.com> >> --- >> drivers/hv/hv_balloon.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c >> index 32e3bc0aa665a..49a6305f0fb73 100644 >> --- a/drivers/hv/hv_balloon.c >> +++ b/drivers/hv/hv_balloon.c >> @@ -745,6 +745,9 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, >> has->covered_end_pfn -= processed_pfn; >> spin_unlock_irqrestore(&dm_device.ha_lock, flags); >> break; >> + } else { >> + /* Try to reduce the number of system ram resources. */ >> + merge_system_ram_resources(&iomem_resource); >> } > > You don't need to put the call under the "else" branch. It will have > broken out of the loop if ret is not zero. >Agreed, thanks! -- Thanks, David / dhildenb
Possibly Parallel Threads
- [PATCH v1 0/5] mm/memory_hotplug: selective merging of system ram resources
- [PATCH RFCv1 0/5] mm/memory_hotplug: selective merging of memory resources
- [PATCH v2 3/7] mm/memory_hotplug: prepare passing flags to add_memory() and friends
- [PATCH v3 3/7] mm/memory_hotplug: prepare passing flags to add_memory() and friends
- [PATCH v2 0/7] mm/memory_hotplug: selective merging of system ram resources