Re: [PATCH v3 6/7] dax/kmem: Always enroll hotplugged memory for memmap_on_memory

2023-07-11 Thread David Hildenbrand

On 11.07.23 06:48, Aneesh Kumar K.V wrote:

From: Vishal Verma 

With DAX memory regions originating from CXL memory expanders or
NVDIMMs, the kmem driver may be hot-adding huge amounts of system memory
on a system without enough 'regular' main memory to support the memmap
for it. To avoid this, ensure that all kmem managed hotplugged memory is
added with the MHP_MEMMAP_ON_MEMORY flag to place the memmap on the
new memory region being hot added.

To do this, call add_memory() in chunks of memory_block_size_bytes() as
that is a requirement for memmap_on_memory.

Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Andrew Morton 
Cc: David Hildenbrand 
Cc: Oscar Salvador 
Cc: Dan Williams 
Cc: Dave Jiang 
Cc: Dave Hansen 
Cc: Huang Ying 
Signed-off-by: Vishal Verma 
Signed-off-by: Aneesh Kumar K.V 
---



See my feedback as reply to the original patch.

--
Cheers,

David / dhildenb



[PATCH v3 6/7] dax/kmem: Always enroll hotplugged memory for memmap_on_memory

2023-07-10 Thread Aneesh Kumar K.V
From: Vishal Verma 

With DAX memory regions originating from CXL memory expanders or
NVDIMMs, the kmem driver may be hot-adding huge amounts of system memory
on a system without enough 'regular' main memory to support the memmap
for it. To avoid this, ensure that all kmem managed hotplugged memory is
added with the MHP_MEMMAP_ON_MEMORY flag to place the memmap on the
new memory region being hot added.

To do this, call add_memory() in chunks of memory_block_size_bytes() as
that is a requirement for memmap_on_memory.

Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Andrew Morton 
Cc: David Hildenbrand 
Cc: Oscar Salvador 
Cc: Dan Williams 
Cc: Dave Jiang 
Cc: Dave Hansen 
Cc: Huang Ying 
Signed-off-by: Vishal Verma 
Signed-off-by: Aneesh Kumar K.V 
---
 drivers/dax/kmem.c | 81 +-
 1 file changed, 59 insertions(+), 22 deletions(-)

diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index 898ca9505754..840bf7b40a44 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "dax-private.h"
 #include "bus.h"
 
@@ -105,6 +106,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
data->mgid = rc;
 
for (i = 0; i < dev_dax->nr_range; i++) {
+   u64 cur_start, cur_len, remaining;
struct resource *res;
struct range range;
 
@@ -137,21 +139,42 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
res->flags = IORESOURCE_SYSTEM_RAM;
 
/*
-* Ensure that future kexec'd kernels will not treat
-* this as RAM automatically.
+* Add memory in chunks of memory_block_size_bytes() so that
+* it is considered for MHP_MEMMAP_ON_MEMORY
+* @range has already been aligned to memory_block_size_bytes(),
+* so the following loop will always break it down cleanly.
 */
-   rc = add_memory_driver_managed(data->mgid, range.start,
-   range_len(), kmem_name, MHP_NID_IS_MGID);
-
-   if (rc) {
-   dev_warn(dev, "mapping%d: %#llx-%#llx memory add 
failed\n",
-   i, range.start, range.end);
-   remove_resource(res);
-   kfree(res);
-   data->res[i] = NULL;
-   if (mapped)
-   continue;
-   goto err_request_mem;
+   cur_start = range.start;
+   cur_len = memory_block_size_bytes();
+   remaining = range_len();
+   while (remaining) {
+   /*
+* If alignment rules are not satisified we will
+* fallback normal memmap allocation.
+*/
+   mhp_t mhp_flags = MHP_NID_IS_MGID | 
MHP_MEMMAP_ON_MEMORY;
+   /*
+* Ensure that future kexec'd kernels will not treat
+* this as RAM automatically.
+*/
+   rc = add_memory_driver_managed(data->mgid, cur_start,
+  cur_len, kmem_name,
+  mhp_flags);
+
+   if (rc) {
+   dev_warn(dev,
+"mapping%d: %#llx-%#llx memory add 
failed\n",
+i, cur_start, cur_start + cur_len - 1);
+   remove_resource(res);
+   kfree(res);
+   data->res[i] = NULL;
+   if (mapped)
+   continue;
+   goto err_request_mem;
+   }
+
+   cur_start += cur_len;
+   remaining -= cur_len;
}
mapped++;
}
@@ -186,25 +209,39 @@ static void dev_dax_kmem_remove(struct dev_dax *dev_dax)
 * unbind will succeed even if we return failure.
 */
for (i = 0; i < dev_dax->nr_range; i++) {
+
+   u64 cur_start, cur_len, remaining;
struct range range;
+   bool resource_remove;
int rc;
 
rc = dax_kmem_range(dev_dax, i, );
if (rc)
continue;
 
-   rc = remove_memory(range.start, range_len());
-   if (rc == 0) {
+   resource_remove = true;
+   cur_start = range.start;
+   cur_len = memory_block_size_bytes();
+   remaining = range_len();
+   while (remaining) {
+
+   rc = remove_memory(cur_start, cur_len);
+