| Message ID | 20230706085041.826340-4-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for memmap on memory feature on ppc64 | expand |
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?
On 7/6/23 2:54 PM, David Hildenbrand wrote: > 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? > I was trying to avoid rest of the kernel doing if (mhp_supports_memmap_on_memory(info->length)) mhp_flags |= MHP_MEMMAP_ON_MEMORY; I was also following pattern similar to static inline gfp_t alloc_hugepage_khugepaged_gfpmask(void) { return khugepaged_defrag() ? GFP_TRANSHUGE : GFP_TRANSHUGE_LIGHT; } Overall goal of the patch is to handle the fallback of not using altmap/memmap in memory inside add_memory_resource and the callsites only express the desire to use memmap on memory if possible/enabled. -aneesh
On 06.07.23 12:04, Aneesh Kumar K V wrote: > On 7/6/23 2:54 PM, David Hildenbrand wrote: >> 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? >> > > I was trying to avoid rest of the kernel doing > > if (mhp_supports_memmap_on_memory(info->length)) > mhp_flags |= MHP_MEMMAP_ON_MEMORY; It would look much cleaner if you would simply have: mhp_flags |= MHP_MEMMAP_ON_MEMORY; result = __add_memory(mgid, info->start_addr, info->length, mhp_flags); And modify the semantics of MHP_MEMMAP_ON_MEMORY to mean "allocate the memmap from hotplugged memory if supported and enabled globally". Then, we can simply ignore the flag in __add_memory() if either the global toggle is off or if it's not supported for the range / by the arch. Maybe, in the future we want more fine-grained control (as asked by dax/kmem) and maybe not have the global toggle. But for now, that should be good enough I guess.
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 #endif /* __LINUX_MEMORY_HOTPLUG_H */ diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 423f96dd5481..a1542b8f64e6 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -45,19 +45,9 @@ /* * memory_hotplug.memmap_on_memory parameter */ -static bool memmap_on_memory __ro_after_init; +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) -{ - return memmap_on_memory; -} -#else -static inline bool mhp_memmap_on_memory(void) -{ - return false; -} #endif enum { @@ -1280,10 +1270,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)); } bool __weak mhp_supports_memmap_on_memory(unsigned long size) @@ -2082,15 +2071,13 @@ static int __ref try_remove_memory(u64 start, u64 size) * the same granularity it was added - a single memory block. */ - if (mhp_memmap_on_memory()) { - ret = walk_memory_blocks(start, size, &altmap, get_vmemmap_altmap_cb); - if (ret) { - if (size != memory_block_size_bytes()) { - pr_warn("Refuse to remove %#llx - %#llx," - "wrong granularity\n", - start, start + size); - return -EINVAL; - } + ret = walk_memory_blocks(start, size, &altmap, get_vmemmap_altmap_cb); + if (ret) { + if (size != memory_block_size_bytes()) { + pr_warn("Refuse to remove %#llx - %#llx," + "wrong granularity\n", + start, start + size); + return -EINVAL; } }
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(-)