Skip to contentSkip to conversation

Clean up comments and codes in sparse_add_one_section()

PATCH Superseded Baoquan He 6 participants 26 messages Mar 26, 2019 – Mar 29, 2019

Subsystems (4)
Memory management MM core Driver core Memory hot(un)plug

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Baoquan He

author
#0 lore raw

v2 cover Clean up comments and codes in sparse_add_one_section()

This is v2 post. V1 is here:
http://lkml.kernel.org/r/[email protected]

This patchset includes 4 patches. The first three patches are around sparse_add_one_section(). The last one is a simple clean up patch when review codes in hotplug path, carry it in this patchset.

Baoquan He (4):
mm/sparse: Clean up the obsolete code comment
mm/sparse: Optimize sparse_add_one_section()
mm/sparse: Rename function related to section memmap allocation/free drivers/base/memory.c: Rename the misleading parameter

 drivers/base/memory.c |  6 ++---
 mm/sparse.c           | 58 ++++++++++++++++++++++---------------------
 2 files changed, 33 insertions(+), 31 deletions(-)
Signature · 1 line
2.17.2
author
#1 lore raw

PATCH v2 1/4 mm/sparse: Clean up the obsolete code comment

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

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

---
v1-v2:
Add comments to explain what the returned value means for each error code.

 mm/sparse.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
mm/sparse.c +12 -3
index 69904aa6165b..b2111f996aa6 100644
@@ -685,9 +685,18 @@ static void free_map_bootmem(struct page *memmap)
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */
 
 /*
- * returns the number of sections whose mem_maps were properly
- * set.  If this is <=0, then that means that the passed-in
- * map was not consumed and must be freed.
+ * sparse_add_one_section - add a memory section
+ * @nid: The node to add section on
+ * @start_pfn: start pfn of the memory range
+ * @altmap: device page map
+ *
+ * This is only intended for hotplug.
+ *
+ * Returns:
+ *   0 on success.
+ *   Other error code on failure:
+ *     - -EEXIST - section has been present.
+ *     - -ENOMEM - out of memory.
  */
 int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 				     struct vmem_altmap *altmap)
Signature · 1 line
2.17.2
Reviewed-by on v2 1/4
#2 lore raw

Re: [PATCH v2 1/4] mm/sparse: Clean up the obsolete code comment

Quoted 4 lines · Baoquan He

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

Signed-off-by Baoquan He <[email protected]>
Reviewed-by Mike Rapoport <[email protected]>
Quoted 37 lines

---
v1-v2:
Add comments to explain what the returned value means for each error code.

 mm/sparse.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
mm/sparse.c +12 -3
index 69904aa6165b..b2111f996aa6 100644
@@ -685,9 +685,18 @@ static void free_map_bootmem(struct page *memmap)
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */
 
 /*
- * returns the number of sections whose mem_maps were properly
- * set.  If this is <=0, then that means that the passed-in
- * map was not consumed and must be freed.
+ * sparse_add_one_section - add a memory section
+ * @nid: The node to add section on
+ * @start_pfn: start pfn of the memory range
+ * @altmap: device page map
+ *
+ * This is only intended for hotplug.
+ *
+ * Returns:
+ *   0 on success.
+ *   Other error code on failure:
+ *     - -EEXIST - section has been present.
+ *     - -ENOMEM - out of memory.
  */
 int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 				     struct vmem_altmap *altmap)
Signature · 1 line
2.17.2
Signature · 2 lines
Sincerely yours,
Mike.
Acked-by on v2 1/4
#3 lore raw

Re: [PATCH v2 1/4] mm/sparse: Clean up the obsolete code comment

In reply to Baoquan He
Quoted 4 lines · Baoquan He

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

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

Please note that you need /** to start a kernel doc. Other than that.

Acked-by Michal Hocko <[email protected]>
Quoted 37 lines

---
v1-v2:
Add comments to explain what the returned value means for each error code.

 mm/sparse.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
mm/sparse.c +12 -3
index 69904aa6165b..b2111f996aa6 100644
@@ -685,9 +685,18 @@ static void free_map_bootmem(struct page *memmap)
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */
 
 /*
- * returns the number of sections whose mem_maps were properly
- * set.  If this is <=0, then that means that the passed-in
- * map was not consumed and must be freed.
+ * sparse_add_one_section - add a memory section
+ * @nid: The node to add section on
+ * @start_pfn: start pfn of the memory range
+ * @altmap: device page map
+ *
+ * This is only intended for hotplug.
+ *
+ * Returns:
+ *   0 on success.
+ *   Other error code on failure:
+ *     - -EEXIST - section has been present.
+ *     - -ENOMEM - out of memory.
  */
 int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 				     struct vmem_altmap *altmap)
Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko
SUSE Labs
author
#4 lore raw

