On Wed, 03 Apr, 2024 18:44:52 +0000 "Keller, Jacob E" <jacob.e.keller@xxxxxxxxx> wrote: >> On Tue, 02 Apr, 2024 19:18:42 -0700 Jakub Kicinski <kuba@xxxxxxxxxx> wrote: >> > On Tue, 2 Apr 2024 13:52:01 -0700 Rahul Rameshbabu wrote: >> >> +/** >> >> + * struct ethtool_ts_stats - HW timestamping statistics >> >> + * @tx_stats: struct group for TX HW timestamping >> >> + * @pkts: Number of packets successfully timestamped by the hardware. >> >> + * @lost: Number of hardware timestamping requests where the >> timestamping >> >> + * information from the hardware never arrived for submission with >> >> + * the skb. >> >> + * @err: Number of arbitrary timestamp generation error events that the >> >> + * hardware encountered, exclusive of @lost statistics. Cases such >> >> + * as resource exhaustion, unavailability, firmware errors, and >> >> + * detected illogical timestamp values not submitted with the skb >> >> + * are inclusive to this counter. >> >> + */ >> >> +struct ethtool_ts_stats { >> >> + struct_group(tx_stats, >> > >> > Doesn't seem like the group should be documented: >> > >> > include/linux/ethtool.h:503: warning: Excess struct member 'tx_stats' >> description in 'ethtool_ts_stats' >> >> Was looking into why our internal verification did not catch this. We >> run W=1 with clang, but looks like the warning does not get triggered >> unless explicitly run with scripts/kernel-doc. >> >> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to- >> format-kernel-doc-comments >> >> I have debugged using strace that the way the kernel doc checking works >> when W=1 is set is that the matching source file that is being compiled >> is passed to scripts/kernel-doc, so include files are missed from the >> doc check. I think this is worth adding to the kernel documentation. >> > > It would be great if the W=1 setup could figure out the include files and send > those to kernel-doc too, but I'm not sure if this is possible and if so how > difficult it would be to implement it. A lot of headers produce warnings because > a lot fewer people manually run kernel-doc on the entire source. > I took a look into this, and the one naive solution I had in mind was a checkdocs target for the kernel where you use gcc -MM to deduce all the includes, create a unique list, and then run scripts/kernel-doc against the list of include files. That said, I do think this is excessive compared to having a checkpatch logic that runs scripts/kernel-doc on the parts that change in your commit before and after the patch is applied. Kudos to the netdev CI for having this. https://github.com/linux-netdev/nipa/blob/main/tests/patch/kdoc/kdoc.sh