[PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[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 <bhe@redhat.com>
---
v2->v3:
  Normalize the code comment to use '/**' at 1st line of doc
  above function.
v1-v2:
  Add comments to explain what the returned value means for
  each error code.
 mm/sparse.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index 69904aa6165b..363f9d31b511 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -684,10 +684,19 @@ static void free_map_bootmem(struct page *memmap)
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 #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)
-- 
2.17.2

[PATCH v3 2/2] drivers/base/memory.c: Rename the misleading parameter

[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 <bhe@redhat.com>
---
v2->v3:
  Rename the parameter to 'start_section_nr' from 'sec'.

 drivers/base/memory.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index cb8347500ce2..9ea972b2ae79 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -231,13 +231,14 @@ 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 start_section_nr, 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(start_section_nr);
 
 	switch (action) {
 	case MEM_ONLINE:
@@ -251,7 +252,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__, start_section_nr, action, action);
 		ret = -EINVAL;
 	}
 
-- 
2.17.2

Re: [PATCH v3 2/2] drivers/base/memory.c: Rename the misleading parameter

[Michal Hocko]

On Fri 29-03-19 16:29:15, Baoquan He wrote:
> The input parameter 'phys_index' of memory_block_action() is actually
> the section number, but not the phys_index of memory_block. Fix it.

I have tried to explain that the naming is mostly a relict from the past
than really a misleading name http://lkml.kernel.org/r/20190326093315.GL28406@dhcp22.suse.cz
Maybe it would be good to reflect that in the changelog
 
> Signed-off-by: Baoquan He <bhe@redhat.com>

btw. I've acked the previous version as well.

> ---
> v2->v3:
>   Rename the parameter to 'start_section_nr' from 'sec'.
> 
>  drivers/base/memory.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index cb8347500ce2..9ea972b2ae79 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -231,13 +231,14 @@ 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 start_section_nr, 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(start_section_nr);
>  
>  	switch (action) {
>  	case MEM_ONLINE:
> @@ -251,7 +252,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__, start_section_nr, action, action);
>  		ret = -EINVAL;
>  	}
>  
> -- 
> 2.17.2
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 2/2] drivers/base/memory.c: Rename the misleading parameter

[Baoquan He]

On 03/29/19 at 10:13am, Michal Hocko wrote:
> On Fri 29-03-19 16:29:15, Baoquan He wrote:
> > The input parameter 'phys_index' of memory_block_action() is actually
> > the section number, but not the phys_index of memory_block. Fix it.
> 
> I have tried to explain that the naming is mostly a relict from the past
> than really a misleading name http://lkml.kernel.org/r/20190326093315.GL28406@dhcp22.suse.cz
> Maybe it would be good to reflect that in the changelog
>  
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> 
> btw. I've acked the previous version as well.

Sure, will rewrite the log and add people's Acked-by tag. Thanks.

> 
> > ---
> > v2->v3:
> >   Rename the parameter to 'start_section_nr' from 'sec'.
> > 
> >  drivers/base/memory.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> > index cb8347500ce2..9ea972b2ae79 100644
> > --- a/drivers/base/memory.c
> > +++ b/drivers/base/memory.c
> > @@ -231,13 +231,14 @@ 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 start_section_nr, 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(start_section_nr);
> >  
> >  	switch (action) {
> >  	case MEM_ONLINE:
> > @@ -251,7 +252,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__, start_section_nr, action, action);
> >  		ret = -EINVAL;
> >  	}
> >  
> > -- 
> > 2.17.2
> > 
> 
> -- 
> Michal Hocko
> SUSE Labs

Re: [PATCH v3 2/2] drivers/base/memory.c: Rename the misleading parameter

[Oscar Salvador]

On Fri, Mar 29, 2019 at 10:13:25AM +0100, Michal Hocko wrote:
> On Fri 29-03-19 16:29:15, Baoquan He wrote:
> > The input parameter 'phys_index' of memory_block_action() is actually
> > the section number, but not the phys_index of memory_block. Fix it.
> 
> I have tried to explain that the naming is mostly a relict from the past
> than really a misleading name http://lkml.kernel.org/r/20190326093315.GL28406@dhcp22.suse.cz
> Maybe it would be good to reflect that in the changelog

