mm/sparse.c: Use kvmalloc/kvfree to alloc/free memmap for the classic sparse

---
v3->v4:
Split the old v3 into two patches, to carve out the using 'nid' as preferred node to allocate memmap into a separate patch. This is suggested by Michal, and the carving out is put in patch 2.

v2->v3:
Remove __GFP_NOWARN and use array_size when calling kvmalloc_node() per Matthew's comments.
http://lkml.kernel.org/r/20200312141749.GL27711@MiWiFi-R3L-srv

 mm/sparse.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

mm/sparse.c +3 ~~-24~~

index e747a238a860..d01d09cc7d99 100644
@@ -719,35 +719,14 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	struct page *page, *ret;
-	unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION;
-
-	page = alloc_pages(GFP_KERNEL|__GFP_NOWARN, get_order(memmap_size));
-	if (page)
-		goto got_map_page;
-
-	ret = vmalloc(memmap_size);
-	if (ret)
-		goto got_map_ptr;
-
-	return NULL;
-got_map_page:
-	ret = (struct page *)pfn_to_kaddr(page_to_pfn(page));
-got_map_ptr:
-
-	return ret;
+	return kvmalloc(array_size(sizeof(struct page),
+			PAGES_PER_SECTION), GFP_KERNEL);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
 		struct vmem_altmap *altmap)
 {
-	struct page *memmap = pfn_to_page(pfn);
-
-	if (is_vmalloc_addr(memmap))
-		vfree(memmap);
-	else
-		free_pages((unsigned long)memmap,
-			   get_order(sizeof(struct page) * PAGES_PER_SECTION));
+	kvfree(pfn_to_page(pfn));
 }
 
 static void free_map_bootmem(struct page *memmap)

Signature · 1 line

2.17.2

Baoquan He

author

2020-03-16 10:22 UTC #1 lore raw

PATCH v4 2/2 mm/sparse.c: allocate memmap preferring the given node

When allocating memmap for hot added memory with the classic sparse, the specified 'nid' is ignored in populate_section_memmap().

While in allocating memmap for the classic sparse during boot, the node given by 'nid' is preferred. And VMEMMAP prefers the node of 'nid' in both boot stage and memory hot adding. So seems no reason to not respect the node of 'nid' for the classic sparse when hot adding memory.

Use kvmalloc_node instead to use the passed in 'nid'.

Signed-off-by Baoquan He <[email protected]>

---
 mm/sparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/sparse.c +2 -2

index d01d09cc7d99..513d765e8c72 100644
@@ -719,8 +719,8 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	return kvmalloc(array_size(sizeof(struct page),
-			PAGES_PER_SECTION), GFP_KERNEL);
+	return kvmalloc_node(array_size(sizeof(struct page),
+			     PAGES_PER_SECTION), GFP_KERNEL, nid);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,

Signature · 1 line

2.17.2

Baoquan He

author

2020-03-16 12:56 UTC #2 lore raw

PATCH v5 2/2 mm/sparse.c: allocate memmap preferring the given node

When allocating memmap for hot added memory with the classic sparse, the specified 'nid' is ignored in populate_section_memmap().

Use kvmalloc_node instead to use the passed in 'nid'.

Signed-off-by Baoquan He <[email protected]>

Reviewed-by Matthew Wilcox (Oracle) <[email protected]>

---
 mm/sparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/sparse.c +2 -2

index 3fa407d7f70a..31dcdfb55c72 100644
@@ -719,8 +719,8 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	return kvmalloc(array_size(sizeof(struct page),
-				   PAGES_PER_SECTION), GFP_KERNEL);
+	return kvmalloc_node(array_size(sizeof(struct page),
+					PAGES_PER_SECTION), GFP_KERNEL, nid);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,

Signature · 1 line

2.17.2

Pankaj Gupta

Acked-by on v5 2/2

2020-03-16 16:28 UTC #3 lore raw

Re: [PATCH v5 2/2] mm/sparse.c: allocate memmap preferring the given node

