linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Optimize the code of mem_map allocation in
@ 2018-02-01  7:19 Baoquan He
  2018-02-01  7:19 ` [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later Baoquan He
  2018-02-01  7:19 ` [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation Baoquan He
  0 siblings, 2 replies; 10+ messages in thread
From: Baoquan He @ 2018-02-01  7:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst, Baoquan He

In 5-level paging mode, allocating memory with the size of NR_MEM_SECTIONS
is a bad idea. So in this patchset, trying to optimize to save memory.
Othersise kdump kernel can't boot up with normal crashkernel reservation
setting. And for normal kernel, the 512M consumption is not also not
wise, though it's a temporary allocation. 

Baoquan He (2):
  mm/sparsemem: Defer the ms->section_mem_map clearing a little later
  mm/sparse.c: Add nr_present_sections to change the mem_map allocation

 mm/sparse-vmemmap.c |  9 +++++----
 mm/sparse.c         | 54 ++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 42 insertions(+), 21 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later
  2018-02-01  7:19 [PATCH 0/2] Optimize the code of mem_map allocation in Baoquan He
@ 2018-02-01  7:19 ` Baoquan He
  2018-02-01 14:15   ` Dave Hansen
  2018-02-01  7:19 ` [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation Baoquan He
  1 sibling, 1 reply; 10+ messages in thread
From: Baoquan He @ 2018-02-01  7:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst, Baoquan He

This will make sure number of sections marked as present won't be changed
in sparse_init(), so that for_each_present_section_nr() can iterate
each of them. This is preparation for later fix.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 mm/sparse-vmemmap.c |  1 -
 mm/sparse.c         | 15 ++++++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index 17acf01791fa..315bea91e276 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -324,7 +324,6 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 		ms = __nr_to_section(pnum);
 		pr_err("%s: sparsemem memory map backing failed some memory will not be available\n",
 		       __func__);
-		ms->section_mem_map = 0;
 	}
 
 	if (vmemmap_buf_start) {
diff --git a/mm/sparse.c b/mm/sparse.c
index 2609aba121e8..54eba92b72a1 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -478,7 +478,6 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 		ms = __nr_to_section(pnum);
 		pr_err("%s: sparsemem memory map backing failed some memory will not be available\n",
 		       __func__);
-		ms->section_mem_map = 0;
 	}
 }
 #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
@@ -610,17 +609,27 @@ void __init sparse_init(void)
 #endif
 
 	for_each_present_section_nr(0, pnum) {
+		struct mem_section *ms;
+		ms = __nr_to_section(pnum);
 		usemap = usemap_map[pnum];
-		if (!usemap)
+		if (!usemap) {
+#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
+			ms->section_mem_map = 0;
+#endif
 			continue;
+		}
 
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
 		map = map_map[pnum];
 #else
 		map = sparse_early_mem_map_alloc(pnum);
 #endif
-		if (!map)
+		if (!map) {
+#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
+			ms->section_mem_map = 0;
+#endif
 			continue;
+		}
 
 		sparse_init_one_section(__nr_to_section(pnum), pnum, map,
 								usemap);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation
  2018-02-01  7:19 [PATCH 0/2] Optimize the code of mem_map allocation in Baoquan He
  2018-02-01  7:19 ` [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later Baoquan He
@ 2018-02-01  7:19 ` Baoquan He
  2018-02-01 10:16   ` Kirill A. Shutemov
  1 sibling, 1 reply; 10+ messages in thread
From: Baoquan He @ 2018-02-01  7:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst, Baoquan He

In sparse_init(), we allocate usemap_map and map_map which are pointer
array with the size of NR_MEM_SECTIONS. The memory consumption can be
ignorable in 4-level paging mode. While in 5-level paging, this costs
much memory, 512M. Kdump kernel even can't boot up with a normal
'crashkernel=' setting.

Here add a new variable to record the number of present sections. Let's
allocate the usemap_map and map_map with the size of nr_present_sections.
We only need to make sure that for the ith present section, usemap_map[i]
and map_map[i] store its usemap and mem_map separately.

This change can save much memory on most of systems. Anytime, we should
avoid to define array or allocate memory with the size of NR_MEM_SECTIONS.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 mm/sparse-vmemmap.c |  8 +++++---
 mm/sparse.c         | 39 +++++++++++++++++++++++++--------------
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index 315bea91e276..5bb7b63276b3 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -302,6 +302,7 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 	unsigned long pnum;
 	unsigned long size = sizeof(struct page) * PAGES_PER_SECTION;
 	void *vmemmap_buf_start;
+	int i = 0;
 
 	size = ALIGN(size, PMD_SIZE);
 	vmemmap_buf_start = __earlyonly_bootmem_alloc(nodeid, size * map_count,
@@ -312,14 +313,15 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 		vmemmap_buf_end = vmemmap_buf_start + size * map_count;
 	}
 
-	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
+	for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
 		struct mem_section *ms;
 
 		if (!present_section_nr(pnum))
 			continue;
 
-		map_map[pnum] = sparse_mem_map_populate(pnum, nodeid);
-		if (map_map[pnum])
+		i++;
+		map_map[i-1] = sparse_mem_map_populate(pnum, nodeid);
+		if (map_map[i-1])
 			continue;
 		ms = __nr_to_section(pnum);
 		pr_err("%s: sparsemem memory map backing failed some memory will not be available\n",
diff --git a/mm/sparse.c b/mm/sparse.c
index 54eba92b72a1..18273261be6d 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -202,6 +202,7 @@ static inline int next_present_section_nr(int section_nr)
 	      (section_nr <= __highest_present_section_nr));	\
 	     section_nr = next_present_section_nr(section_nr))
 
+static int nr_present_sections;
 /* Record a memory area against a node. */
 void __init memory_present(int nid, unsigned long start, unsigned long end)
 {
@@ -231,6 +232,7 @@ void __init memory_present(int nid, unsigned long start, unsigned long end)
 			ms->section_mem_map = sparse_encode_early_nid(nid) |
 							SECTION_IS_ONLINE;
 			section_mark_present(ms);
+			nr_present_sections++;
 		}
 	}
 }
