Re: [PATCH v3 2/7] mm/hotplug: Allow memmap on memory hotplug request to fallback

2023-07-11 Thread Aneesh Kumar K V 
On 7/11/23 3:53 PM, David Hildenbrand wrote:
>> -bool mhp_supports_memmap_on_memory(unsigned long size)
>> +static bool mhp_supports_memmap_on_memory(unsigned long size)
>>   {
>>   unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
>>   unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
>> @@ -1339,13 +1339,12 @@ int __ref add_memory_resource(int nid, struct 
>> resource *res, mhp_t mhp_flags)
>>    * Self hosted memmap array
>>    */
>>   if (mhp_flags & MHP_MEMMAP_ON_MEMORY) {
>> -    if (!mhp_supports_memmap_on_memory(size)) {
>> -    ret = -EINVAL;
>> -    goto error;
>> +    if (mhp_supports_memmap_on_memory(size)) {
>> +    mhp_altmap.free = PHYS_PFN(size);
>> +    mhp_altmap.base_pfn = PHYS_PFN(start);
>> +    params.altmap = _altmap;
>>   }
>> -    mhp_altmap.free = PHYS_PFN(size);
>> -    mhp_altmap.base_pfn = PHYS_PFN(start);
>> -    params.altmap = _altmap;
>> +    /* fallback to not using altmap  */
>>   }
>>     /* call arch's memory hotadd */
> 
> In general, LGTM, but please extend the documentation of the parameter in 
> memory_hotplug.h, stating that this is just a hint and that the core can 
> decide to no do that.
> 

will update

modified   include/linux/memory_hotplug.h
@@ -97,6 +97,8 @@ typedef int __bitwise mhp_t;
  * To do so, we will use the beginning of the hot-added range to build
  * the page tables for the memmap array that describes the entire range.
  * Only selected architectures support it with SPARSE_VMEMMAP.
+ * This is only a hint, core kernel can decide to not do this based on
+ * different alignment checks.
  */
 #define MHP_MEMMAP_ON_MEMORY   ((__force mhp_t)BIT(1))

Re: [PATCH v3 2/7] mm/hotplug: Allow memmap on memory hotplug request to fallback

2023-07-11 Thread David Hildenbrand 

-bool mhp_supports_memmap_on_memory(unsigned long size)
+static bool mhp_supports_memmap_on_memory(unsigned long size)
  {
unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
@@ -1339,13 +1339,12 @@ int __ref add_memory_resource(int nid, struct resource 
*res, mhp_t mhp_flags)
 * Self hosted memmap array
 */
if (mhp_flags & MHP_MEMMAP_ON_MEMORY) {
-   if (!mhp_supports_memmap_on_memory(size)) {
-   ret = -EINVAL;
-   goto error;
+   if (mhp_supports_memmap_on_memory(size)) {
+   mhp_altmap.free = PHYS_PFN(size);
+   mhp_altmap.base_pfn = PHYS_PFN(start);
+   params.altmap = _altmap;
}
-   mhp_altmap.free = PHYS_PFN(size);
-   mhp_altmap.base_pfn = PHYS_PFN(start);
-   params.altmap = _altmap;
+   /* fallback to not using altmap  */
}
  
  	/* call arch's memory hotadd */


In general, LGTM, but please extend the documentation of the parameter 
in memory_hotplug.h, stating that this is just a hint and that the core 
can decide to no do that.


--
Cheers,

David / dhildenb

[PATCH v3 2/7] mm/hotplug: Allow memmap on memory hotplug request to fallback

2023-07-10 Thread Aneesh Kumar K.V 
If not supported, fallback to not using memap on memmory. This avoids
the need for callers to do the fallback.

Signed-off-by: Aneesh Kumar K.V 
---
 drivers/acpi/acpi_memhotplug.c |  3 +--
 include/linux/memory_hotplug.h |  1 -
 mm/memory_hotplug.c| 13 ++---
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 24f662d8bd39..d0c1a71007d0 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -211,8 +211,7 @@ static int acpi_memory_enable_device(struct 
acpi_memory_device *mem_device)
if (!info->length)
continue;
 
-   if (mhp_supports_memmap_on_memory(info->length))
-   mhp_flags |= MHP_MEMMAP_ON_MEMORY;
+   mhp_flags |= MHP_MEMMAP_ON_MEMORY;
result = __add_memory(mgid, info->start_addr, info->length,
  mhp_flags);
 
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 013c69753c91..96f6127f197f 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -354,7 +354,6 @@ extern struct zone *zone_for_pfn_range(int online_type, int 
nid,
 extern int arch_create_linear_mapping(int nid, u64 start, u64 size,
  struct mhp_params *params);
 void arch_remove_linear_mapping(u64 start, u64 size);
-extern bool mhp_supports_memmap_on_memory(unsigned long size);
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
 #endif /* __LINUX_MEMORY_HOTPLUG_H */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 3f231cf1b410..1b19462f4e72 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1247,7 +1247,7 @@ static int online_memory_block(struct memory_block *mem, 
void *arg)
return device_online(>dev);
 }
 
-bool mhp_supports_memmap_on_memory(unsigned long size)
+static bool mhp_supports_memmap_on_memory(unsigned long size)
 {
unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
@@ -1339,13 +1339,12 @@ int __ref add_memory_resource(int nid, struct resource 
*res, mhp_t mhp_flags)
 * Self hosted memmap array
 */
if (mhp_flags & MHP_MEMMAP_ON_MEMORY) {
-   if (!mhp_supports_memmap_on_memory(size)) {
-   ret = -EINVAL;
-   goto error;
+   if (mhp_supports_memmap_on_memory(size)) {
+   mhp_altmap.free = PHYS_PFN(size);
+   mhp_altmap.base_pfn = PHYS_PFN(start);
+   params.altmap = _altmap;
}
-   mhp_altmap.free = PHYS_PFN(size);
-   mhp_altmap.base_pfn = PHYS_PFN(start);
-   params.altmap = _altmap;
+   /* fallback to not using altmap  */
}
 
/* call arch's memory hotadd */
-- 
2.41.0