diff mbox series

[1/3] mm/memory_hotplug: Allow an override for the memmap_on_memory param

Message ID 20230613-vv-kmem_memmap-v1-1-f6de9c6af2c6@intel.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series mm: use memmap_on_memory semantics for dax/kmem | expand

Commit Message

Verma, Vishal L June 15, 2023, 10 p.m. UTC
For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the
'memmap_on_memory' module parameter was a hard requirement.

In preparation for the dax/kmem driver to use memmap_on_memory
semantics, arrange for the module parameter check to be bypassed via the
appropriate mhp_flag.

Recall that the kmem driver could contribute huge amounts of hotplugged
memory originating from special purposes devices such as CXL memory
expanders. In some cases memmap_on_memory may be the /only/ way this new
memory can be hotplugged. Hence it makes sense for kmem to have a way to
force memmap_on_memory without depending on a module param, if all the
other conditions for it are met.

The only other user of this interface is acpi/acpi_memoryhotplug.c,
which only enables the mhp_flag if an initial
mhp_supports_memmap_on_memory() test passes. Maintain the existing
behavior and semantics for this by performing the initial check from
acpi without the MHP_MEMMAP_ON_MEMORY flag, so its decision falls back
to the module parameter.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 include/linux/memory_hotplug.h |  2 +-
 drivers/acpi/acpi_memhotplug.c |  2 +-
 mm/memory_hotplug.c            | 24 ++++++++++++++++--------
 3 files changed, 18 insertions(+), 10 deletions(-)

Comments

Huang, Ying June 16, 2023, 6:35 a.m. UTC | #1
Hi, Vishal,

Thanks for your patch!

Vishal Verma <vishal.l.verma@intel.com> writes:

