| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c0f51e0e-63ba-2cd2-fb07-a924a081a3fe@redhat.com>
Date: Fri, 23 Jun 2023 14:35:11 +0200
From: David Hildenbrand <david@...hat.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>,
Vishal Verma <vishal.l.verma@...el.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Oscar Salvador <osalvador@...e.de>,
Dan Williams <dan.j.williams@...el.com>,
Dave Jiang <dave.jiang@...el.com>
Cc: linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, nvdimm@...ts.linux.dev,
linux-cxl@...r.kernel.org, Huang Ying <ying.huang@...el.com>,
Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 1/3] mm/memory_hotplug: Allow an override for the
memmap_on_memory param
On 23.06.23 10:40, Aneesh Kumar K.V wrote:
> Vishal Verma <vishal.l.verma@...el.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@...nel.org>
>> Cc: Len Brown <lenb@...nel.org>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> Cc: David Hildenbrand <david@...hat.com>
>> Cc: Oscar Salvador <osalvador@...e.de>
>> Cc: Dan Williams <dan.j.williams@...el.com>
>> Cc: Dave Jiang <dave.jiang@...el.com>
>> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
>> Cc: Huang Ying <ying.huang@...el.com>
>> Signed-off-by: Vishal Verma <vishal.l.verma@...el.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@...ux.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@...ux.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.
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists