[PATCH v5 0/5] mm: clean up names and parameters of memmap_init_xxxx functions

[PATCH v5 0/5] mm: clean up names and parameters of memmap_init_xxxx functions

[Baoquan He]

This patchset is correcting inappropriate function names of
memmap_init_xxx, and simplify parameters of functions in the code flow.
And also fix a prototype warning reported by lkp.

This is based on the latest next/master.

V4 can be found here:
https://lore.kernel.org/linux-mm/20210120045213.6571-1-bhe@redhat.com/

v4->v5:
 - Add patch 1 into series which fixes a prototype warning from kernel
   test robot. Then rebase the v4 patches on top of it.

v3->v4:
 - Rebased patch 1, 2 on top of Mike's below new patch.
   [PATCH v3 0/2] mm: fix initialization of struct page for holes in  memory layout

 - Move the code of renaming function parameter 'range_start_pfn' and local
   variable 'range_end_pfn' of memmap_init() from patch 1 to patch 2
   according to David's comment.

 - Use the reverse Christmas tree style to reorder the local variables
   in memmap_init_zone() in patch 2 accodrding to David's comment.

Baoquan He (5):
  mm: fix prototype warning from kernel test robot
  mm: rename memmap_init() and memmap_init_zone()
  mm: simplify parater of function memmap_init_zone()
  mm: simplify parameter of setup_usemap()
  mm: remove unneeded local variable in free_area_init_core

 arch/ia64/include/asm/pgtable.h |  6 -----
 arch/ia64/mm/init.c             | 14 +++++-----
 include/linux/mm.h              |  3 ++-
 mm/memory_hotplug.c             |  2 +-
 mm/page_alloc.c                 | 46 ++++++++++++++-------------------
 5 files changed, 31 insertions(+), 40 deletions(-)

-- 
2.17.2

[PATCH v5 1/5] mm: fix prototype warning from kernel test robot

[Baoquan He]

Kernel test robot calling make with 'W=1' is triggering warning like
below for memmap_init_zone() function.

mm/page_alloc.c:6259:23: warning: no previous prototype for 'memmap_init_zone' [-Wmissing-prototypes]
 6259 | void __meminit __weak memmap_init_zone(unsigned long size, int nid,
      |                       ^~~~~~~~~~~~~~~~

Fix it by adding the function declaration in include/linux/mm.h.
Since memmap_init_zone() has a generic version with '__weak',
the declaratoin in ia64 header file can be simply removed.

Signed-off-by: Baoquan He <bhe@redhat.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 arch/ia64/include/asm/pgtable.h | 6 ------
 include/linux/mm.h              | 2 ++
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/ia64/include/asm/pgtable.h b/arch/ia64/include/asm/pgtable.h
index 779b6972aa84..9b4efe89e62d 100644
--- a/arch/ia64/include/asm/pgtable.h
+++ b/arch/ia64/include/asm/pgtable.h
@@ -517,12 +517,6 @@ extern struct page *zero_page_memmap_ptr;
 	__changed;							\
 })
 #endif