Quoted 23 lines

When allocating memmap for hot added memory with the classic sparse, the specified 'nid' is ignored in populate_section_memmap().
While in allocating memmap for the classic sparse during boot, the node given by 'nid' is preferred. And VMEMMAP prefers the node of 'nid' in both boot stage and memory hot adding. So seems no reason to not respect the node of 'nid' for the classic sparse when hot adding memory.
Use kvmalloc_node instead to use the passed in 'nid'.
Signed-off-by Baoquan He <[email protected]>
Acked-by Michal Hocko <[email protected]>
Reviewed-by Matthew Wilcox (Oracle) <[email protected]>
---
 mm/sparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
mm/sparse.c +0 -0
index 3fa407d7f70a..31dcdfb55c72 100644
@@ -719,8 +719,8 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,

mm/sparse.c static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)

Quoted 10 lines

mm/sparse.c +2 -3

                unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-       return kvmalloc(array_size(sizeof(struct page),
-                                  PAGES_PER_SECTION), GFP_KERNEL);
+       return kvmalloc_node(array_size(sizeof(struct page),
+                                       PAGES_PER_SECTION), GFP_KERNEL, nid);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
--

Acked-by Pankaj Gupta <[email protected]>

Quoted 3 lines

2.17.2

David Hildenbrand

Reviewed-by on v5 2/2

2020-03-16 16:37 UTC #4 lore raw

Re: [PATCH v5 2/2] mm/sparse.c: allocate memmap preferring the given node

Quoted 23 lines · Baoquan He

When allocating memmap for hot added memory with the classic sparse, the specified 'nid' is ignored in populate_section_memmap().
While in allocating memmap for the classic sparse during boot, the node given by 'nid' is preferred. And VMEMMAP prefers the node of 'nid' in both boot stage and memory hot adding. So seems no reason to not respect the node of 'nid' for the classic sparse when hot adding memory.
Use kvmalloc_node instead to use the passed in 'nid'.
Signed-off-by Baoquan He <[email protected]>
Acked-by Michal Hocko <[email protected]>
Reviewed-by Matthew Wilcox (Oracle) <[email protected]>
---
 mm/sparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
mm/sparse.c +0 -0
index 3fa407d7f70a..31dcdfb55c72 100644

mm/sparse.c static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages) · line 726

Quoted 10 lines

mm/sparse.c +2 -2

@@ -719,8 +719,8 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	return kvmalloc(array_size(sizeof(struct page),
-				   PAGES_PER_SECTION), GFP_KERNEL);
+	return kvmalloc_node(array_size(sizeof(struct page),
+					PAGES_PER_SECTION), GFP_KERNEL, nid);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,

Signature · 3 lines

Thanks,

David / dhildenb

Wei Yang

Reviewed-by on v5 2/2

2020-03-16 22:16 UTC #5 lore raw

Re: [PATCH v5 2/2] mm/sparse.c: allocate memmap preferring the given node

Quoted 8 lines · Baoquan He

When allocating memmap for hot added memory with the classic sparse, the specified 'nid' is ignored in populate_section_memmap().
While in allocating memmap for the classic sparse during boot, the node given by 'nid' is preferred. And VMEMMAP prefers the node of 'nid' in both boot stage and memory hot adding. So seems no reason to not respect the node of 'nid' for the classic sparse when hot adding memory.

Quoted 5 lines

Use kvmalloc_node instead to use the passed in 'nid'.
Signed-off-by Baoquan He <[email protected]>
Acked-by Michal Hocko <[email protected]>
Reviewed-by Matthew Wilcox (Oracle) <[email protected]>

Reviewed-by Wei Yang <[email protected]>

Signature · 2 lines

Wei Yang
Help you, Help me

Baoquan He

author

2020-03-24 01:07 UTC #6 lore raw

Re: [PATCH v5 2/2] mm/sparse.c: allocate memmap preferring the given node

Hi Andrew,

Quoted 9 lines · Baoquan He