Re: [PATCH v2 1/4] mm/sparse: Clean up the obsolete code comment

Quoted 7 lines · Michal Hocko
Quoted 4 lines · Baoquan He

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

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

Please note that you need /** to start a kernel doc. Other than that.

I didn't find a template in coding-style.rst, and saw someone is using /*, others use /**. I will use '/**' instead. Thanks for telling.

Quoted 43 lines
Acked-by Michal Hocko <[email protected]>
Quoted 37 lines

---
v1-v2:
Add comments to explain what the returned value means for each error code.

 mm/sparse.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
mm/sparse.c +12 -3
index 69904aa6165b..b2111f996aa6 100644
@@ -685,9 +685,18 @@ static void free_map_bootmem(struct page *memmap)
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */
 
 /*
- * returns the number of sections whose mem_maps were properly
- * set.  If this is <=0, then that means that the passed-in
- * map was not consumed and must be freed.
+ * sparse_add_one_section - add a memory section
+ * @nid: The node to add section on
+ * @start_pfn: start pfn of the memory range
+ * @altmap: device page map
+ *
+ * This is only intended for hotplug.
+ *
+ * Returns:
+ *   0 on success.
+ *   Other error code on failure:
+ *     - -EEXIST - section has been present.
+ *     - -ENOMEM - out of memory.
  */
 int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 				     struct vmem_altmap *altmap)
Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko
SUSE Labs
#5 lore raw

Re: [PATCH v2 1/4] mm/sparse: Clean up the obsolete code comment

Quoted 11 lines · Baoquan He
Quoted 7 lines · Michal Hocko
Quoted 4 lines · Baoquan He

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

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

Please note that you need /** to start a kernel doc. Other than that.

I didn't find a template in coding-style.rst, and saw someone is using /*, others use /**. I will use '/**' instead. Thanks for telling.

How to format kernel-doc comments
---------------------------------

The opening comment mark ``/**`` is used for kernel-doc comments. The ``kernel-doc`` tool will extract comments marked this way. The rest of the comment is formatted like a normal multi-line comment with a column of asterisks on the left side, closing with ``*/`` on a line by itself.

See Documentation/doc-guide/kernel-doc.rst for more details. Hope that can help you.

Thanks,
Chao Fan

Quoted 46 lines
Quoted 44 lines
Acked-by Michal Hocko <[email protected]>
Quoted 37 lines

---
v1-v2:
Add comments to explain what the returned value means for each error code.

 mm/sparse.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
mm/sparse.c +12 -3
index 69904aa6165b..b2111f996aa6 100644
@@ -685,9 +685,18 @@ static void free_map_bootmem(struct page *memmap)
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */
 
 /*
- * returns the number of sections whose mem_maps were properly
- * set.  If this is <=0, then that means that the passed-in
- * map was not consumed and must be freed.
+ * sparse_add_one_section - add a memory section
+ * @nid: The node to add section on
+ * @start_pfn: start pfn of the memory range
+ * @altmap: device page map
+ *
+ * This is only intended for hotplug.
+ *
+ * Returns:
+ *   0 on success.
+ *   Other error code on failure:
+ *     - -EEXIST - section has been present.
+ *     - -ENOMEM - out of memory.
  */
 int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 				     struct vmem_altmap *altmap)
Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko
SUSE Labs
author
#6 lore raw

Re: [PATCH v2 1/4] mm/sparse: Clean up the obsolete code comment

Quoted 16 lines · Chao Fan
Quoted 11 lines · Baoquan He
Quoted 7 lines · Michal Hocko
Quoted 4 lines · Baoquan He

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

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

Please note that you need /** to start a kernel doc. Other than that.

I didn't find a template in coding-style.rst, and saw someone is using /*, others use /**. I will use '/**' instead. Thanks for telling.

How to format kernel-doc comments
---------------------------------

Quoted 7 lines

The opening comment mark ``/**`` is used for kernel-doc comments. The ``kernel-doc`` tool will extract comments marked this way. The rest of the comment is formatted like a normal multi-line comment with a column of asterisks on the left side, closing with ``*/`` on a line by itself.

See Documentation/doc-guide/kernel-doc.rst for more details. Hope that can help you.

Great, there's a specific kernel-doc file. Thanks, I will update and repost this one with '/**'.

#7 lore raw

Re: [PATCH v2 1/4] mm/sparse: Clean up the obsolete code comment

Quoted 22 lines · Baoquan He
Quoted 21 lines · Chao Fan
Quoted 11 lines · Baoquan He
Quoted 7 lines · Michal Hocko
Quoted 4 lines · Baoquan He

The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one.

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

Please note that you need /** to start a kernel doc. Other than that.

I didn't find a template in coding-style.rst, and saw someone is using /*, others use /**. I will use '/**' instead. Thanks for telling.

How to format kernel-doc comments
---------------------------------

The opening comment mark ``/**`` is used for kernel-doc comments. The ``kernel-doc`` tool will extract comments marked this way. The rest of the comment is formatted like a normal multi-line comment with a column of asterisks on the left side, closing with ``*/`` on a line by itself.