-
-#  ifdef CONFIG_VIRTUAL_MEM_MAP
-  /* arch mem_map init routine is needed due to holes in a virtual mem_map */
-    extern void memmap_init (unsigned long size, int nid, unsigned long zone,
-			     unsigned long start_pfn);
-#  endif /* CONFIG_VIRTUAL_MEM_MAP */
 # endif /* !__ASSEMBLY__ */
 
 /*
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3dac7bc667ee..3d82b4f7cabc 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2401,6 +2401,8 @@ extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_zone(unsigned long, int, unsigned long,
 		unsigned long, unsigned long, enum meminit_context,
 		struct vmem_altmap *, int migratetype);
+extern void memmap_init(unsigned long size, int nid,
+		unsigned long zone, unsigned long range_start_pfn);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
 extern void mem_init(void);
-- 
2.17.2

Re: [PATCH v5 1/5] mm: fix prototype warning from kernel test robot

[David Hildenbrand]

On 22.01.21 14:59, Baoquan He wrote:
> Kernel test robot calling make with 'W=1' is triggering warning like
> below for memmap_init_zone() function.
> 
> mm/page_alloc.c:6259:23: warning: no previous prototype for 'memmap_init_zone' [-Wmissing-prototypes]
>  6259 | void __meminit __weak memmap_init_zone(unsigned long size, int nid,
>       |                       ^~~~~~~~~~~~~~~~
> 
> Fix it by adding the function declaration in include/linux/mm.h.
> Since memmap_init_zone() has a generic version with '__weak',
> the declaratoin in ia64 header file can be simply removed.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>  arch/ia64/include/asm/pgtable.h | 6 ------
>  include/linux/mm.h              | 2 ++
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/ia64/include/asm/pgtable.h b/arch/ia64/include/asm/pgtable.h
> index 779b6972aa84..9b4efe89e62d 100644
> --- a/arch/ia64/include/asm/pgtable.h
> +++ b/arch/ia64/include/asm/pgtable.h
> @@ -517,12 +517,6 @@ extern struct page *zero_page_memmap_ptr;
>  	__changed;							\
>  })
>  #endif
> -
> -#  ifdef CONFIG_VIRTUAL_MEM_MAP
> -  /* arch mem_map init routine is needed due to holes in a virtual mem_map */
> -    extern void memmap_init (unsigned long size, int nid, unsigned long zone,
> -			     unsigned long start_pfn);
> -#  endif /* CONFIG_VIRTUAL_MEM_MAP */
>  # endif /* !__ASSEMBLY__ */
>  
>  /*
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 3dac7bc667ee..3d82b4f7cabc 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2401,6 +2401,8 @@ extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long,
>  		unsigned long, unsigned long, enum meminit_context,
>  		struct vmem_altmap *, int migratetype);
> +extern void memmap_init(unsigned long size, int nid,
> +		unsigned long zone, unsigned long range_start_pfn);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

[PATCH v5 2/5] mm: rename memmap_init() and memmap_init_zone()

[Baoquan He]

The current memmap_init_zone() only handles memory region inside one zone,
actually memmap_init() does the memmap init of one zone. So rename both of
them accordingly.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/ia64/mm/init.c | 6 +++---
 include/linux/mm.h  | 4 ++--
 mm/memory_hotplug.c | 2 +-
 mm/page_alloc.c     | 8 ++++----
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index e76386a3479e..c8e68e92beb3 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -535,18 +535,18 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
 		    / sizeof(struct page));
 
 	if (map_start < map_end)
-		memmap_init_zone((unsigned long)(map_end - map_start),
+		memmap_init_range((unsigned long)(map_end - map_start),
 				 args->nid, args->zone, page_to_pfn(map_start), page_to_pfn(map_end),
 				 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
 	return 0;
 }
 
 void __meminit
-memmap_init (unsigned long size, int nid, unsigned long zone,
+memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 	     unsigned long start_pfn)
 {
 	if (!vmem_map) {
-		memmap_init_zone(size, nid, zone, start_pfn, start_pfn + size,
+		memmap_init_range(size, nid, zone, start_pfn, start_pfn + size,
 				 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
 	} else {
 		struct page *start;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3d82b4f7cabc..2395dc212221 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2398,10 +2398,10 @@ extern int __meminit early_pfn_to_nid(unsigned long pfn);
 #endif
 
 extern void set_dma_reserve(unsigned long new_dma_reserve);
-extern void memmap_init_zone(unsigned long, int, unsigned long,
+extern void memmap_init_range(unsigned long, int, unsigned long,
 		unsigned long, unsigned long, enum meminit_context,
 		struct vmem_altmap *, int migratetype);
-extern void memmap_init(unsigned long size, int nid,
+extern void memmap_init_zone(unsigned long size, int nid,
 		unsigned long zone, unsigned long range_start_pfn);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index f9d57b9be8c7..ddcb1cd24c60 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -713,7 +713,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	 * expects the zone spans the pfn range. All the pages in the range
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
-	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn, 0,
+	memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
 			 MEMINIT_HOTPLUG, altmap, migratetype);
 
 	set_zone_contiguous(zone);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 44ec5594798d..42a1d2d2a87d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6117,7 +6117,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
  * (usually MIGRATE_MOVABLE). Besides setting the migratetype, no related
  * zone stats (e.g., nr_isolate_pageblock) are touched.
  */
-void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
+void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone,
 		unsigned long start_pfn, unsigned long zone_end_pfn,
 		enum meminit_context context,
 		struct vmem_altmap *altmap, int migratetype)
@@ -6254,7 +6254,7 @@ static void __meminit zone_init_free_lists(struct zone *zone)
 	}
 }
 