I think that phys_device variable in remove_memory_section() is also a relict
from the past, and it is no longer used.
Neither node_id variable is used.
Actually, unregister_memory_section() sets those two to 0 no matter what.

Since we are cleaning up, I wonder if we can go a bit further and we can get
rid of that as well.

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 2/2] drivers/base/memory.c: Rename the misleading parameter

[Baoquan He]

On 03/29/19 at 10:37am, Oscar Salvador wrote:
> On Fri, Mar 29, 2019 at 10:13:25AM +0100, Michal Hocko wrote:
> > On Fri 29-03-19 16:29:15, Baoquan He wrote:
> > > The input parameter 'phys_index' of memory_block_action() is actually
> > > the section number, but not the phys_index of memory_block. Fix it.
> > 
> > I have tried to explain that the naming is mostly a relict from the past
> > than really a misleading name http://lkml.kernel.org/r/20190326093315.GL28406@dhcp22.suse.cz
> > Maybe it would be good to reflect that in the changelog
> 
> I think that phys_device variable in remove_memory_section() is also a relict
> from the past, and it is no longer used.
> Neither node_id variable is used.
> Actually, unregister_memory_section() sets those two to 0 no matter what.
> 
> Since we are cleaning up, I wonder if we can go a bit further and we can get
> rid of that as well.

Yes, certainly. I would like to post a new one to carry this.

[PATCH v4 2/2] drivers/base/memory.c: Rename the misleading parameter

[Baoquan He]

The input parameter 'phys_index' of memory_block_action() is actually
the section number, but not the phys_index of memory_block. This is
a relict from the past when one memory block could only contain one
section.

Rename it to start_section_nr.

Signed-off-by: Baoquan He <bhe@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/memory.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index cb8347500ce2..9ea972b2ae79 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -231,13 +231,14 @@ 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 start_section_nr, 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(start_section_nr);
 
 	switch (action) {
 	case MEM_ONLINE:
@@ -251,7 +252,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__, start_section_nr, action, action);
 		ret = -EINVAL;
 	}
 
-- 
2.17.2

Re: [PATCH v4 2/2] drivers/base/memory.c: Rename the misleading parameter

[Oscar Salvador]

On Fri, Mar 29, 2019 at 05:36:59PM +0800, Baoquan He wrote:
> The input parameter 'phys_index' of memory_block_action() is actually
> the section number, but not the phys_index of memory_block. This is
> a relict from the past when one memory block could only contain one
> section.
> 
> Rename it to start_section_nr.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>


> ---
>  drivers/base/memory.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index cb8347500ce2..9ea972b2ae79 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -231,13 +231,14 @@ 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 start_section_nr, 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(start_section_nr);
>  
>  	switch (action) {
>  	case MEM_ONLINE:
> @@ -251,7 +252,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__, start_section_nr, action, action);
>  		ret = -EINVAL;
>  	}
>  
> -- 
> 2.17.2
> 

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v4 2/2] drivers/base/memory.c: Rename the misleading parameter

[Mukesh Ojha]

On 3/29/2019 3:06 PM, Baoquan He wrote:
> The input parameter 'phys_index' of memory_block_action() is actually
> the section number, but not the phys_index of memory_block. This is
> a relict from the past when one memory block could only contain one
> section.
>
> Rename it to start_section_nr.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>


Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh

> ---
>   drivers/base/memory.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index cb8347500ce2..9ea972b2ae79 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -231,13 +231,14 @@ 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 start_section_nr, 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(start_section_nr);
>   
>   	switch (action) {
>   	case MEM_ONLINE:
> @@ -251,7 +252,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__, start_section_nr, action, action);
>   		ret = -EINVAL;
>   	}
>   