@@ -399,6 +401,7 @@ static void __init sparse_early_usemaps_alloc_node(void *data,
 	unsigned long pnum;
 	unsigned long **usemap_map = (unsigned long **)data;
 	int size = usemap_size();
+	int i = 0;
 
 	usemap = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nodeid),
 							  size * usemap_count);
@@ -407,12 +410,13 @@ static void __init sparse_early_usemaps_alloc_node(void *data,
 		return;
 	}
 
-	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
+	for (pnum = pnum_begin; pnum < pnum_end && i < usemap_count; pnum++) {
 		if (!present_section_nr(pnum))
 			continue;
-		usemap_map[pnum] = usemap;
+		usemap_map[i] = usemap;
 		usemap += size;
-		check_usemap_section_nr(nodeid, usemap_map[pnum]);
+		check_usemap_section_nr(nodeid, usemap_map[i]);
+		i++;
 	}
 }
 
@@ -440,13 +444,15 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 	void *map;
 	unsigned long pnum;
 	unsigned long size = sizeof(struct page) * PAGES_PER_SECTION;
+	int i;
 
 	map = alloc_remap(nodeid, size * map_count);
 	if (map) {
-		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
+		i = 0;
+		for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
 			if (!present_section_nr(pnum))
 				continue;
-			map_map[pnum] = map;
+			map_map[i] = map;
 			map += size;
 		}
 		return;
@@ -457,23 +463,26 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 					      PAGE_SIZE, __pa(MAX_DMA_ADDRESS),
 					      BOOTMEM_ALLOC_ACCESSIBLE, nodeid);
 	if (map) {
-		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
+		i = 0;
+		for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
 			if (!present_section_nr(pnum))
 				continue;
-			map_map[pnum] = map;
+			map_map[i] = map;
 			map += size;
 		}
 		return;
 	}
 
 	/* fallback */