-void __meminit __weak memmap_init(unsigned long size, int nid,
+void __meminit __weak memmap_init_zone(unsigned long size, int nid,
 				  unsigned long zone,
 				  unsigned long range_start_pfn)
 {
@@ -6268,7 +6268,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
 
 		if (end_pfn > start_pfn) {
 			size = end_pfn - start_pfn;
-			memmap_init_zone(size, nid, zone, start_pfn, range_end_pfn,
+			memmap_init_range(size, nid, zone, start_pfn, range_end_pfn,
 					 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
 		}
 	}
@@ -6978,7 +6978,7 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
 		set_pageblock_order();
 		setup_usemap(pgdat, zone, zone_start_pfn, size);
 		init_currently_empty_zone(zone, zone_start_pfn, size);
-		memmap_init(size, nid, j, zone_start_pfn);
+		memmap_init_zone(size, nid, j, zone_start_pfn);
 	}
 }
 
-- 
2.17.2

Re: [PATCH v5 2/5] mm: rename memmap_init() and memmap_init_zone()

[David Hildenbrand]

On 22.01.21 14:59, Baoquan He wrote:
> The current memmap_init_zone() only handles memory region inside one zone,
> actually memmap_init() does the memmap init of one zone. So rename both of
> them accordingly.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  arch/ia64/mm/init.c | 6 +++---
>  include/linux/mm.h  | 4 ++--
>  mm/memory_hotplug.c | 2 +-
>  mm/page_alloc.c     | 8 ++++----
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index e76386a3479e..c8e68e92beb3 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -535,18 +535,18 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  		    / sizeof(struct page));
>  
>  	if (map_start < map_end)
> -		memmap_init_zone((unsigned long)(map_end - map_start),
> +		memmap_init_range((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start), page_to_pfn(map_end),
>  				 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
>  	return 0;
>  }
>  
>  void __meminit
> -memmap_init (unsigned long size, int nid, unsigned long zone,
> +memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, start_pfn + size,
> +		memmap_init_range(size, nid, zone, start_pfn, start_pfn + size,
>  				 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
>  	} else {
>  		struct page *start;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 3d82b4f7cabc..2395dc212221 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2398,10 +2398,10 @@ extern int __meminit early_pfn_to_nid(unsigned long pfn);
>  #endif
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
> -extern void memmap_init_zone(unsigned long, int, unsigned long,
> +extern void memmap_init_range(unsigned long, int, unsigned long,
>  		unsigned long, unsigned long, enum meminit_context,
>  		struct vmem_altmap *, int migratetype);
> -extern void memmap_init(unsigned long size, int nid,
> +extern void memmap_init_zone(unsigned long size, int nid,
>  		unsigned long zone, unsigned long range_start_pfn);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index f9d57b9be8c7..ddcb1cd24c60 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -713,7 +713,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * expects the zone spans the pfn range. All the pages in the range
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
> -	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn, 0,
> +	memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
>  			 MEMINIT_HOTPLUG, altmap, migratetype);
>  
>  	set_zone_contiguous(zone);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 44ec5594798d..42a1d2d2a87d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6117,7 +6117,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * (usually MIGRATE_MOVABLE). Besides setting the migratetype, no related
>   * zone stats (e.g., nr_isolate_pageblock) are touched.
>   */
> -void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> +void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone,
>  		unsigned long start_pfn, unsigned long zone_end_pfn,
>  		enum meminit_context context,
>  		struct vmem_altmap *altmap, int migratetype)
> @@ -6254,7 +6254,7 @@ static void __meminit zone_init_free_lists(struct zone *zone)
>  	}
>  }
>  
> -void __meminit __weak memmap_init(unsigned long size, int nid,
> +void __meminit __weak memmap_init_zone(unsigned long size, int nid,
>  				  unsigned long zone,
>  				  unsigned long range_start_pfn)
>  {
> @@ -6268,7 +6268,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
> -			memmap_init_zone(size, nid, zone, start_pfn, range_end_pfn,
> +			memmap_init_range(size, nid, zone, start_pfn, range_end_pfn,
>  					 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
>  		}
>  	}
> @@ -6978,7 +6978,7 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
>  		set_pageblock_order();
>  		setup_usemap(pgdat, zone, zone_start_pfn, size);
>  		init_currently_empty_zone(zone, zone_start_pfn, size);
> -		memmap_init(size, nid, j, zone_start_pfn);
> +		memmap_init_zone(size, nid, j, zone_start_pfn);
>  	}
>  }
>  
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

[PATCH v5 3/5] mm: simplify parater of function memmap_init_zone()

[Baoquan He]

As David suggested, simply passing 'struct zone *zone' is enough. We can
get all needed information from 'struct zone*' easily.

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/ia64/mm/init.c | 12 +++++++-----
 include/linux/mm.h  |  3 +--
 mm/page_alloc.c     | 24 +++++++++++-------------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index c8e68e92beb3..88fb44895408 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -541,12 +541,14 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
 	return 0;
 }
 