Quoted 5 lines
Quoted 2 lines

See Documentation/doc-guide/kernel-doc.rst for more details. Hope that can help you.

Great, there's a specific kernel-doc file. Thanks, I will update and repost this one with '/**'.

In that file, there is also some sample for a function comment:

Function documentation
----------------------

The general format of a function and function-like macro kernel-doc comment is::

/**
* function_name() - Brief description of function.
* @arg1: Describe the first argument.
* @arg2: Describe the second argument.
* One can provide multiple line descriptions
* for arguments.
*
* A longer description, with more discussion of the function function_name()
* that might be useful to those using or modifying it. Begins with an
* empty comment line, and may include additional embedded empty
* comment lines.
*
* The longer description may have multiple paragraphs.
*
* Context: Describes whether the function can sleep, what locks it takes,
* releases, or expects to be held. It can extend over multiple
* lines.
* Return: Describe the return value of function_name.
*
* The return value description can also have multiple paragraphs, and should
* be placed at the end of the comment block.
*/

Anyway, I think you can get more information in that document.

Thanks,
Chao Fan

Quoted 2 lines
author
#8 lore raw

PATCH v2 2/4 mm/sparse: Optimize sparse_add_one_section()

In reply to Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

And also check if section is present earlier. Then don't bother to allocate usemap and memmap if yes.

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

---
v1->v2:
Do section existence checking earlier to further optimize code.

 mm/sparse.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
mm/sparse.c +11 -18
index b2111f996aa6..f4f34d69131e 100644
@@ -714,20 +714,18 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	ret = sparse_index_init(section_nr, nid);
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
-	ret = 0;
-	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
-	if (!memmap)
-		return -ENOMEM;
-	usemap = __kmalloc_section_usemap();
-	if (!usemap) {
-		__kfree_section_memmap(memmap, altmap);
-		return -ENOMEM;
-	}
 
 	ms = __pfn_to_section(start_pfn);
