diff mbox series

[v2,5/7] mm,memory_hotplug: Enforce pageblock alignment when memmap_on_memory

Message ID 20210209133854.17399-6-osalvador@suse.de (mailing list archive)
State New, archived
Headers show
Series Allocate memmap from hotadded memory (per device) | expand

Commit Message

Oscar Salvador Feb. 9, 2021, 1:38 p.m. UTC
Many places expects us to pass a pageblock aligned range.
E.g: memmap_init_zone() needs a pageblock aligned range in order
to set the proper migrate type for it.
online_pages() needs to operate on a pageblock aligned range for
isolation purposes.

Make sure we disable the feature in case we cannot guarantee the
right alignment.

Signed-off-by: Oscar Salvador <osalvador@suse.de>
---
 mm/memory_hotplug.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

David Hildenbrand Feb. 25, 2021, 6:27 p.m. UTC | #1
On 09.02.21 14:38, Oscar Salvador wrote:
> Many places expects us to pass a pageblock aligned range.
> E.g: memmap_init_zone() needs a pageblock aligned range in order
> to set the proper migrate type for it.
> online_pages() needs to operate on a pageblock aligned range for
> isolation purposes.
> 
> Make sure we disable the feature in case we cannot guarantee the
> right alignment.
> 
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> ---
>   mm/memory_hotplug.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index d3fb036d33fd..1a4d5dd1a2c8 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -56,12 +56,16 @@ static int memmap_on_memory_show(char *buffer, const struct kernel_param *kp)
>   static __meminit int memmap_on_memory_store(const char *val,
>   					    const struct kernel_param *kp)
>   {
> +	unsigned long pageblock_size = PFN_PHYS(pageblock_nr_pages);
> +
>   	/*
>   	 * Fail silently in case we cannot enable it due to system constraints.
>   	 * User can always check whether it is enabled or not via /sys/module.
>   	 */
>   	if (!IS_ENABLED(CONFIG_ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE) ||
> -	    (PMD_SIZE % sizeof(struct page)))
> +	    (PMD_SIZE % sizeof(struct page)) ||
> +	    !(MIN_MEMORY_BLOCK_SIZE - PMD_SIZE) ||
> +	    !(MIN_MEMORY_BLOCK_SIZE - PMD_SIZE) % pageblock_size)
>   		return 0;
>   
>   	return param_set_bool(val, kp);
> 

Dito, rather squash in #1 and add a comment explaining what's happening 
there.
Oscar Salvador Feb. 26, 2021, 12:06 p.m. UTC | #2
On Thu, Feb 25, 2021 at 07:27:33PM +0100, David Hildenbrand wrote:
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index d3fb036d33fd..1a4d5dd1a2c8 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -56,12 +56,16 @@ static int memmap_on_memory_show(char *buffer, const struct kernel_param *kp)
> >   static __meminit int memmap_on_memory_store(const char *val,
> >   					    const struct kernel_param *kp)
> >   {
> > +	unsigned long pageblock_size = PFN_PHYS(pageblock_nr_pages);
> > +
> >   	/*
> >   	 * Fail silently in case we cannot enable it due to system constraints.
> >   	 * User can always check whether it is enabled or not via /sys/module.
> >   	 */
> >   	if (!IS_ENABLED(CONFIG_ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE) ||
> > -	    (PMD_SIZE % sizeof(struct page)))
> > +	    (PMD_SIZE % sizeof(struct page)) ||
> > +	    !(MIN_MEMORY_BLOCK_SIZE - PMD_SIZE) ||
> > +	    !(MIN_MEMORY_BLOCK_SIZE - PMD_SIZE) % pageblock_size)
> >   		return 0;
> >   	return param_set_bool(val, kp);
> > 
> 
> Dito, rather squash in #1 and add a comment explaining what's happening
> there.

I was not sure about putting this and the PMD aligned patch as a
standalone patch, but I thought it might ease the review.
But I have no problem in placing them in patch#1 and put some more
detail into the changelog.
diff mbox series

Patch

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index d3fb036d33fd..1a4d5dd1a2c8 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -56,12 +56,16 @@  static int memmap_on_memory_show(char *buffer, const struct kernel_param *kp)
 static __meminit int memmap_on_memory_store(const char *val,
 					    const struct kernel_param *kp)
 {
+	unsigned long pageblock_size = PFN_PHYS(pageblock_nr_pages);
+
 	/*
 	 * Fail silently in case we cannot enable it due to system constraints.
 	 * User can always check whether it is enabled or not via /sys/module.
 	 */
 	if (!IS_ENABLED(CONFIG_ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE) ||
-	    (PMD_SIZE % sizeof(struct page)))
+	    (PMD_SIZE % sizeof(struct page)) ||
+	    !(MIN_MEMORY_BLOCK_SIZE - PMD_SIZE) ||
+	    !(MIN_MEMORY_BLOCK_SIZE - PMD_SIZE) % pageblock_size)
 		return 0;
 
 	return param_set_bool(val, kp);