> For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the
> 'memmap_on_memory' module parameter was a hard requirement.
>
> In preparation for the dax/kmem driver to use memmap_on_memory
> semantics, arrange for the module parameter check to be bypassed via the
> appropriate mhp_flag.
>
> Recall that the kmem driver could contribute huge amounts of hotplugged
> memory originating from special purposes devices such as CXL memory
> expanders. In some cases memmap_on_memory may be the /only/ way this new
> memory can be hotplugged. Hence it makes sense for kmem to have a way to
> force memmap_on_memory without depending on a module param, if all the
> other conditions for it are met.
>
> The only other user of this interface is acpi/acpi_memoryhotplug.c,
> which only enables the mhp_flag if an initial
> mhp_supports_memmap_on_memory() test passes. Maintain the existing
> behavior and semantics for this by performing the initial check from
> acpi without the MHP_MEMMAP_ON_MEMORY flag, so its decision falls back
> to the module parameter.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  include/linux/memory_hotplug.h |  2 +-
>  drivers/acpi/acpi_memhotplug.c |  2 +-
>  mm/memory_hotplug.c            | 24 ++++++++++++++++--------
>  3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 9fcbf5706595..c9ddcd3cad70 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -358,7 +358,7 @@ 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);
> +extern bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags);
>  #endif /* CONFIG_MEMORY_HOTPLUG */
>  
>  #endif /* __LINUX_MEMORY_HOTPLUG_H */
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 24f662d8bd39..119d3bb49753 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -211,7 +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))
> +		if (mhp_supports_memmap_on_memory(info->length, 0))
>  			mhp_flags |= MHP_MEMMAP_ON_MEMORY;
>  		result = __add_memory(mgid, info->start_addr, info->length,
>  				      mhp_flags);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 8e0fa209d533..bb3845830922 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1283,15 +1283,21 @@ static int online_memory_block(struct memory_block *mem, void *arg)
>  	return device_online(&mem->dev);
>  }
>  
> -bool mhp_supports_memmap_on_memory(unsigned long size)
> +bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags)
>  {
>  	unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
>  	unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
>  	unsigned long remaining_size = size - vmemmap_size;
>  
>  	/*
> -	 * Besides having arch support and the feature enabled at runtime, we
> -	 * need a few more assumptions to hold true:
> +	 * The MHP_MEMMAP_ON_MEMORY flag indicates a caller that wants to force
> +	 * memmap_on_memory (if other conditions are met), regardless of the
> +	 * module parameter. drivers/dax/kmem.c is an example, where large
> +	 * amounts of hotplug memory may come from, and the only option to
> +	 * successfully online all of it is to place the memmap on this memory.
> +	 *
> +	 * Besides having arch support and the feature enabled at runtime or
> +	 * via the mhp_flag, we need a few more assumptions to hold true:
>  	 *
>  	 * a) We span a single memory block: memory onlining/offlinin;g happens
>  	 *    in memory block granularity. We don't want the vmemmap of online
> @@ -1315,10 +1321,12 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
>  	 *       altmap as an alternative source of memory, and we do not exactly
>  	 *       populate a single PMD.
>  	 */
> -	return mhp_memmap_on_memory() &&
> -	       size == memory_block_size_bytes() &&
> -	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> -	       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
> +
> +	if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) || mhp_memmap_on_memory())
> +		return size == memory_block_size_bytes() &&
> +		       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> +		       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
> +	return false;
>  }
>  
>  /*
> @@ -1375,7 +1383,7 @@ 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)) {
> +		if (!mhp_supports_memmap_on_memory(size, mhp_flags)) {
>  			ret = -EINVAL;
>  			goto error;
>  		}

It appears that we need to deal with the hot-remove path too.
try_remove_memory() will call mhp_memmap_on_memory() and only work with
MHP_MEMMAP_ON_MEMORY properly if it returns true.

Best Regards,
Huang, Ying
David Hildenbrand June 16, 2023, 7:46 a.m. UTC | #2
On 16.06.23 00:00, Vishal Verma wrote:
> For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the
> 'memmap_on_memory' module parameter was a hard requirement.
> 
> In preparation for the dax/kmem driver to use memmap_on_memory
> semantics, arrange for the module parameter check to be bypassed via the
> appropriate mhp_flag.
> 
> Recall that the kmem driver could contribute huge amounts of hotplugged
> memory originating from special purposes devices such as CXL memory
> expanders. In some cases memmap_on_memory may be the /only/ way this new
> memory can be hotplugged. Hence it makes sense for kmem to have a way to
> force memmap_on_memory without depending on a module param, if all the
> other conditions for it are met.

Just let the admin configure it. After all, an admin is involved in 
configuring the dax/kmem device to begin with. If add_memory() fails you 
could give a useful hint to the admin.
Jonathan Cameron June 22, 2023, 1:37 p.m. UTC | #3
On Fri, 16 Jun 2023 09:46:59 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 16.06.23 00:00, Vishal Verma wrote:
> > For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the
> > 'memmap_on_memory' module parameter was a hard requirement.
> > 
> > In preparation for the dax/kmem driver to use memmap_on_memory
> > semantics, arrange for the module parameter check to be bypassed via the
> > appropriate mhp_flag.
> > 
> > Recall that the kmem driver could contribute huge amounts of hotplugged
> > memory originating from special purposes devices such as CXL memory
> > expanders. In some cases memmap_on_memory may be the /only/ way this new
> > memory can be hotplugged. Hence it makes sense for kmem to have a way to
> > force memmap_on_memory without depending on a module param, if all the
> > other conditions for it are met.  
> 
> Just let the admin configure it. After all, an admin is involved in 
> configuring the dax/kmem device to begin with. If add_memory() fails you 
> could give a useful hint to the admin.
> 

Agreed. If it were just the default then fine, but making it the only option
limits admin choices.

Jonathan
Aneesh Kumar K.V June 23, 2023, 8:40 a.m. UTC | #4
Vishal Verma <vishal.l.verma@intel.com> writes:

> For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the
> 'memmap_on_memory' module parameter was a hard requirement.
>
> In preparation for the dax/kmem driver to use memmap_on_memory
> semantics, arrange for the module parameter check to be bypassed via the
> appropriate mhp_flag.
>
> Recall that the kmem driver could contribute huge amounts of hotplugged
> memory originating from special purposes devices such as CXL memory
> expanders. In some cases memmap_on_memory may be the /only/ way this new
> memory can be hotplugged. Hence it makes sense for kmem to have a way to
> force memmap_on_memory without depending on a module param, if all the
> other conditions for it are met.
>
> The only other user of this interface is acpi/acpi_memoryhotplug.c,
> which only enables the mhp_flag if an initial
> mhp_supports_memmap_on_memory() test passes. Maintain the existing
> behavior and semantics for this by performing the initial check from
> acpi without the MHP_MEMMAP_ON_MEMORY flag, so its decision falls back
> to the module parameter.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  include/linux/memory_hotplug.h |  2 +-
>  drivers/acpi/acpi_memhotplug.c |  2 +-
>  mm/memory_hotplug.c            | 24 ++++++++++++++++--------
>  3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 9fcbf5706595..c9ddcd3cad70 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -358,7 +358,7 @@ 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);
> +extern bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags);
>  #endif /* CONFIG_MEMORY_HOTPLUG */
>  
>  #endif /* __LINUX_MEMORY_HOTPLUG_H */
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 24f662d8bd39..119d3bb49753 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -211,7 +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))
> +		if (mhp_supports_memmap_on_memory(info->length, 0))
>  			mhp_flags |= MHP_MEMMAP_ON_MEMORY;
>  		result = __add_memory(mgid, info->start_addr, info->length,
>  				      mhp_flags);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 8e0fa209d533..bb3845830922 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1283,15 +1283,21 @@ static int online_memory_block(struct memory_block *mem, void *arg)
>  	return device_online(&mem->dev);
>  }
>  
> -bool mhp_supports_memmap_on_memory(unsigned long size)
> +bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags)
>  {
>  	unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
>  	unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
>  	unsigned long remaining_size = size - vmemmap_size;
>  
>  	/*
> -	 * Besides having arch support and the feature enabled at runtime, we
> -	 * need a few more assumptions to hold true:
> +	 * The MHP_MEMMAP_ON_MEMORY flag indicates a caller that wants to force
> +	 * memmap_on_memory (if other conditions are met), regardless of the
> +	 * module parameter. drivers/dax/kmem.c is an example, where large
> +	 * amounts of hotplug memory may come from, and the only option to
> +	 * successfully online all of it is to place the memmap on this memory.
> +	 *
> +	 * Besides having arch support and the feature enabled at runtime or
> +	 * via the mhp_flag, we need a few more assumptions to hold true:
>  	 *
>  	 * a) We span a single memory block: memory onlining/offlinin;g happens
>  	 *    in memory block granularity. We don't want the vmemmap of online
> @@ -1315,10 +1321,12 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
>  	 *       altmap as an alternative source of memory, and we do not exactly
>  	 *       populate a single PMD.
>  	 */
> -	return mhp_memmap_on_memory() &&
> -	       size == memory_block_size_bytes() &&
> -	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> -	       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
> +
> +	if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) || mhp_memmap_on_memory())
> +		return size == memory_block_size_bytes() &&
> +		       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> +		       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
> +	return false;
>  }
>  
>  /*
> @@ -1375,7 +1383,7 @@ 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)) {
> +		if (!mhp_supports_memmap_on_memory(size, mhp_flags)) {
>  			ret = -EINVAL;
>  			goto error;
>  		}
>

I was working on enabling memmap_on_memory for ppc64 and came across
this. Can we do this as below?

commit d55eddf71032cad3e057d8fa4c61ad2b8e05aaa8
Author: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Date:   Fri Jun 23 12:11:31 2023 +0530

    mm/hotplug: Allow to force enable memmap on memory
    
    In some cases like DAX kmem, we want to enable memmap on memory
    even if we have memory_hotplug.memmap_on_memory disabled. This
    patch enables such usage. DAX kmem can use MHP_MEMMAP_ON_MEMORY
    directly to request for memmap on memory.
    
    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 24f662d8bd39..4d0096fc4cc2 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 |= get_memmap_on_memory_flags();
 		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 9fcbf5706595..f7f534084393 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -359,6 +359,13 @@ 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);
+bool mhp_memmap_on_memory(void);
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
+static inline unsigned long get_memmap_on_memory_flags(void)
+{
+	if (mhp_memmap_on_memory())
+		return MHP_MEMMAP_ON_MEMORY;
+	return 0;
+}
 #endif /* __LINUX_MEMORY_HOTPLUG_H */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index eb18d1ac7e84..85d29909fd89 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -50,7 +50,7 @@ static bool memmap_on_memory __ro_after_init;
 module_param(memmap_on_memory, bool, 0444);
 MODULE_PARM_DESC(memmap_on_memory, "Enable memmap on memory for memory hotplug");
 
-static inline bool mhp_memmap_on_memory(void)
+bool mhp_memmap_on_memory(void)
 {
 	return memmap_on_memory;
 }
@@ -1316,10 +1316,9 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
 	 *       altmap as an alternative source of memory, and we do not exactly
 	 *       populate a single PMD.
 	 */
-	return mhp_memmap_on_memory() &&
-	       size == memory_block_size_bytes() &&
-	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
-	       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
+	return size == memory_block_size_bytes() &&
+		IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
+		IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
 }
 
 /*
David Hildenbrand June 23, 2023, 12:35 p.m. UTC | #5
On 23.06.23 10:40, Aneesh Kumar K.V wrote:
> Vishal Verma <vishal.l.verma@intel.com> writes:
> 
>> For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the
>> 'memmap_on_memory' module parameter was a hard requirement.
>>
>> In preparation for the dax/kmem driver to use memmap_on_memory
>> semantics, arrange for the module parameter check to be bypassed via the
>> appropriate mhp_flag.
>>
>> Recall that the kmem driver could contribute huge amounts of hotplugged
>> memory originating from special purposes devices such as CXL memory
>> expanders. In some cases memmap_on_memory may be the /only/ way this new
>> memory can be hotplugged. Hence it makes sense for kmem to have a way to
>> force memmap_on_memory without depending on a module param, if all the
>> other conditions for it are met.
>>
>> The only other user of this interface is acpi/acpi_memoryhotplug.c,
>> which only enables the mhp_flag if an initial
>> mhp_supports_memmap_on_memory() test passes. Maintain the existing
>> behavior and semantics for this by performing the initial check from
>> acpi without the MHP_MEMMAP_ON_MEMORY flag, so its decision falls back
>> to the module parameter.
>>
>> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Dave Jiang <dave.jiang@intel.com>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Huang Ying <ying.huang@intel.com>
>> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
>> ---
>>   include/linux/memory_hotplug.h |  2 +-
>>   drivers/acpi/acpi_memhotplug.c |  2 +-
>>   mm/memory_hotplug.c            | 24 ++++++++++++++++--------
>>   3 files changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>> index 9fcbf5706595..c9ddcd3cad70 100644
>> --- a/include/linux/memory_hotplug.h
>> +++ b/include/linux/memory_hotplug.h
>> @@ -358,7 +358,7 @@ 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);
>> +extern bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags);
>>   #endif /* CONFIG_MEMORY_HOTPLUG */
>>   
>>   #endif /* __LINUX_MEMORY_HOTPLUG_H */
>> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
>> index 24f662d8bd39..119d3bb49753 100644
>> --- a/drivers/acpi/acpi_memhotplug.c
>> +++ b/drivers/acpi/acpi_memhotplug.c
>> @@ -211,7 +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))
>> +		if (mhp_supports_memmap_on_memory(info->length, 0))
>>   			mhp_flags |= MHP_MEMMAP_ON_MEMORY;
>>   		result = __add_memory(mgid, info->start_addr, info->length,
>>   				      mhp_flags);
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 8e0fa209d533..bb3845830922 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1283,15 +1283,21 @@ static int online_memory_block(struct memory_block *mem, void *arg)
>>   	return device_online(&mem->dev);
>>   }
>>   
>> -bool mhp_supports_memmap_on_memory(unsigned long size)
>> +bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags)
>>   {
>>   	unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
>>   	unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
>>   	unsigned long remaining_size = size - vmemmap_size;
>>   
>>   	/*
>> -	 * Besides having arch support and the feature enabled at runtime, we
>> -	 * need a few more assumptions to hold true:
>> +	 * The MHP_MEMMAP_ON_MEMORY flag indicates a caller that wants to force
>> +	 * memmap_on_memory (if other conditions are met), regardless of the
>> +	 * module parameter. drivers/dax/kmem.c is an example, where large
>> +	 * amounts of hotplug memory may come from, and the only option to
>> +	 * successfully online all of it is to place the memmap on this memory.
>> +	 *
>> +	 * Besides having arch support and the feature enabled at runtime or
>> +	 * via the mhp_flag, we need a few more assumptions to hold true:
>>   	 *
>>   	 * a) We span a single memory block: memory onlining/offlinin;g happens
>>   	 *    in memory block granularity. We don't want the vmemmap of online
>> @@ -1315,10 +1321,12 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
>>   	 *       altmap as an alternative source of memory, and we do not exactly
>>   	 *       populate a single PMD.
>>   	 */
>> -	return mhp_memmap_on_memory() &&
>> -	       size == memory_block_size_bytes() &&
>> -	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
>> -	       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
>> +
>> +	if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) || mhp_memmap_on_memory())
>> +		return size == memory_block_size_bytes() &&
>> +		       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
>> +		       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
>> +	return false;
>>   }
>>   
>>   /*
>> @@ -1375,7 +1383,7 @@ 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)) {
>> +		if (!mhp_supports_memmap_on_memory(size, mhp_flags)) {
>>   			ret = -EINVAL;
>>   			goto error;
>>   		}
>>
> 
> I was working on enabling memmap_on_memory for ppc64 and came across
> this. Can we do this as below?
> 
> commit d55eddf71032cad3e057d8fa4c61ad2b8e05aaa8
> Author: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Date:   Fri Jun 23 12:11:31 2023 +0530
> 
>      mm/hotplug: Allow to force enable memmap on memory
>      
>      In some cases like DAX kmem, we want to enable memmap on memory
>      even if we have memory_hotplug.memmap_on_memory disabled. This
>      patch enables such usage. DAX kmem can use MHP_MEMMAP_ON_MEMORY
>      directly to request for memmap on memory.
>      
>      Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> 
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 24f662d8bd39..4d0096fc4cc2 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 |= get_memmap_on_memory_flags();
>   		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 9fcbf5706595..f7f534084393 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -359,6 +359,13 @@ 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);
> +bool mhp_memmap_on_memory(void);
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>   
> +static inline unsigned long get_memmap_on_memory_flags(void)
> +{
> +	if (mhp_memmap_on_memory())
> +		return MHP_MEMMAP_ON_MEMORY;
> +	return 0;
> +}
>   #endif /* __LINUX_MEMORY_HOTPLUG_H */
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index eb18d1ac7e84..85d29909fd89 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -50,7 +50,7 @@ static bool memmap_on_memory __ro_after_init;
>   module_param(memmap_on_memory, bool, 0444);
>   MODULE_PARM_DESC(memmap_on_memory, "Enable memmap on memory for memory hotplug");
>   
> -static inline bool mhp_memmap_on_memory(void)
> +bool mhp_memmap_on_memory(void)
>   {
>   	return memmap_on_memory;
>   }
> @@ -1316,10 +1316,9 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
>   	 *       altmap as an alternative source of memory, and we do not exactly
>   	 *       populate a single PMD.
>   	 */
> -	return mhp_memmap_on_memory() &&
> -	       size == memory_block_size_bytes() &&
> -	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> -	       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
> +	return size == memory_block_size_bytes() &&
> +		IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> +		IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
>   }
>   
>   /*
> 

Any forced enablement will have to cleanly handle try_remove_memory() as 
well.

memmap_on_memory needs a doc update then, and even the "forced 
enablement" has to still be explicitly enabled by the admin.

... but I'm still not convinced that memmap_on_memory should be 
overridden. Needs good justification.
diff mbox series

Patch

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 9fcbf5706595..c9ddcd3cad70 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -358,7 +358,7 @@  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);
+extern bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags);
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
 #endif /* __LINUX_MEMORY_HOTPLUG_H */
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 24f662d8bd39..119d3bb49753 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -211,7 +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))
+		if (mhp_supports_memmap_on_memory(info->length, 0))
 			mhp_flags |= MHP_MEMMAP_ON_MEMORY;
 		result = __add_memory(mgid, info->start_addr, info->length,
 				      mhp_flags);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 8e0fa209d533..bb3845830922 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1283,15 +1283,21 @@  static int online_memory_block(struct memory_block *mem, void *arg)
 	return device_online(&mem->dev);
 }
 