-	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
-		ret = -EEXIST;
-		goto out;
+	if (ms->section_mem_map & SECTION_MARKED_PRESENT)
+		return -EEXIST;
+
+	usemap = __kmalloc_section_usemap();
+	if (!usemap)
+		return -ENOMEM;
+	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
+	if (!memmap) {
+		kfree(usemap);
+		return  -ENOMEM;
 	}
 
 	/*

@@ -739,12 +737,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	section_mark_present(ms);
 	sparse_init_one_section(ms, section_nr, memmap, usemap);
 
-out:
-	if (ret < 0) {
-		kfree(usemap);
-		__kfree_section_memmap(memmap, altmap);
-	}
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
Signature · 1 line
2.17.2
Reviewed-by on v2 2/4
#9 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 9 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

And also check if section is present earlier. Then don't bother to allocate usemap and memmap if yes.

Signed-off-by Baoquan He <[email protected]>
Reviewed-by Mike Rapoport <[email protected]>
Quoted 59 lines

---
v1->v2:
Do section existence checking earlier to further optimize code.

 mm/sparse.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
mm/sparse.c +11 -18
index b2111f996aa6..f4f34d69131e 100644
@@ -714,20 +714,18 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	ret = sparse_index_init(section_nr, nid);
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
-	ret = 0;
-	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
-	if (!memmap)
-		return -ENOMEM;
-	usemap = __kmalloc_section_usemap();
-	if (!usemap) {
-		__kfree_section_memmap(memmap, altmap);
-		return -ENOMEM;
-	}
 
 	ms = __pfn_to_section(start_pfn);
-	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
-		ret = -EEXIST;
-		goto out;
+	if (ms->section_mem_map & SECTION_MARKED_PRESENT)
+		return -EEXIST;
+
+	usemap = __kmalloc_section_usemap();
+	if (!usemap)
+		return -ENOMEM;
+	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
+	if (!memmap) {
+		kfree(usemap);
+		return  -ENOMEM;
 	}
 
 	/*

@@ -739,12 +737,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	section_mark_present(ms);
 	sparse_init_one_section(ms, section_nr, memmap, usemap);
 
-out:
-	if (ret < 0) {
-		kfree(usemap);
-		__kfree_section_memmap(memmap, altmap);
-	}
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
Signature · 1 line
2.17.2
Signature · 2 lines
Sincerely yours,
Mike.
#10 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

In reply to Baoquan He
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

Quoted 2 lines

And also check if section is present earlier. Then don't bother to allocate usemap and memmap if yes.

Moving the check up makes some sense.

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

The patch is not incorrect but I am wondering whether it is really worth it for the current code base. Is it fixing anything real or it is a mere code shuffling to please an eye?

Quoted 59 lines

---
v1->v2:
Do section existence checking earlier to further optimize code.

 mm/sparse.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
mm/sparse.c +11 -18
index b2111f996aa6..f4f34d69131e 100644
@@ -714,20 +714,18 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	ret = sparse_index_init(section_nr, nid);
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
-	ret = 0;
-	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
-	if (!memmap)
-		return -ENOMEM;
-	usemap = __kmalloc_section_usemap();
-	if (!usemap) {
-		__kfree_section_memmap(memmap, altmap);
-		return -ENOMEM;
-	}
 
 	ms = __pfn_to_section(start_pfn);
-	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
-		ret = -EEXIST;
-		goto out;
+	if (ms->section_mem_map & SECTION_MARKED_PRESENT)
+		return -EEXIST;
+
+	usemap = __kmalloc_section_usemap();
+	if (!usemap)
+		return -ENOMEM;
+	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
+	if (!memmap) {
+		kfree(usemap);
+		return  -ENOMEM;
 	}
 
 	/*

@@ -739,12 +737,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	section_mark_present(ms);
 	sparse_init_one_section(ms, section_nr, memmap, usemap);
 
-out:
-	if (ret < 0) {
-		kfree(usemap);
-		__kfree_section_memmap(memmap, altmap);
-	}
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko
SUSE Labs
author
#11 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 9 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

And this make code a little cleaner, e.g the error handling at the end is taken away.

Quoted 11 lines
Quoted 3 lines

And also check if section is present earlier. Then don't bother to allocate usemap and memmap if yes.

Moving the check up makes some sense.

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

The patch is not incorrect but I am wondering whether it is really worth it for the current code base. Is it fixing anything real or it is a mere code shuffling to please an eye?

It's not a fixing, just a tiny code refactorying inside sparse_add_one_section(), seems it doesn't worsen thing if I got the !VMEMMAP case correctly, not quite sure. I am fine to drop it if it's not worth. I could miss something in different cases.

Thanks
Baoquan

Quoted 64 lines
Quoted 60 lines

---
v1->v2:
Do section existence checking earlier to further optimize code.

 mm/sparse.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
mm/sparse.c +11 -18
index b2111f996aa6..f4f34d69131e 100644
@@ -714,20 +714,18 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	ret = sparse_index_init(section_nr, nid);
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
-	ret = 0;
-	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
-	if (!memmap)
-		return -ENOMEM;
-	usemap = __kmalloc_section_usemap();
-	if (!usemap) {
-		__kfree_section_memmap(memmap, altmap);
-		return -ENOMEM;
-	}
 
 	ms = __pfn_to_section(start_pfn);
-	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
-		ret = -EEXIST;
-		goto out;
+	if (ms->section_mem_map & SECTION_MARKED_PRESENT)
+		return -EEXIST;
+
+	usemap = __kmalloc_section_usemap();
+	if (!usemap)
+		return -ENOMEM;
+	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
+	if (!memmap) {
+		kfree(usemap);
+		return  -ENOMEM;
 	}
 
 	/*

@@ -739,12 +737,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	section_mark_present(ms);
 	sparse_init_one_section(ms, section_nr, memmap, usemap);
 
-out:
-	if (ret < 0) {
-		kfree(usemap);
-		__kfree_section_memmap(memmap, altmap);
-	}
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko
SUSE Labs
#12 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 7 lines · Baoquan He
Quoted 6 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Quoted 7 lines
Quoted 3 lines

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

How does it help there? The failure should be even much less probable there because we simply fall back to a small 4kB pages and those essentially never fail.

Quoted 11 lines

And this make code a little cleaner, e.g the error handling at the end is taken away.

Quoted 8 lines
Quoted 3 lines

And also check if section is present earlier. Then don't bother to allocate usemap and memmap if yes.

Moving the check up makes some sense.

Quoted 1 line
Signed-off-by Baoquan He <[email protected]>
Quoted 8 lines
Quoted 3 lines

The patch is not incorrect but I am wondering whether it is really worth it for the current code base. Is it fixing anything real or it is a mere code shuffling to please an eye?

It's not a fixing, just a tiny code refactorying inside sparse_add_one_section(), seems it doesn't worsen thing if I got the !VMEMMAP case correctly, not quite sure. I am fine to drop it if it's not worth. I could miss something in different cases.

Well, I usually prefer to not do micro-optimizations in a code that really begs for a much larger surgery. There are other people working on the code and patches like these might get into the way and cuase conflicts without a very good justification.

Signature · 2 lines
Michal Hocko
SUSE Labs
author
#13 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 12 lines · Michal Hocko
Quoted 11 lines · Baoquan He
Quoted 9 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

Quoted 7 lines
Quoted 3 lines

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

How does it help there? The failure should be even much less probable there because we simply fall back to a small 4kB pages and those essentially never fail.

OK, I am fine to drop it. Or only put the section existence checking earlier to avoid unnecessary usemap/memmap allocation?

From 7594b86ebf5d6fcc8146eca8fc5625f1961a15b1 Mon Sep 17 00:00:00 2001
From: Baoquan He <[email protected]>
Date: Tue, 26 Mar 2019 18:48:39 +0800
Subject: [PATCH] mm/sparse: Check section's existence earlier in
sparse_add_one_section()

No need to allocate usemap and memmap if section has been present. And can clean up the handling on failure.

Signed-off-by Baoquan He <[email protected]>
---
 mm/sparse.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)
mm/sparse.c +8 -13
index 363f9d31b511..f564b531e0f7 100644
@@ -714,7 +714,13 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	ret = sparse_index_init(section_nr, nid);
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
-	ret = 0;
+
+	ms = __pfn_to_section(start_pfn);
+	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
+		ret = -EEXIST;
+		goto out;
+	}
+
 	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
 	if (!memmap)
 		return -ENOMEM;

@@ -724,12 +730,6 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 		return -ENOMEM;
 	}
 
-	ms = __pfn_to_section(start_pfn);
-	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
-		ret = -EEXIST;
-		goto out;
-	}
-
 	/*
 	 * Poison uninitialized struct pages in order to catch invalid flags
 	 * combinations.

@@ -739,12 +739,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	section_mark_present(ms);
 	sparse_init_one_section(ms, section_nr, memmap, usemap);
 
-out:
-	if (ret < 0) {
-		kfree(usemap);
-		__kfree_section_memmap(memmap, altmap);
-	}
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
Signature · 1 line
2.17.2
#14 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 44 lines · Baoquan He
Quoted 12 lines · Michal Hocko
Quoted 11 lines · Baoquan He
Quoted 9 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

Quoted 7 lines
Quoted 3 lines

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

How does it help there? The failure should be even much less probable there because we simply fall back to a small 4kB pages and those essentially never fail.

OK, I am fine to drop it. Or only put the section existence checking earlier to avoid unnecessary usemap/memmap allocation?

From 7594b86ebf5d6fcc8146eca8fc5625f1961a15b1 Mon Sep 17 00:00:00 2001
From: Baoquan He <[email protected]>
Date: Tue, 26 Mar 2019 18:48:39 +0800
Subject: [PATCH] mm/sparse: Check section's existence earlier in
sparse_add_one_section()

No need to allocate usemap and memmap if section has been present. And can clean up the handling on failure.

Signed-off-by Baoquan He <[email protected]>
---
 mm/sparse.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)
mm/sparse.c +0 -0
index 363f9d31b511..f564b531e0f7 100644

mm/sparse.c int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, · line 721

Quoted 9 lines
mm/sparse.c +5 -1
@@ -714,7 +714,13 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	ret = sparse_index_init(section_nr, nid);
 	if (ret < 0 && ret != -EEXIST)
 		return ret;
-	ret = 0;
+
+	ms = __pfn_to_section(start_pfn);
+	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
+		ret = -EEXIST;
+		goto out;

return -EEXIST; ?

Quoted 35 lines
mm/sparse.c +3 -12
+	}
+
 	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
 	if (!memmap)
 		return -ENOMEM;

@@ -724,12 +730,6 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 		return -ENOMEM;
 	}
 
-	ms = __pfn_to_section(start_pfn);
-	if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
-		ret = -EEXIST;
-		goto out;
-	}
-
 	/*
 	 * Poison uninitialized struct pages in order to catch invalid flags
 	 * combinations.

@@ -739,12 +739,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	section_mark_present(ms);
 	sparse_init_one_section(ms, section_nr, memmap, usemap);
 
-out:
-	if (ret < 0) {
-		kfree(usemap);
-		__kfree_section_memmap(memmap, altmap);
-	}
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
Signature · 1 line
2.17.2
Signature · 2 lines
Sincerely yours,
Mike.
#15 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

In reply to Baoquan He
Quoted 17 lines · Baoquan He
Quoted 16 lines · Michal Hocko
Quoted 14 lines · Baoquan He
Quoted 9 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

Quoted 6 lines
Quoted 3 lines

How does it help there? The failure should be even much less probable there because we simply fall back to a small 4kB pages and those essentially never fail.

OK, I am fine to drop it. Or only put the section existence checking earlier to avoid unnecessary usemap/memmap allocation?

DO you have any data on how often that happens? Should basically never happening, right?

Signature · 2 lines
Michal Hocko
SUSE Labs
author
#16 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 22 lines · Michal Hocko
Quoted 21 lines · Baoquan He
Quoted 19 lines · Michal Hocko
Quoted 7 lines · Baoquan He
Quoted 6 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Quoted 7 lines
Quoted 3 lines

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

How does it help there? The failure should be even much less probable there because we simply fall back to a small 4kB pages and those essentially never fail.

Quoted 5 lines
Quoted 2 lines

OK, I am fine to drop it. Or only put the section existence checking earlier to avoid unnecessary usemap/memmap allocation?

DO you have any data on how often that happens? Should basically never happening, right?

Oh, you think about it in this aspect. Yes, it rarely happens. Always allocating firstly can increase efficiency. Then I will just drop it.

#17 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Quoted 26 lines · Baoquan He
Quoted 25 lines · Michal Hocko
Quoted 23 lines · Baoquan He
Quoted 12 lines · Michal Hocko
Quoted 11 lines · Baoquan He
Quoted 9 lines · Michal Hocko
Quoted 4 lines · Baoquan He

Reorder the allocation of usemap and memmap since usemap allocation is much simpler and easier. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure.

Is this really worth it? I can see that !VMEMMAP is doing memmap size allocation which would be 2MB aka costly allocation but we do not do __GFP_RETRY_MAYFAIL so the allocator backs off early.

Quoted 7 lines
Quoted 3 lines

In !VMEMMAP case, it truly does simple allocation directly. surely usemap which size is 32 is smaller. So it doesn't matter that much who's ahead or who's behind. However, this benefit a little in VMEMMAP case.

How does it help there? The failure should be even much less probable there because we simply fall back to a small 4kB pages and those essentially never fail.

OK, I am fine to drop it. Or only put the section existence checking earlier to avoid unnecessary usemap/memmap allocation?

Quoted 6 lines
Quoted 2 lines

DO you have any data on how often that happens? Should basically never happening, right?

Oh, you think about it in this aspect. Yes, it rarely happens. Always allocating firstly can increase efficiency. Then I will just drop it.

OK, let me try once more. Doing a check early is something that makes sense in general. Another question is whether the check is needed at all. So rather than fiddling with its placement I would go whether it is actually failing at all. I suspect it doesn't because the memory hotplug is currently enforced to be section aligned. There are people who would like to allow subsection or section unaligned aware hotplug and then this would be much more relevant but without any solid justification such a patch is not really helpful because it might cause code conflicts with other work or obscure the git blame tracking by an additional hop.

In short, if you want to optimize something then make sure you describe what you are optimizing how it helps.

Signature · 2 lines
Michal Hocko
SUSE Labs
author
#18 lore raw

Re: [PATCH v2 2/4] mm/sparse: Optimize sparse_add_one_section()

Hi Michal,

Quoted 20 lines · Michal Hocko
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

OK, I am fine to drop it. Or only put the section existence checking earlier to avoid unnecessary usemap/memmap allocation?

DO you have any data on how often that happens? Should basically never happening, right?

Oh, you think about it in this aspect. Yes, it rarely happens. Always allocating firstly can increase efficiency. Then I will just drop it.

OK, let me try once more. Doing a check early is something that makes sense in general. Another question is whether the check is needed at all. So rather than fiddling with its placement I would go whether it is actually failing at all. I suspect it doesn't because the memory hotplug is currently enforced to be section aligned. There are people who would like to allow subsection or section unaligned aware hotplug and then this would be much more relevant but without any solid justification such a patch is not really helpful because it might cause code conflicts with other work or obscure the git blame tracking by an additional hop.

Quoted 2 lines

In short, if you want to optimize something then make sure you describe what you are optimizing how it helps.

I must be dizzy last night when thinking and replying mails, I thought about it a while, got a point you may mean. Now when I check mail and rethink about it, that reply may make misunderstanding. It doesn't actually makes sense to optimize, just a little code block moving. I now agree with you that it doesn't optimize anything and may impact people's code change. Sorry about that.

Thanks
Baoquan

author
#19 lore raw

PATCH v2 3/4 mm/sparse: Rename function related to section memmap allocation/free

In reply to Baoquan He

These functions are used to allocate/free section memmap, have nothing to do with kmalloc/free during the handling. Rename them to remove the confusion.

Signed-off-by Baoquan He <[email protected]>
Acked-by Mike Rapoport <[email protected]>
---
 mm/sparse.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
mm/sparse.c +8 -8
index f4f34d69131e..68a89d133fa7 100644
@@ -590,13 +590,13 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
 #endif
 
 #ifdef CONFIG_SPARSEMEM_VMEMMAP
-static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
+static inline struct page *alloc_section_memmap(unsigned long pnum, int nid,
 		struct vmem_altmap *altmap)
 {
 	/* This will make the necessary allocations eventually. */
 	return sparse_mem_map_populate(pnum, nid, altmap);
 }
