| Message ID | 20230711044834.72809-3-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for memmap on memory feature on ppc64 | expand |
> -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 = &mhp_altmap; > } > - mhp_altmap.free = PHYS_PFN(size); > - mhp_altmap.base_pfn = PHYS_PFN(start); > - params.altmap = &mhp_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.
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 = &mhp_altmap; >> } >> - mhp_altmap.free = PHYS_PFN(size); >> - mhp_altmap.base_pfn = PHYS_PFN(start); >> - params.altmap = &mhp_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))
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(&mem->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 = &mhp_altmap; } - mhp_altmap.free = PHYS_PFN(size); - mhp_altmap.base_pfn = PHYS_PFN(start); - params.altmap = &mhp_altmap; + /* fallback to not using altmap */ } /* call arch's memory hotadd */
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 <aneesh.kumar@linux.ibm.com> --- drivers/acpi/acpi_memhotplug.c | 3 +-- include/linux/memory_hotplug.h | 1 - mm/memory_hotplug.c | 13 ++++++------- 3 files changed, 7 insertions(+), 10 deletions(-)