Re: [PATCH v2 3/5] mm/hotplug: Simplify the handling of MHP_MEMMAP_ON_MEMORY flag

[David Hildenbrand <david@redhat.com>]

On 06.07.23 10:50, Aneesh Kumar K.V wrote:
> Instead of checking for memmap on memory feature enablement within the
> functions checking for alignment, use the kernel parameter to control the
> memory hotplug flags. The generic kernel now enables memmap on memory
> feature if the hotplug flag request for the same.
> 
> The ACPI code now can pass the flag unconditionally because the kernel will
> fallback to not using the feature if the alignment rules are not met.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>   drivers/acpi/acpi_memhotplug.c |  3 +--
>   include/linux/memory_hotplug.h | 14 ++++++++++++++
>   mm/memory_hotplug.c            | 35 +++++++++++-----------------------
>   3 files changed, 26 insertions(+), 26 deletions(-)
> 
> 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 a769f44b8368..af7017122506 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -358,4 +358,18 @@ bool mhp_supports_memmap_on_memory(unsigned long size);
>   bool __mhp_supports_memmap_on_memory(unsigned long size);
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>   
> +#ifdef CONFIG_MHP_MEMMAP_ON_MEMORY
> +extern bool memmap_on_memory;
> +static inline unsigned long get_memmap_on_memory_flags(void)
> +{
> +	if (memmap_on_memory)
> +		return MHP_MEMMAP_ON_MEMORY;
> +	return 0;
> +}
> +#else
> +static inline unsigned long get_memmap_on_memory_flags(void)
> +{
> +	return 0;
> +}
> +#endif

That's kind-of ugly TBH.


Why do we need this change?

-- 
Cheers,

David / dhildenb