| Message ID | 20190326090227.3059-2-bhe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up comments and codes in sparse_add_one_section() | expand |
On Tue, Mar 26, 2019 at 05:02:24PM +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> > --- > 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(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 69904aa6165b..b2111f996aa6 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -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) > -- > 2.17.2 >
On Tue 26-03-19 17:02:24, 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> Please note that you need /** to start a kernel doc. Other than that. Acked-by: Michal Hocko <mhocko@suse.com> > --- > 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(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 69904aa6165b..b2111f996aa6 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -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) > -- > 2.17.2 >
On 03/26/19 at 10:23am, Michal Hocko wrote: > On Tue 26-03-19 17:02:24, 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> > > 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. > > Acked-by: Michal Hocko <mhocko@suse.com> > > --- > > 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(-) > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > index 69904aa6165b..b2111f996aa6 100644 > > --- a/mm/sparse.c > > +++ b/mm/sparse.c > > @@ -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) > > -- > > 2.17.2 > > > > -- > Michal Hocko > SUSE Labs
On Tue, Mar 26, 2019 at 05:30:57PM +0800, Baoquan He wrote: >On 03/26/19 at 10:23am, Michal Hocko wrote: >> On Tue 26-03-19 17:02:24, 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> >> >> 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 > >> >> Acked-by: Michal Hocko <mhocko@suse.com> >> > --- >> > 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(-) >> > >> > diff --git a/mm/sparse.c b/mm/sparse.c >> > index 69904aa6165b..b2111f996aa6 100644 >> > --- a/mm/sparse.c >> > +++ b/mm/sparse.c >> > @@ -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) >> > -- >> > 2.17.2 >> > >> >> -- >> Michal Hocko >> SUSE Labs > >
On 03/26/19 at 05:36pm, Chao Fan wrote: > On Tue, Mar 26, 2019 at 05:30:57PM +0800, Baoquan He wrote: > >On 03/26/19 at 10:23am, Michal Hocko wrote: > >> On Tue 26-03-19 17:02:24, 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> > >> > >> 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. Great, there's a specific kernel-doc file. Thanks, I will update and repost this one with '/**'.
On Tue, Mar 26, 2019 at 05:43:48PM +0800, Baoquan He wrote: >On 03/26/19 at 05:36pm, Chao Fan wrote: >> On Tue, Mar 26, 2019 at 05:30:57PM +0800, Baoquan He wrote: >> >On 03/26/19 at 10:23am, Michal Hocko wrote: >> >> On Tue 26-03-19 17:02:24, 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> >> >> >> >> 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. > >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 > >
diff --git a/mm/sparse.c b/mm/sparse.c index 69904aa6165b..b2111f996aa6 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -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)
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> --- 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(-)