-	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
+	i = 0;
+	for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
 		struct mem_section *ms;
 
 		if (!present_section_nr(pnum))
 			continue;
-		map_map[pnum] = sparse_mem_map_populate(pnum, nodeid);
-		if (map_map[pnum])
+		i++;
+		map_map[i-1] = sparse_mem_map_populate(pnum, nodeid);
+		if (map_map[i-1])
 			continue;
 		ms = __nr_to_section(pnum);
 		pr_err("%s: sparsemem memory map backing failed some memory will not be available\n",
@@ -552,6 +561,7 @@ static void __init alloc_usemap_and_memmap(void (*alloc_func)
 		/* new start, update count etc*/
 		nodeid_begin = nodeid;
 		pnum_begin = pnum;
+		data += map_count;
 		map_count = 1;
 	}
 	/* ok, last chunk */
@@ -570,6 +580,7 @@ void __init sparse_init(void)
 	unsigned long *usemap;
 	unsigned long **usemap_map;
 	int size;
+	int i = 0;
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
 	int size2;
 	struct page **map_map;
@@ -592,7 +603,7 @@ void __init sparse_init(void)
 	 * powerpc need to call sparse_init_one_section right after each
 	 * sparse_early_mem_map_alloc, so allocate usemap_map at first.
 	 */
-	size = sizeof(unsigned long *) * NR_MEM_SECTIONS;
+	size = sizeof(unsigned long *) * nr_present_sections;
 	usemap_map = memblock_virt_alloc(size, 0);
 	if (!usemap_map)
 		panic("can not allocate usemap_map\n");
@@ -600,7 +611,7 @@ void __init sparse_init(void)
 							(void *)usemap_map);
 
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
-	size2 = sizeof(struct page *) * NR_MEM_SECTIONS;
+	size2 = sizeof(struct page *) * nr_present_sections;
 	map_map = memblock_virt_alloc(size2, 0);
 	if (!map_map)
 		panic("can not allocate map_map\n");