-void __meminit
-memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-	     unsigned long start_pfn)
+void __meminit memmap_init_zone(struct zone *zone)
 {
+	int nid = zone_to_nid(zone), zone_id = zone_idx(zone);
+	unsigned long start_pfn = zone->zone_start_pfn;
+	unsigned long size = zone->spanned_pages;
+
 	if (!vmem_map) {
-		memmap_init_range(size, nid, zone, start_pfn, start_pfn + size,
+		memmap_init_range(size, nid, zone_id, start_pfn, start_pfn + size,
 				 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
 	} else {
 		struct page *start;
@@ -556,7 +558,7 @@ memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		args.start = start;
 		args.end = start + size;
 		args.nid = nid;
-		args.zone = zone;
+		args.zone = zone_id;
 
 		efi_memmap_walk(virtual_memmap_init, &args);
 	}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2395dc212221..073049bd0b29 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2401,8 +2401,7 @@ extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_range(unsigned long, int, unsigned long,
 		unsigned long, unsigned long, enum meminit_context,
 		struct vmem_altmap *, int migratetype);
-extern void memmap_init_zone(unsigned long size, int nid,
-		unsigned long zone, unsigned long range_start_pfn);
+extern void memmap_init_zone(struct zone *zone);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
 extern void mem_init(void);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 42a1d2d2a87d..cbb67d9c1b2a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6254,23 +6254,21 @@ static void __meminit zone_init_free_lists(struct zone *zone)
 	}
 }
 
-void __meminit __weak memmap_init_zone(unsigned long size, int nid,
-				  unsigned long zone,
-				  unsigned long range_start_pfn)
+void __meminit __weak memmap_init_zone(struct zone *zone)
 {
+	unsigned long zone_start_pfn = zone->zone_start_pfn;
+	unsigned long zone_end_pfn = zone_start_pfn + zone->spanned_pages;
+	int i, nid = zone_to_nid(zone), zone_id = zone_idx(zone);
 	unsigned long start_pfn, end_pfn;
-	unsigned long range_end_pfn = range_start_pfn + size;
-	int i;
 
 	for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
-		start_pfn = clamp(start_pfn, range_start_pfn, range_end_pfn);
-		end_pfn = clamp(end_pfn, range_start_pfn, range_end_pfn);
+		start_pfn = clamp(start_pfn, zone_start_pfn, zone_end_pfn);
+		end_pfn = clamp(end_pfn, zone_start_pfn, zone_end_pfn);
 
-		if (end_pfn > start_pfn) {
-			size = end_pfn - start_pfn;
-			memmap_init_range(size, nid, zone, start_pfn, range_end_pfn,
-					 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
-		}
+		if (end_pfn > start_pfn)
+			memmap_init_range(end_pfn - start_pfn, nid,
+					zone_id, start_pfn, zone_end_pfn,
+					MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
 	}
 }
 
@@ -6978,7 +6976,7 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
 		set_pageblock_order();
 		setup_usemap(pgdat, zone, zone_start_pfn, size);
 		init_currently_empty_zone(zone, zone_start_pfn, size);
-		memmap_init_zone(size, nid, j, zone_start_pfn);
+		memmap_init_zone(zone);
 	}
 }
 
-- 
2.17.2

Re: [PATCH v5 3/5] mm: simplify parater of function memmap_init_zone()

[David Hildenbrand]