When allocating memmap for hot added memory with the classic sparse, the specified 'nid' is ignored in populate_section_memmap().
While in allocating memmap for the classic sparse during boot, the node given by 'nid' is preferred. And VMEMMAP prefers the node of 'nid' in both boot stage and memory hot adding. So seems no reason to not respect the node of 'nid' for the classic sparse when hot adding memory.
Use kvmalloc_node instead to use the passed in 'nid'.

Just checked linux-next, seems this one is missed. Michal suggested splitting the old v4 into two patches, this patch is to use the passed in 'nid' to allocate memmap with !VMEMMAP.

Thanks
Baoquan

Quoted 27 lines

Signed-off-by Baoquan He <[email protected]>

Reviewed-by Matthew Wilcox (Oracle) <[email protected]>

---
 mm/sparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/sparse.c +2 -2

index 3fa407d7f70a..31dcdfb55c72 100644
@@ -719,8 +719,8 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	return kvmalloc(array_size(sizeof(struct page),
-				   PAGES_PER_SECTION), GFP_KERNEL);
+	return kvmalloc_node(array_size(sizeof(struct page),
+					PAGES_PER_SECTION), GFP_KERNEL, nid);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,

Signature · 1 line

2.17.2

David Hildenbrand

2020-03-16 11:00 UTC #7 lore raw

Quoted 38 lines · Baoquan He

This change makes populate_section_memmap()/depopulate_section_memmap much simpler.
Suggested-by Michal Hocko <[email protected]>
Signed-off-by Baoquan He <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Acked-by Michal Hocko <[email protected]>
---
v3->v4:
Split the old v3 into two patches, to carve out the using 'nid' as preferred node to allocate memmap into a separate patch. This is suggested by Michal, and the carving out is put in patch 2.
v2->v3:
Remove __GFP_NOWARN and use array_size when calling kvmalloc_node() per Matthew's comments.
http://lkml.kernel.org/r/20200312141749.GL27711@MiWiFi-R3L-srv
 mm/sparse.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)
mm/sparse.c +0 -9
index e747a238a860..d01d09cc7d99 100644
@@ -719,35 +719,14 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	struct page *page, *ret;
-	unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION;
-
-	page = alloc_pages(GFP_KERNEL|__GFP_NOWARN, get_order(memmap_size));
-	if (page)
-		goto got_map_page;
-
-	ret = vmalloc(memmap_size);
-	if (ret)

mm/sparse.c static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages) · line 723

Quoted 10 lines

mm/sparse.c +2 -8

-		goto got_map_ptr;
-
-	return NULL;
-got_map_page:
-	ret = (struct page *)pfn_to_kaddr(page_to_pfn(page));
-got_map_ptr:
-
-	return ret;
+	return kvmalloc(array_size(sizeof(struct page),
+			PAGES_PER_SECTION), GFP_KERNEL);

FWIW, this is what I meant:

return kvmalloc(array_size(sizeof(struct page),
PAGES_PER_SECTION), GFP_KERNEL);

Signature · 3 lines

Thanks,

David / dhildenb

Baoquan He

author

2020-03-16 12:41 UTC #8 lore raw

Quoted 52 lines · David Hildenbrand

Quoted 38 lines · Baoquan He
This change makes populate_section_memmap()/depopulate_section_memmap much simpler.
Suggested-by Michal Hocko <[email protected]>
Signed-off-by Baoquan He <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Acked-by Michal Hocko <[email protected]>
---
v3->v4:
Split the old v3 into two patches, to carve out the using 'nid' as preferred node to allocate memmap into a separate patch. This is suggested by Michal, and the carving out is put in patch 2.
v2->v3:
Remove __GFP_NOWARN and use array_size when calling kvmalloc_node() per Matthew's comments.
http://lkml.kernel.org/r/20200312141749.GL27711@MiWiFi-R3L-srv
 mm/sparse.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)