Re: [PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[Michal Hocko]

On Fri 29-03-19 16:29:14, Baoquan He wrote:
> The code comment above sparse_add_one_section() is obsolete and
> incorrect, clean it up and write new one.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
> v2->v3:
>   Normalize the code comment to use '/**' at 1st line of doc
>   above function.
> v1-v2:
>   Add comments to explain what the returned value means for
>   each error code.
>  mm/sparse.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 69904aa6165b..363f9d31b511 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -684,10 +684,19 @@ static void free_map_bootmem(struct page *memmap)
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>  #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)
> -- 
> 2.17.2
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[Oscar Salvador]

On Fri, Mar 29, 2019 at 04:29:14PM +0800, Baoquan He wrote:
> The code comment above sparse_add_one_section() is obsolete and
> incorrect, clean it up and write new one.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
> v2->v3:
>   Normalize the code comment to use '/**' at 1st line of doc
>   above function.
> v1-v2:
>   Add comments to explain what the returned value means for
>   each error code.
>  mm/sparse.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 69904aa6165b..363f9d31b511 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -684,10 +684,19 @@ static void free_map_bootmem(struct page *memmap)
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>  #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.

I am not really into kernel-doc format, but I thought it was something like:

<--
Return:
  0: success
  -EEXIST: Section is already present
  -ENOMEM: Out of memory
-->

But as I said, I might very well be wrong.

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[Baoquan He]

On 03/29/19 at 11:36am, Oscar Salvador wrote:
> > +/**
> > + * 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.
> 
> I am not really into kernel-doc format, but I thought it was something like:
> 
> <--
> Return:
>   0: success
>   -EEXIST: Section is already present
>   -ENOMEM: Out of memory
> -->
> 
> But as I said, I might very well be wrong.

Below is excerpt from doc-guide/kernel-doc.rst. Seems they suggest it
like this if format returned values with multi-line style. While the
format is not strictly defined. I will use it to update.

*Return:
* * 0		- Success
* * -EEXIST 	- Section is already present
* * -ENOMEM	- Out of memory

The return value, if any, should be described in a dedicated section
named ``Return``.

.. note::

  #) The multi-line descriptive text you provide does *not* recognize
     line breaks, so if you try to format some text nicely, as in::

        * Return:
        * 0 - OK
        * -EINVAL - invalid argument
        * -ENOMEM - out of memory

     this will all run together and produce::

        Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory

     So, in order to produce the desired line breaks, you need to use a
     ReST list, e. g.::                                                                                                                           

      * Return:
      * * 0             - OK to runtime suspend the device
      * * -EBUSY        - Device should not be runtime suspended

Re: [PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[Mukesh Ojha]

On 3/29/2019 1:59 PM, Baoquan He wrote:
> The code comment above sparse_add_one_section() is obsolete and
> incorrect, clean it up and write new one.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>


Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh

> ---
> v2->v3:
>    Normalize the code comment to use '/**' at 1st line of doc
>    above function.
> v1-v2:
>    Add comments to explain what the returned value means for
>    each error code.
>   mm/sparse.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 69904aa6165b..363f9d31b511 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -684,10 +684,19 @@ static void free_map_bootmem(struct page *memmap)
>   #endif /* CONFIG_MEMORY_HOTREMOVE */
>   #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)

Re: [PATCH v3 1/2] mm/sparse: Clean up the obsolete code comment

[Mike Rapoport]

On Fri, Mar 29, 2019 at 04:29:14PM +0800, Baoquan He wrote:
> The code comment above sparse_add_one_section() is obsolete and
> incorrect, clean it up and write new one.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>

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

> ---
> v2->v3:
>   Normalize the code comment to use '/**' at 1st line of doc
>   above function.
> v1-v2:
>   Add comments to explain what the returned value means for
>   each error code.
>  mm/sparse.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 69904aa6165b..363f9d31b511 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -684,10 +684,19 @@ static void free_map_bootmem(struct page *memmap)
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>  #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)
> -- 
> 2.17.2
> 

-- 
Sincerely yours,
Mike.