On 22.01.21 14:59, Baoquan He wrote:
> As David suggested, simply passing 'struct zone *zone' is enough. We can
> get all needed information from 'struct zone*' easily.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  arch/ia64/mm/init.c | 12 +++++++-----
>  include/linux/mm.h  |  3 +--
>  mm/page_alloc.c     | 24 +++++++++++-------------
>  3 files changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index c8e68e92beb3..88fb44895408 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -541,12 +541,14 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	return 0;
>  }
>  
> -void __meminit
> -memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -	     unsigned long start_pfn)
> +void __meminit memmap_init_zone(struct zone *zone)
>  {
> +	int nid = zone_to_nid(zone), zone_id = zone_idx(zone);
> +	unsigned long start_pfn = zone->zone_start_pfn;
> +	unsigned long size = zone->spanned_pages;
> +
>  	if (!vmem_map) {
> -		memmap_init_range(size, nid, zone, start_pfn, start_pfn + size,
> +		memmap_init_range(size, nid, zone_id, start_pfn, start_pfn + size,
>  				 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
>  	} else {
>  		struct page *start;
> @@ -556,7 +558,7 @@ memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		args.start = start;
>  		args.end = start + size;
>  		args.nid = nid;
> -		args.zone = zone;
> +		args.zone = zone_id;
>  
>  		efi_memmap_walk(virtual_memmap_init, &args);
>  	}
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 2395dc212221..073049bd0b29 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2401,8 +2401,7 @@ extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_range(unsigned long, int, unsigned long,
>  		unsigned long, unsigned long, enum meminit_context,
>  		struct vmem_altmap *, int migratetype);
> -extern void memmap_init_zone(unsigned long size, int nid,
> -		unsigned long zone, unsigned long range_start_pfn);
> +extern void memmap_init_zone(struct zone *zone);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 42a1d2d2a87d..cbb67d9c1b2a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6254,23 +6254,21 @@ static void __meminit zone_init_free_lists(struct zone *zone)
>  	}
>  }
>  
> -void __meminit __weak memmap_init_zone(unsigned long size, int nid,
> -				  unsigned long zone,
> -				  unsigned long range_start_pfn)
> +void __meminit __weak memmap_init_zone(struct zone *zone)
>  {
> +	unsigned long zone_start_pfn = zone->zone_start_pfn;
> +	unsigned long zone_end_pfn = zone_start_pfn + zone->spanned_pages;
> +	int i, nid = zone_to_nid(zone), zone_id = zone_idx(zone);
>  	unsigned long start_pfn, end_pfn;
> -	unsigned long range_end_pfn = range_start_pfn + size;
> -	int i;
>  
>  	for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
> -		start_pfn = clamp(start_pfn, range_start_pfn, range_end_pfn);
> -		end_pfn = clamp(end_pfn, range_start_pfn, range_end_pfn);
> +		start_pfn = clamp(start_pfn, zone_start_pfn, zone_end_pfn);
> +		end_pfn = clamp(end_pfn, zone_start_pfn, zone_end_pfn);
>  
> -		if (end_pfn > start_pfn) {
> -			size = end_pfn - start_pfn;
> -			memmap_init_range(size, nid, zone, start_pfn, range_end_pfn,
> -					 MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
> -		}
> +		if (end_pfn > start_pfn)
> +			memmap_init_range(end_pfn - start_pfn, nid,
> +					zone_id, start_pfn, zone_end_pfn,
> +					MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
>  	}
>  }
>  
> @@ -6978,7 +6976,7 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
>  		set_pageblock_order();
>  		setup_usemap(pgdat, zone, zone_start_pfn, size);
>  		init_currently_empty_zone(zone, zone_start_pfn, size);
> -		memmap_init_zone(size, nid, j, zone_start_pfn);
> +		memmap_init_zone(zone);
>  	}
>  }
>  
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

[PATCH v5 4/5] mm: simplify parameter of setup_usemap()

[Baoquan He]

Parameter 'zone' has got needed information, let's remove other
unnecessary parameters.

Signed-off-by: Baoquan He <bhe@redhat.com>
Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 mm/page_alloc.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cbb67d9c1b2a..69cf19baac12 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6761,25 +6761,22 @@ static unsigned long __init usemap_size(unsigned long zone_start_pfn, unsigned l
 	return usemapsize / 8;
 }
 