mm/sparse.c +0 -9
index e747a238a860..d01d09cc7d99 100644
@@ -719,35 +719,14 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-	struct page *page, *ret;
-	unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION;
-
-	page = alloc_pages(GFP_KERNEL|__GFP_NOWARN, get_order(memmap_size));
-	if (page)
-		goto got_map_page;
-
-	ret = vmalloc(memmap_size);
-	if (ret)
mm/sparse.c static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages) · line 723
Quoted 10 lines
mm/sparse.c +2 -8
-		goto got_map_ptr;
-
-	return NULL;
-got_map_page:
-	ret = (struct page *)pfn_to_kaddr(page_to_pfn(page));
-got_map_ptr:
-
-	return ret;
+	return kvmalloc(array_size(sizeof(struct page),
+			PAGES_PER_SECTION), GFP_KERNEL);
FWIW, this is what I meant:

Quoted 2 lines

return kvmalloc(array_size(sizeof(struct page),
PAGES_PER_SECTION), GFP_KERNEL);

Since there's another parameter, I didn't indent it with sizeof. But Pankaj and Matthew have added other two votes on this, I will change it, thanks.

Pankaj Gupta

Reviewed-by on v4 1/2

2020-03-16 11:18 UTC #9 lore raw

Quoted 65 lines

This change makes populate_section_memmap()/depopulate_section_memmap much simpler.

Signed-off-by Baoquan He <[email protected]>

v2->v3:
Remove __GFP_NOWARN and use array_size when calling kvmalloc_node() per Matthew's comments.
http://lkml.kernel.org/r/20200312141749.GL27711@MiWiFi-R3L-srv

 mm/sparse.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

mm/sparse.c +3 ~~-25~~

index e747a238a860..d01d09cc7d99 100644
@@ -719,35 +719,14 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 struct page * __meminit populate_section_memmap(unsigned long pfn,
                unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
 {
-       struct page *page, *ret;
-       unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION;
-
-       page = alloc_pages(GFP_KERNEL|__GFP_NOWARN, get_order(memmap_size));
-       if (page)
-               goto got_map_page;
-
-       ret = vmalloc(memmap_size);
-       if (ret)
-               goto got_map_ptr;
-
-       return NULL;
-got_map_page:
-       ret = (struct page *)pfn_to_kaddr(page_to_pfn(page));
-got_map_ptr:
-
-       return ret;
+       return kvmalloc(array_size(sizeof(struct page),
+                       PAGES_PER_SECTION), GFP_KERNEL);
 }
 
 static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
                struct vmem_altmap *altmap)
 {
-       struct page *memmap = pfn_to_page(pfn);
-
-       if (is_vmalloc_addr(memmap))
-               vfree(memmap);
-       else
-               free_pages((unsigned long)memmap,
-                          get_order(sizeof(struct page) * PAGES_PER_SECTION));
+       kvfree(pfn_to_page(pfn));
 }
 
 static void free_map_bootmem(struct page *memmap)
--

Quoted 1 line

2.17.2

With David's indentation suggestion:

Reviewed-by Pankaj Gupta <[email protected]>

Quoted 2 lines

Matthew Wilcox

Reviewed-by

2020-03-16 12:18 UTC #10 lore raw

Quoted 10 lines · Pankaj Gupta

Quoted 7 lines
This change makes populate_section_memmap()/depopulate_section_memmap much simpler.
Suggested-by Michal Hocko <[email protected]>
Signed-off-by Baoquan He <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Acked-by Michal Hocko <[email protected]>
With David's indentation suggestion:
Reviewed-by Pankaj Gupta <[email protected]>

Reviewed-by Matthew Wilcox (Oracle) <[email protected]>

(for both patches)

Baoquan He

author

2020-03-16 12:55 UTC #11 lore raw

PATCH v5 1/2 mm/sparse.c: Use kvmalloc/kvfree to alloc/free memmap for the classic sparse

This change makes populate_section_memmap()/depopulate_section_memmap much simpler.

Signed-off-by Baoquan He <[email protected]>