-static void __kfree_section_memmap(struct page *memmap,
+static void __free_section_memmap(struct page *memmap,
 		struct vmem_altmap *altmap)
 {
 	unsigned long start = (unsigned long)memmap;

@@ -614,7 +614,7 @@ static void free_map_bootmem(struct page *memmap)
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 #else
-static struct page *__kmalloc_section_memmap(void)
+static struct page *__alloc_section_memmap(void)
 {
 	struct page *page, *ret;
 	unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION;

@@ -635,13 +635,13 @@ static struct page *__kmalloc_section_memmap(void)
 	return ret;
 }
 
-static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
+static inline struct page *alloc_section_memmap(unsigned long pnum, int nid,
 		struct vmem_altmap *altmap)
 {
-	return __kmalloc_section_memmap();
+	return __alloc_section_memmap();
 }
 
-static void __kfree_section_memmap(struct page *memmap,
+static void __free_section_memmap(struct page *memmap,
 		struct vmem_altmap *altmap)
 {
 	if (is_vmalloc_addr(memmap))

@@ -722,7 +722,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
 	usemap = __kmalloc_section_usemap();
 	if (!usemap)
 		return -ENOMEM;
-	memmap = kmalloc_section_memmap(section_nr, nid, altmap);
+	memmap = alloc_section_memmap(section_nr, nid, altmap);
 	if (!memmap) {
 		kfree(usemap);
 		return  -ENOMEM;

@@ -786,7 +786,7 @@ static void free_section_usemap(struct page *memmap, unsigned long *usemap,
 	if (PageSlab(usemap_page) || PageCompound(usemap_page)) {
 		kfree(usemap);
 		if (memmap)
-			__kfree_section_memmap(memmap, altmap);
+			__free_section_memmap(memmap, altmap);
 		return;
 	}
Signature · 1 line
2.17.2
author
#20 lore raw

PATCH v2 4/4 drivers/base/memory.c: Rename the misleading parameter

In reply to Baoquan He

The input parameter 'phys_index' of memory_block_action() is actually the section number, but not the phys_index of memory_block. Fix it.

Signed-off-by Baoquan He <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
---
 drivers/base/memory.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/base/memory.c +3 -3
index cb8347500ce2..184f4f8d1b62 100644
@@ -231,13 +231,13 @@ static bool pages_correctly_probed(unsigned long start_pfn)
  * OK to have direct references to sparsemem variables in here.
  */
 static int
-memory_block_action(unsigned long phys_index, unsigned long action, int online_type)
+memory_block_action(unsigned long sec, unsigned long action, int online_type)
 {
 	unsigned long start_pfn;
 	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
 	int ret;
 
-	start_pfn = section_nr_to_pfn(phys_index);
+	start_pfn = section_nr_to_pfn(sec);
 
 	switch (action) {
 	case MEM_ONLINE:

@@ -251,7 +251,7 @@ memory_block_action(unsigned long phys_index, unsigned long action, int online_t
 		break;
 	default:
 		WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: "
-		     "%ld\n", __func__, phys_index, action, action);
+		     "%ld\n", __func__, sec, action, action);
 		ret = -EINVAL;
 	}
Signature · 1 line
2.17.2
maintainer Reviewed-by on v2 4/4
#21 lore raw

Re: [PATCH v2 4/4] drivers/base/memory.c: Rename the misleading parameter

Quoted 7 lines · Baoquan He

The input parameter 'phys_index' of memory_block_action() is actually the section number, but not the phys_index of memory_block. Fix it.

Signed-off-by Baoquan He <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Reviewed-by Rafael J. Wysocki <[email protected]>
Quoted 36 lines
---
 drivers/base/memory.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/base/memory.c +3 -4
index cb8347500ce2..184f4f8d1b62 100644
@@ -231,13 +231,13 @@ static bool pages_correctly_probed(unsigned long start_pfn)
  * OK to have direct references to sparsemem variables in here.
  */
 static int
-memory_block_action(unsigned long phys_index, unsigned long action, int online_type)
+memory_block_action(unsigned long sec, unsigned long action, int online_type)
 {
        unsigned long start_pfn;
        unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
        int ret;
 
-       start_pfn = section_nr_to_pfn(phys_index);
+       start_pfn = section_nr_to_pfn(sec);
 
        switch (action) {
        case MEM_ONLINE:

@@ -251,7 +251,7 @@ memory_block_action(unsigned long phys_index, unsigned long action, int online_t
                break;
        default:
                WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: "
-                    "%ld\n", __func__, phys_index, action, action);
+                    "%ld\n", __func__, sec, action, action);
                ret = -EINVAL;
        }
 
--

2.17.2

Acked-by on v2 4/4
#22 lore raw

Re: [PATCH v2 4/4] drivers/base/memory.c: Rename the misleading parameter

In reply to Baoquan He
Quoted 2 lines · Baoquan He

The input parameter 'phys_index' of memory_block_action() is actually the section number, but not the phys_index of memory_block. Fix it.

phys_index is a relict from the past and it indeed denotes the section number which is exported as phys_index via sysfs. start_section_nr would be a better name IMHO but nothing to really bike shed about.

Quoted 3 lines
Signed-off-by Baoquan He <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Acked-by Michal Hocko <[email protected]>
Quoted 36 lines
---
 drivers/base/memory.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/base/memory.c +3 -3
index cb8347500ce2..184f4f8d1b62 100644
@@ -231,13 +231,13 @@ static bool pages_correctly_probed(unsigned long start_pfn)
  * OK to have direct references to sparsemem variables in here.
  */
 static int
-memory_block_action(unsigned long phys_index, unsigned long action, int online_type)
+memory_block_action(unsigned long sec, unsigned long action, int online_type)
 {
 	unsigned long start_pfn;
 	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
 	int ret;
 
-	start_pfn = section_nr_to_pfn(phys_index);
+	start_pfn = section_nr_to_pfn(sec);
 
 	switch (action) {
 	case MEM_ONLINE:

@@ -251,7 +251,7 @@ memory_block_action(unsigned long phys_index, unsigned long action, int online_t
 		break;
 	default:
 		WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: "
-		     "%ld\n", __func__, phys_index, action, action);
+		     "%ld\n", __func__, sec, action, action);
 		ret = -EINVAL;
 	}
Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko
SUSE Labs
#23 lore raw

Re: [PATCH v2 4/4] drivers/base/memory.c: Rename the misleading parameter

In reply to Baoquan He

drivers/base/memory.c static bool pages_correctly_probed(unsigned long start_pfn) · line 234

Quoted 6 lines · Baoquan He

The input parameter 'phys_index' of memory_block_action() is actually the section number, but not the phys_index of memory_block. Fix it.

static int

drivers/base/memory.c +1 -1
-memory_block_action(unsigned long phys_index, unsigned long action, int online_type)
+memory_block_action(unsigned long sec, unsigned long action, int online_type)

'sec' is a bad abbreviation for 'section'. We don't use it anyhere else in the vm.

Looking through include/, I see it used as an abbreviation for second, security, ELF section, and section of a book. Nowhere as a memory block section. Please use an extra four letters for this parameter.

author
#24 lore raw

Re: [PATCH v2 4/4] drivers/base/memory.c: Rename the misleading parameter

Quoted 10 lines · Matthew Wilcox

drivers/base/memory.c static bool pages_correctly_probed(unsigned long start_pfn) · line 234

Quoted 6 lines · Baoquan He

The input parameter 'phys_index' of memory_block_action() is actually the section number, but not the phys_index of memory_block. Fix it.

static int

drivers/base/memory.c +1 -1
-memory_block_action(unsigned long phys_index, unsigned long action, int online_type)
+memory_block_action(unsigned long sec, unsigned long action, int online_type)

'sec' is a bad abbreviation for 'section'. We don't use it anyhere else in the vm.

Hmm, here 'sec' is in a particular context, we may not confuse it with other abbreviation. Since Michal also complained about it, seems an update is needed. I will change it to start_section_nr as Michal suggested. Thanks.

Quoted 4 lines

Looking through include/, I see it used as an abbreviation for second, security, ELF section, and section of a book. Nowhere as a memory block section. Please use an extra four letters for this parameter.

author
#25 lore raw
In reply to Baoquan He

Talked to Michal, the local code refactorying may impact those big feature or improvement patchset, e.g patch 2/4 and patch 3/4 will conflict with Dan's patchset:
[PATCH v5 00/10] mm: Sub-section memory hotplug support

So I would like to discard them and only repost patch 1/4 and 4/4 after addressing reviewers' concern. Sorry for the noise.

Quoted 20 lines · Baoquan He

This is v2 post. V1 is here:
http://lkml.kernel.org/r/[email protected]

This patchset includes 4 patches. The first three patches are around sparse_add_one_section(). The last one is a simple clean up patch when review codes in hotplug path, carry it in this patchset.

Baoquan He (4):
mm/sparse: Clean up the obsolete code comment
mm/sparse: Optimize sparse_add_one_section()
mm/sparse: Rename function related to section memmap allocation/free drivers/base/memory.c: Rename the misleading parameter

 drivers/base/memory.c |  6 ++---
 mm/sparse.c           | 58 ++++++++++++++++++++++---------------------
 2 files changed, 33 insertions(+), 31 deletions(-)
Signature · 1 line
2.17.2