Re: [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.

From: Thiago Jung Bauermann
Date: Thu Jun 23 2016 - 11:38:07 EST


Am Donnerstag, 23 Juni 2016, 01:44:07 schrieb Dave Young:
> Hmm, hold on. For declaring a struct in a header file, comment should be
> just after each fields, like below, your format is for a function instead:
> struct pci_slot {
> struct pci_bus *bus; /* The bus this slot is on */
> struct list_head list; /* node in list of slots on this
> bus */ struct hotplug_slot *hotplug; /* Hotplug info (migrate over
> time) */ unsigned char number; /* PCI_SLOT(pci_dev->devfn) */
> struct kobject kobj;
> };

The comment style you mention above is not extractable documentation. The
style I used is what is described in section "kernel-doc for structs,
unions, enums, and typedefs" in Documentation/kernel-doc-nano-HOWTO.txt.

> BTW, what is @size? there's no size field in kexec_buf. I think it is not
> necessary to add these comment, they are easy to understand. If you really
> want, please rewrite them correctly, for example "image" description is
> wrong. It is not only for searching memory only, top_down description is
> also bad.

Sorry, I moved these comments from kexec_locate_mem_hole but forgot to
rename the parameters to what they are called in struct kexec_buf. @size
should have been @memsz (other fields also have wrong names, I'll fix them
as well). The image description is correct in the context of where struct
kexec_buf is used and explains what it will be used for in the function
taking kexec_buf as an argument. It is not meant as a general description of
the purpose of struct kimage. What is bad about the description of top_down?

I decided to add these comments because struct kexec_buf is now part of the
kernel API for kexec. kernel-doc-nano-HOWTO.txt says:

> We definitely need kernel-doc formatted documentation for functions
> that are exported to loadable modules using EXPORT_SYMBOL.
>
> We also look to provide kernel-doc formatted documentation for
> functions externally visible to other kernel files (not marked
> "static").
>
> We also recommend providing kernel-doc formatted documentation
> for private (file "static") routines, for consistency of kernel
> source code layout. But this is lower priority and at the
> discretion of the MAINTAINER of that kernel source file.

If you think they are not necessary or just add clutter I can leave them
out.

--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center