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

2020-09-16 Thread Wei Yang
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).
>
>Reviewed-by: Pankaj Gupta 
>Cc: Andrew Morton 
>Cc: Michal Hocko 
>Cc: Dan Williams 
>Cc: Jason Gunthorpe 
>Cc: Kees Cook 
>Cc: Ard Biesheuvel 
>Cc: Thomas Gleixner 
>Cc: "K. Y. Srinivasan" 
>Cc: Haiyang Zhang 
>Cc: Stephen Hemminger 
>Cc: Wei Liu 
>Cc: Boris Ostrovsky 
>Cc: Juergen Gross 
>Cc: Stefano Stabellini 
>Cc: Roger Pau Monné 
>Cc: Julien Grall 
>Cc: Pankaj Gupta 
>Cc: Baoquan He 
>Signed-off-by: David Hildenbrand 

Reviewed-by: Wei Yang 

>---
> 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  0x0200 /* Always detected 
> via a driver. */
>+#define IORESOURCE_SYSRAM_MERGEABLE   0x0400 /* Resource can be 
>merged. */
> 
> #define IORESOURCE_EXCLUSIVE  0x0800  /* 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 

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

2020-09-15 Thread David Hildenbrand
On 15.09.20 04:43, Wei Yang wrote:
> 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.

Yeah, sorry, I dropped the "mm" prefix on the subject of the cover
letter by mistake.

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

At least you're in the CC list below with your old mail address (I
assume you monitor that).

I'll try to use your new address in the future.


-- 
Thanks,

David / dhildenb
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


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

2020-09-14 Thread Wei Yang
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 
>Cc: Andrew Morton 
>Cc: Michal Hocko 
>Cc: Dan Williams 
>Cc: Jason Gunthorpe 
>Cc: Kees Cook 
>Cc: Ard Biesheuvel 
>Cc: Thomas Gleixner 
>Cc: "K. Y. Srinivasan" 
>Cc: Haiyang Zhang 
>Cc: Stephen Hemminger 
>Cc: Wei Liu 
>Cc: Boris Ostrovsky 
>Cc: Juergen Gross 
>Cc: Stefano Stabellini 
>Cc: Roger Pau Monné 
>Cc: Julien Grall 
>Cc: Pankaj Gupta 
>Cc: Baoquan He 
>Cc: Wei Yang 
>Signed-off-by: David Hildenbrand 
>---
> 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  0x0200 /* Always detected 
> via a driver. */
>+#define IORESOURCE_SYSRAM_MERGEABLE   0x0400 /* Resource can be 
>merged. */
> 
> #define IORESOURCE_EXCLUSIVE  0x0800  /* 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 

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

2020-09-11 Thread David Hildenbrand
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).

Reviewed-by: Pankaj Gupta 
Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Dan Williams 
Cc: Jason Gunthorpe 
Cc: Kees Cook 
Cc: Ard Biesheuvel 
Cc: Thomas Gleixner 
Cc: "K. Y. Srinivasan" 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: Wei Liu 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Cc: Roger Pau Monné 
Cc: Julien Grall 
Cc: Pankaj Gupta 
Cc: Baoquan He 
Cc: Wei Yang 
Signed-off-by: David Hildenbrand 
---
 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   0x0200 /* Always detected 
via a driver. */
+#define IORESOURCE_SYSRAM_MERGEABLE0x0400 /* Resource can be 
merged. */
 
 #define IORESOURCE_EXCLUSIVE   0x0800  /* 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 |