@@ -611,7 +622,7 @@ void __init sparse_init(void)
 	for_each_present_section_nr(0, pnum) {
 		struct mem_section *ms;
 		ms = __nr_to_section(pnum);
-		usemap = usemap_map[pnum];
+		usemap = usemap_map[i];
 		if (!usemap) {
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
 			ms->section_mem_map = 0;
@@ -620,7 +631,7 @@ void __init sparse_init(void)
 		}
 
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
-		map = map_map[pnum];
+		map = map_map[i];
 #else
 		map = sparse_early_mem_map_alloc(pnum);
 #endif
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation
  2018-02-01  7:19 ` [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation Baoquan He
@ 2018-02-01 10:16   ` Kirill A. Shutemov
  2018-02-01 13:49     ` Dave Hansen
  0 siblings, 1 reply; 10+ messages in thread
From: Kirill A. Shutemov @ 2018-02-01 10:16 UTC (permalink / raw)
  To: Baoquan He, Dave Hansen
  Cc: linux-kernel, linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst

On Thu, Feb 01, 2018 at 03:19:56PM +0800, Baoquan He wrote:
> In sparse_init(), we allocate usemap_map and map_map which are pointer
> array with the size of NR_MEM_SECTIONS. The memory consumption can be
> ignorable in 4-level paging mode. While in 5-level paging, this costs
> much memory, 512M. Kdump kernel even can't boot up with a normal
> 'crashkernel=' setting.
> 
> Here add a new variable to record the number of present sections. Let's
> allocate the usemap_map and map_map with the size of nr_present_sections.
> We only need to make sure that for the ith present section, usemap_map[i]
> and map_map[i] store its usemap and mem_map separately.
> 
> This change can save much memory on most of systems. Anytime, we should
> avoid to define array or allocate memory with the size of NR_MEM_SECTIONS.

That's very desirable outcome. But I don't know much about sparsemem.

Dave, could you take a look?

> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  mm/sparse-vmemmap.c |  8 +++++---
>  mm/sparse.c         | 39 +++++++++++++++++++++++++--------------
>  2 files changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 315bea91e276..5bb7b63276b3 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -302,6 +302,7 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>  	unsigned long pnum;
>  	unsigned long size = sizeof(struct page) * PAGES_PER_SECTION;
>  	void *vmemmap_buf_start;
> +	int i = 0;
>  
>  	size = ALIGN(size, PMD_SIZE);
>  	vmemmap_buf_start = __earlyonly_bootmem_alloc(nodeid, size * map_count,
> @@ -312,14 +313,15 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>  		vmemmap_buf_end = vmemmap_buf_start + size * map_count;
>  	}
>  
> -	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> +	for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
>  		struct mem_section *ms;
>  
>  		if (!present_section_nr(pnum))
>  			continue;
>  
> -		map_map[pnum] = sparse_mem_map_populate(pnum, nodeid);
> -		if (map_map[pnum])
> +		i++;
> +		map_map[i-1] = sparse_mem_map_populate(pnum, nodeid);
> +		if (map_map[i-1])
>  			continue;
>  		ms = __nr_to_section(pnum);
>  		pr_err("%s: sparsemem memory map backing failed some memory will not be available\n",
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 54eba92b72a1..18273261be6d 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -202,6 +202,7 @@ static inline int next_present_section_nr(int section_nr)
>  	      (section_nr <= __highest_present_section_nr));	\
>  	     section_nr = next_present_section_nr(section_nr))
>  
> +static int nr_present_sections;
>  /* Record a memory area against a node. */
>  void __init memory_present(int nid, unsigned long start, unsigned long end)
>  {
> @@ -231,6 +232,7 @@ void __init memory_present(int nid, unsigned long start, unsigned long end)
>  			ms->section_mem_map = sparse_encode_early_nid(nid) |
>  							SECTION_IS_ONLINE;
>  			section_mark_present(ms);
> +			nr_present_sections++;
>  		}
>  	}
>  }
> @@ -399,6 +401,7 @@ static void __init sparse_early_usemaps_alloc_node(void *data,
>  	unsigned long pnum;
>  	unsigned long **usemap_map = (unsigned long **)data;
>  	int size = usemap_size();
> +	int i = 0;
>  
>  	usemap = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nodeid),
>  							  size * usemap_count);
> @@ -407,12 +410,13 @@ static void __init sparse_early_usemaps_alloc_node(void *data,
>  		return;
>  	}
>  
> -	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> +	for (pnum = pnum_begin; pnum < pnum_end && i < usemap_count; pnum++) {
>  		if (!present_section_nr(pnum))
>  			continue;
> -		usemap_map[pnum] = usemap;
> +		usemap_map[i] = usemap;
>  		usemap += size;
> -		check_usemap_section_nr(nodeid, usemap_map[pnum]);
> +		check_usemap_section_nr(nodeid, usemap_map[i]);
> +		i++;
>  	}
>  }
>  
> @@ -440,13 +444,15 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>  	void *map;
>  	unsigned long pnum;
>  	unsigned long size = sizeof(struct page) * PAGES_PER_SECTION;
> +	int i;
>  
>  	map = alloc_remap(nodeid, size * map_count);
>  	if (map) {
> -		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> +		i = 0;
> +		for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
>  			if (!present_section_nr(pnum))
>  				continue;
> -			map_map[pnum] = map;
> +			map_map[i] = map;
>  			map += size;
>  		}
>  		return;
> @@ -457,23 +463,26 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>  					      PAGE_SIZE, __pa(MAX_DMA_ADDRESS),
>  					      BOOTMEM_ALLOC_ACCESSIBLE, nodeid);
>  	if (map) {
> -		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> +		i = 0;
> +		for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
>  			if (!present_section_nr(pnum))
>  				continue;
> -			map_map[pnum] = map;
> +			map_map[i] = map;
>  			map += size;
>  		}
>  		return;
>  	}
>  
>  	/* fallback */
> -	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> +	i = 0;
> +	for (pnum = pnum_begin; pnum < pnum_end && i < map_count; pnum++) {
>  		struct mem_section *ms;
>  
>  		if (!present_section_nr(pnum))
>  			continue;
> -		map_map[pnum] = sparse_mem_map_populate(pnum, nodeid);
> -		if (map_map[pnum])
> +		i++;
> +		map_map[i-1] = sparse_mem_map_populate(pnum, nodeid);
> +		if (map_map[i-1])
>  			continue;
>  		ms = __nr_to_section(pnum);
>  		pr_err("%s: sparsemem memory map backing failed some memory will not be available\n",
> @@ -552,6 +561,7 @@ static void __init alloc_usemap_and_memmap(void (*alloc_func)
>  		/* new start, update count etc*/
>  		nodeid_begin = nodeid;
>  		pnum_begin = pnum;
> +		data += map_count;
>  		map_count = 1;
>  	}
>  	/* ok, last chunk */
> @@ -570,6 +580,7 @@ void __init sparse_init(void)
>  	unsigned long *usemap;
>  	unsigned long **usemap_map;
>  	int size;
> +	int i = 0;
>  #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
>  	int size2;
>  	struct page **map_map;
> @@ -592,7 +603,7 @@ void __init sparse_init(void)
>  	 * powerpc need to call sparse_init_one_section right after each
>  	 * sparse_early_mem_map_alloc, so allocate usemap_map at first.
>  	 */
> -	size = sizeof(unsigned long *) * NR_MEM_SECTIONS;
> +	size = sizeof(unsigned long *) * nr_present_sections;
>  	usemap_map = memblock_virt_alloc(size, 0);
>  	if (!usemap_map)
>  		panic("can not allocate usemap_map\n");
> @@ -600,7 +611,7 @@ void __init sparse_init(void)
>  							(void *)usemap_map);
>  
>  #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> -	size2 = sizeof(struct page *) * NR_MEM_SECTIONS;
> +	size2 = sizeof(struct page *) * nr_present_sections;
>  	map_map = memblock_virt_alloc(size2, 0);
>  	if (!map_map)
>  		panic("can not allocate map_map\n");
> @@ -611,7 +622,7 @@ void __init sparse_init(void)
>  	for_each_present_section_nr(0, pnum) {
>  		struct mem_section *ms;
>  		ms = __nr_to_section(pnum);
> -		usemap = usemap_map[pnum];
> +		usemap = usemap_map[i];
>  		if (!usemap) {
>  #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
>  			ms->section_mem_map = 0;
> @@ -620,7 +631,7 @@ void __init sparse_init(void)
>  		}
>  
>  #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> -		map = map_map[pnum];
> +		map = map_map[i];
>  #else
>  		map = sparse_early_mem_map_alloc(pnum);
>  #endif
> -- 
> 2.13.6
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation
  2018-02-01 10:16   ` Kirill A. Shutemov
@ 2018-02-01 13:49     ` Dave Hansen
  2018-02-01 14:19       ` Baoquan He
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2018-02-01 13:49 UTC (permalink / raw)
  To: Kirill A. Shutemov, Baoquan He
  Cc: linux-kernel, linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst

On 02/01/2018 02:16 AM, Kirill A. Shutemov wrote:
> On Thu, Feb 01, 2018 at 03:19:56PM +0800, Baoquan He wrote:
>> In sparse_init(), we allocate usemap_map and map_map which are pointer
>> array with the size of NR_MEM_SECTIONS. The memory consumption can be
>> ignorable in 4-level paging mode. While in 5-level paging, this costs
>> much memory, 512M. Kdump kernel even can't boot up with a normal
>> 'crashkernel=' setting.
>>
>> Here add a new variable to record the number of present sections. Let's
>> allocate the usemap_map and map_map with the size of nr_present_sections.
>> We only need to make sure that for the ith present section, usemap_map[i]
>> and map_map[i] store its usemap and mem_map separately.
>>
>> This change can save much memory on most of systems. Anytime, we should
>> avoid to define array or allocate memory with the size of NR_MEM_SECTIONS.
> That's very desirable outcome. But I don't know much about sparsemem.

... with the downside being that we can no longer hot-add memory that
was not part of the original, present sections.

Is that OK?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later
  2018-02-01  7:19 ` [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later Baoquan He
@ 2018-02-01 14:15   ` Dave Hansen
  2018-02-01 14:38     ` Baoquan He
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2018-02-01 14:15 UTC (permalink / raw)
  To: Baoquan He, linux-kernel
  Cc: linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst

On 01/31/2018 11:19 PM, Baoquan He wrote:
>  	for_each_present_section_nr(0, pnum) {
> +		struct mem_section *ms;
> +		ms = __nr_to_section(pnum);
>  		usemap = usemap_map[pnum];
> -		if (!usemap)
> +		if (!usemap) {
> +#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> +			ms->section_mem_map = 0;
> +#endif
>  			continue;
> +		}
>  
>  #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
>  		map = map_map[pnum];
>  #else
>  		map = sparse_early_mem_map_alloc(pnum);
>  #endif
> -		if (!map)
> +		if (!map) {
> +#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> +			ms->section_mem_map = 0;
> +#endif
>  			continue;
> +		}

This is starting to look like code that only a mother could love.  Can
this be cleaned up a bit?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation
  2018-02-01 13:49     ` Dave Hansen
@ 2018-02-01 14:19       ` Baoquan He
  2018-02-01 14:23         ` Dave Hansen
  0 siblings, 1 reply; 10+ messages in thread
From: Baoquan He @ 2018-02-01 14:19 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Kirill A. Shutemov, linux-kernel, linux-mm, akpm,
	kirill.shutemov, mhocko, tglx, douly.fnst

On 02/01/18 at 05:49am, Dave Hansen wrote:
> On 02/01/2018 02:16 AM, Kirill A. Shutemov wrote:
> > On Thu, Feb 01, 2018 at 03:19:56PM +0800, Baoquan He wrote:
> >> In sparse_init(), we allocate usemap_map and map_map which are pointer
> >> array with the size of NR_MEM_SECTIONS. The memory consumption can be
> >> ignorable in 4-level paging mode. While in 5-level paging, this costs
> >> much memory, 512M. Kdump kernel even can't boot up with a normal
> >> 'crashkernel=' setting.
> >>
> >> Here add a new variable to record the number of present sections. Let's
> >> allocate the usemap_map and map_map with the size of nr_present_sections.
> >> We only need to make sure that for the ith present section, usemap_map[i]
> >> and map_map[i] store its usemap and mem_map separately.
> >>
> >> This change can save much memory on most of systems. Anytime, we should
> >> avoid to define array or allocate memory with the size of NR_MEM_SECTIONS.
> > That's very desirable outcome. But I don't know much about sparsemem.
> 
> ... with the downside being that we can no longer hot-add memory that
> was not part of the original, present sections.
> 
> Is that OK?

Thanks for looking into this, Dave!

I suppose these functions changed here are only called during system
bootup, namely in paging_init(). Hot-add memory goes in a different
path, __add_section() -> sparse_add_one_section(), different called
functions.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation
  2018-02-01 14:19       ` Baoquan He
@ 2018-02-01 14:23         ` Dave Hansen
  2018-02-01 14:33           ` Baoquan He
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2018-02-01 14:23 UTC (permalink / raw)
  To: Baoquan He
  Cc: Kirill A. Shutemov, linux-kernel, linux-mm, akpm,
	kirill.shutemov, mhocko, tglx, douly.fnst

On 02/01/2018 06:19 AM, Baoquan He wrote:
> 
> I suppose these functions changed here are only called during system
> bootup, namely in paging_init(). Hot-add memory goes in a different
> path, __add_section() -> sparse_add_one_section(), different called
> functions.

But does this keep those sections that were not present on boot from
being added later?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation
  2018-02-01 14:23         ` Dave Hansen
@ 2018-02-01 14:33           ` Baoquan He
  0 siblings, 0 replies; 10+ messages in thread
From: Baoquan He @ 2018-02-01 14:33 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Kirill A. Shutemov, linux-kernel, linux-mm, akpm,
	kirill.shutemov, mhocko, tglx, douly.fnst

On 02/01/18 at 06:23am, Dave Hansen wrote:
> On 02/01/2018 06:19 AM, Baoquan He wrote:
> > 
> > I suppose these functions changed here are only called during system
> > bootup, namely in paging_init(). Hot-add memory goes in a different
> > path, __add_section() -> sparse_add_one_section(), different called
> > functions.
> 
> But does this keep those sections that were not present on boot from
> being added later?

I think it won't. As you can see, the referred functions are only called
during init stage. If anyone try to use any of them for later hot-add memory,
that will cause problem, lucky there isn't. And this is only used to
store the allocated usemap and mem_map for each present section on boot.
After that, the usemap_map and map_map pointer array will be freed, they
are temporary here. I forget mentioning this in patch log, sorry for bringing
confusion.

void __init sparse_memory_present_with_active_regions(int nid)
void __init memory_present(int nid, unsigned long start, unsigned long end)
void __init sparse_init(void) 
static void __init alloc_usemap_and_memmap(...)
static void __init sparse_early_usemaps_alloc_node(...)
static void __init sparse_early_mem_maps_alloc_node(...)
void __init sparse_mem_maps_populate_node(...)
void __init sparse_mem_maps_populate_node(...)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later
  2018-02-01 14:15   ` Dave Hansen
@ 2018-02-01 14:38     ` Baoquan He
  0 siblings, 0 replies; 10+ messages in thread
From: Baoquan He @ 2018-02-01 14:38 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-kernel, linux-mm, akpm, kirill.shutemov, mhocko, tglx, douly.fnst

On 02/01/18 at 06:15am, Dave Hansen wrote:
> On 01/31/2018 11:19 PM, Baoquan He wrote:
> >  	for_each_present_section_nr(0, pnum) {
> > +		struct mem_section *ms;
> > +		ms = __nr_to_section(pnum);
> >  		usemap = usemap_map[pnum];
> > -		if (!usemap)
> > +		if (!usemap) {
> > +#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> > +			ms->section_mem_map = 0;
> > +#endif
> >  			continue;
> > +		}
> >  
> >  #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> >  		map = map_map[pnum];
> >  #else
> >  		map = sparse_early_mem_map_alloc(pnum);
> >  #endif
> > -		if (!map)
> > +		if (!map) {
> > +#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> > +			ms->section_mem_map = 0;
> > +#endif
> >  			continue;
> > +		}
> 
> This is starting to look like code that only a mother could love.  Can
> this be cleaned up a bit?

Sorry, will try. Just wonder why we don't need to clear
ms->section_mem_map when CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER is not
set. Will look into to find reason.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-02-01 14:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-01  7:19 [PATCH 0/2] Optimize the code of mem_map allocation in Baoquan He
2018-02-01  7:19 ` [PATCH 1/2] mm/sparsemem: Defer the ms->section_mem_map clearing a little later Baoquan He
2018-02-01 14:15   ` Dave Hansen
2018-02-01 14:38     ` Baoquan He
2018-02-01  7:19 ` [PATCH 2/2] mm/sparse.c: Add nr_present_sections to change the mem_map allocation Baoquan He
2018-02-01 10:16   ` Kirill A. Shutemov
2018-02-01 13:49     ` Dave Hansen
2018-02-01 14:19       ` Baoquan He
2018-02-01 14:23         ` Dave Hansen
2018-02-01 14:33           ` Baoquan He

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).