-static void __ref setup_usemap(struct pglist_data *pgdat,
-				struct zone *zone,
-				unsigned long zone_start_pfn,
-				unsigned long zonesize)
+static void __ref setup_usemap(struct zone *zone)
 {
-	unsigned long usemapsize = usemap_size(zone_start_pfn, zonesize);
+	unsigned long usemapsize = usemap_size(zone->zone_start_pfn,
+					       zone->spanned_pages);
 	zone->pageblock_flags = NULL;
 	if (usemapsize) {
 		zone->pageblock_flags =
 			memblock_alloc_node(usemapsize, SMP_CACHE_BYTES,
-					    pgdat->node_id);
+					    zone_to_nid(zone));
 		if (!zone->pageblock_flags)
 			panic("Failed to allocate %ld bytes for zone %s pageblock flags on node %d\n",
-			      usemapsize, zone->name, pgdat->node_id);
+			      usemapsize, zone->name, zone_to_nid(zone));
 	}
 }
 #else
-static inline void setup_usemap(struct pglist_data *pgdat, struct zone *zone,
-				unsigned long zone_start_pfn, unsigned long zonesize) {}
+static inline void setup_usemap(struct zone *zone) {}
 #endif /* CONFIG_SPARSEMEM */
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
@@ -6974,7 +6971,7 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
 			continue;
 
 		set_pageblock_order();
-		setup_usemap(pgdat, zone, zone_start_pfn, size);
+		setup_usemap(zone);
 		init_currently_empty_zone(zone, zone_start_pfn, size);
 		memmap_init_zone(zone);
 	}
-- 
2.17.2

[PATCH v5 5/5] mm: remove unneeded local variable in free_area_init_core

[Baoquan He]

Local variable 'zone_start_pfn' is not needed since there's only
one call site in free_area_init_core(). Let's remove it and pass
zone->zone_start_pfn directly to init_currently_empty_zone().

Signed-off-by: Baoquan He <bhe@redhat.com>
Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 mm/page_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 69cf19baac12..e0df67948ace 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6923,7 +6923,6 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
 	for (j = 0; j < MAX_NR_ZONES; j++) {
 		struct zone *zone = pgdat->node_zones + j;
 		unsigned long size, freesize, memmap_pages;
-		unsigned long zone_start_pfn = zone->zone_start_pfn;
 
 		size = zone->spanned_pages;
 		freesize = zone->present_pages;
@@ -6972,7 +6971,7 @@ static void __init free_area_init_core(struct pglist_data *pgdat)
 
 		set_pageblock_order();
 		setup_usemap(zone);
-		init_currently_empty_zone(zone, zone_start_pfn, size);
+		init_currently_empty_zone(zone, zone->zone_start_pfn, size);
 		memmap_init_zone(zone);
 	}
 }
-- 
2.17.2

Re: [PATCH v5 0/5] mm: clean up names and parameters of memmap_init_xxxx functions

[Mike Rapoport]

On Fri, Jan 22, 2021 at 09:59:51PM +0800, Baoquan He wrote:
> This patchset is correcting inappropriate function names of
> memmap_init_xxx, and simplify parameters of functions in the code flow.
> And also fix a prototype warning reported by lkp.
> 
> This is based on the latest next/master.
> 
> V4 can be found here:
> https://lore.kernel.org/linux-mm/20210120045213.6571-1-bhe@redhat.com/
> 
> v4->v5:
>  - Add patch 1 into series which fixes a prototype warning from kernel
>    test robot. Then rebase the v4 patches on top of it.
> 
> v3->v4:
>  - Rebased patch 1, 2 on top of Mike's below new patch.
>    [PATCH v3 0/2] mm: fix initialization of struct page for holes in  memory layout
> 
>  - Move the code of renaming function parameter 'range_start_pfn' and local
>    variable 'range_end_pfn' of memmap_init() from patch 1 to patch 2
>    according to David's comment.
> 
>  - Use the reverse Christmas tree style to reorder the local variables
>    in memmap_init_zone() in patch 2 accodrding to David's comment.
> 
> Baoquan He (5):
>   mm: fix prototype warning from kernel test robot
>   mm: rename memmap_init() and memmap_init_zone()
>   mm: simplify parater of function memmap_init_zone()
>   mm: simplify parameter of setup_usemap()
>   mm: remove unneeded local variable in free_area_init_core

For the series:

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> 
>  arch/ia64/include/asm/pgtable.h |  6 -----
>  arch/ia64/mm/init.c             | 14 +++++-----
>  include/linux/mm.h              |  3 ++-
>  mm/memory_hotplug.c             |  2 +-
>  mm/page_alloc.c                 | 46 ++++++++++++++-------------------
>  5 files changed, 31 insertions(+), 40 deletions(-)
> 
> -- 
> 2.17.2
> 

-- 
Sincerely yours,
Mike.