-bool mhp_supports_memmap_on_memory(unsigned long size)
+bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags)
 {
 	unsigned long nr_vmemmap_pages = size / PAGE_SIZE;
 	unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page);
 	unsigned long remaining_size = size - vmemmap_size;
 
 	/*
-	 * Besides having arch support and the feature enabled at runtime, we
-	 * need a few more assumptions to hold true:
+	 * The MHP_MEMMAP_ON_MEMORY flag indicates a caller that wants to force
+	 * memmap_on_memory (if other conditions are met), regardless of the
+	 * module parameter. drivers/dax/kmem.c is an example, where large
+	 * amounts of hotplug memory may come from, and the only option to
+	 * successfully online all of it is to place the memmap on this memory.
+	 *
+	 * Besides having arch support and the feature enabled at runtime or
+	 * via the mhp_flag, we need a few more assumptions to hold true:
 	 *
 	 * a) We span a single memory block: memory onlining/offlinin;g happens
 	 *    in memory block granularity. We don't want the vmemmap of online
@@ -1315,10 +1321,12 @@  bool mhp_supports_memmap_on_memory(unsigned long size)
 	 *       altmap as an alternative source of memory, and we do not exactly
 	 *       populate a single PMD.
 	 */
-	return mhp_memmap_on_memory() &&
-	       size == memory_block_size_bytes() &&
-	       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
-	       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
+
+	if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) || mhp_memmap_on_memory())
+		return size == memory_block_size_bytes() &&
+		       IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
+		       IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
+	return false;
 }
 
 /*
@@ -1375,7 +1383,7 @@  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)) {
+		if (!mhp_supports_memmap_on_memory(size, mhp_flags)) {
 			ret = -EINVAL;
 			goto error;
 		}