Re: [PATCH v4 5/8] mm/memory_hotplug: MEMHP_MERGE_RESOURCE to specify merging of System RAM resources

[Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx>]

On Fri, Sep 11, 2020 at 12:34:56PM +0200, David Hildenbrand 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 a flag (MEMHP_MERGE_RESOURCE) to specify that a resource
>either created within add_memory*() or passed via add_memory_resource()
>shall be marked mergeable and merged with applicable siblings.
>
>To implement that, we need a kernel/resource interface to mark selected
>System RAM resources mergeable (IORESOURCE_SYSRAM_MERGEABLE) and trigger
>merging.
>
>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).

Oops, the latest version is here.

BTW, I don't see patch 4. Not sure it is junked by my mail system?

>
>Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx>
>Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>Cc: Michal Hocko <mhocko@xxxxxxxx>
>Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
>Cc: Jason Gunthorpe <jgg@xxxxxxxx>
>Cc: Kees Cook <keescook@xxxxxxxxxxxx>
>Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
>Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>Cc: "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>
>Cc: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
>Cc: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>
>Cc: Wei Liu <wei.liu@xxxxxxxxxx>
>Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>Cc: Juergen Gross <jgross@xxxxxxxx>
>Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>Cc: Julien Grall <julien@xxxxxxx>
>Cc: Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx>
>Cc: Baoquan He <bhe@xxxxxxxxxx>
>Cc: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx>
>Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>---
> include/linux/ioport.h         |  4 +++
> include/linux/memory_hotplug.h |  7 ++++
> kernel/resource.c              | 60 ++++++++++++++++++++++++++++++++++
> mm/memory_hotplug.c            |  7 ++++
> 4 files changed, 78 insertions(+)
>
>diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>index d7620d7c941a0..7e61389dcb017 100644
>--- a/include/linux/ioport.h
>+++ b/include/linux/ioport.h
>@@ -60,6 +60,7 @@ struct resource {
> 
> /* IORESOURCE_SYSRAM specific bits. */
> #define IORESOURCE_SYSRAM_DRIVER_MANAGED      0x02000000 /* Always detected 
> via a driver. */
>+#define IORESOURCE_SYSRAM_MERGEABLE           0x04000000 /* Resource can be 
>merged. */
> 
> #define IORESOURCE_EXCLUSIVE  0x08000000      /* Userland may not map this 
> resource */
> 
>@@ -253,6 +254,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_resource(struct resource *res);
>+#endif
> 
> /* Wrappers for managed devices */
> struct device;
>diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>index 33eb80fdba22f..d65c6fdc5cfc3 100644
>--- a/include/linux/memory_hotplug.h
>+++ b/include/linux/memory_hotplug.h
>@@ -62,6 +62,13 @@ typedef int __bitwise mhp_t;
> 
> /* No special request */
> #define MHP_NONE              ((__force mhp_t)0)
>+/*
>+ * Allow merging of the added System RAM resource with adjacent,
>+ * mergeable resources. After a successful call to add_memory_resource()
>+ * with this flag set, the resource pointer must no longer be used as it
>+ * might be stale, or the resource might have changed.
>+ */
>+#define MEMHP_MERGE_RESOURCE  ((__force mhp_t)BIT(0))
> 
> /*
>  * Extended parameters for memory hotplug:
>diff --git a/kernel/resource.c b/kernel/resource.c
>index 36b3552210120..7a91b935f4c20 100644
>--- a/kernel/resource.c
>+++ b/kernel/resource.c
>@@ -1363,6 +1363,66 @@ 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)
>+{
>+      /* We assume either r1 or r2 is IORESOURCE_SYSRAM_MERGEABLE. */
>+      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_resource - mark the System RAM resource mergeable and try 
>to
>+ * merge it with adjacent, mergeable resources
>+ * @res: 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 resources that are marked mergeable, that have 
>the
>+ * same parent, and that don't have any children are considered. All mergeable
>+ * resources must be immutable during the request.
>+ *
>+ * Note:
>+ * - The caller has to make sure that no pointers to resources that are
>+ *   marked mergeable are used anymore after this call - the resource might
>+ *   be freed and the pointer might be stale!
>+ * - release_mem_region_adjustable() will split on demand on memory hotunplug
>+ */
>+void merge_system_ram_resource(struct resource *res)
>+{
>+      const unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>+      struct resource *cur;
>+
>+      if (WARN_ON_ONCE((res->flags & flags) != flags))
>+              return;
>+
>+      write_lock(&resource_lock);
>+      res->flags |= IORESOURCE_SYSRAM_MERGEABLE;
>+
>+      /* Try to merge with next item in the list. */
>+      cur = res->sibling;
>+      if (cur && system_ram_resources_mergeable(res, cur)) {
>+              res->end = cur->end;
>+              res->sibling = cur->sibling;
>+              free_resource(cur);
>+      }
>+
>+      /* Try to merge with previous item in the list. */
>+      cur = res->parent->child;
>+      while (cur && cur->sibling != res)
>+              cur = cur->sibling;
>+      if (cur && system_ram_resources_mergeable(cur, res)) {
>+              cur->end = res->end;
>+              cur->sibling = res->sibling;
>+              free_resource(res);
>+      }
>+      write_unlock(&resource_lock);
>+}
>+#endif        /* CONFIG_MEMORY_HOTPLUG */
>+
> /*
>  * Managed region resource
>  */
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index 8f0bd7c9a63a5..553c718226b3e 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -1102,6 +1102,13 @@ int __ref add_memory_resource(int nid, struct resource 
>*res, mhp_t mhp_flags)
>       /* device_online() will take the lock when calling online_pages() */
>       mem_hotplug_done();
> 
>+      /*
>+       * In case we're allowed to merge the resource, flag it and trigger
>+       * merging now that adding succeeded.
>+       */
>+      if (mhp_flags & MEMHP_MERGE_RESOURCE)
>+              merge_system_ram_resource(res);
>+
>       /* online pages if requested */
>       if (memhp_default_online_type != MMOP_OFFLINE)
>               walk_memory_blocks(start, size, NULL, online_memory_block);
>-- 
>2.26.2

-- 
Wei Yang
Help you, Help me