[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87wmpeyx70.fsf@nvidia.com>
Date: Wed, 03 Apr 2024 13:56:06 -0700
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>
Cc: Jakub Kicinski <kuba@...nel.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-doc@...r.kernel.org"
<linux-doc@...r.kernel.org>, "Zaki, Ahmed" <ahmed.zaki@...el.com>,
"Lobakin, Aleksander" <aleksander.lobakin@...el.com>,
"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
"andrew@...n.ch" <andrew@...n.ch>, "cjubran@...dia.com"
<cjubran@...dia.com>, "corbet@....net" <corbet@....net>,
"davem@...emloft.net" <davem@...emloft.net>, "dtatulea@...dia.com"
<dtatulea@...dia.com>, "edumazet@...gle.com" <edumazet@...gle.com>,
"gal@...dia.com" <gal@...dia.com>, "hkallweit1@...il.com"
<hkallweit1@...il.com>, "jiri@...nulli.us" <jiri@...nulli.us>,
"joabreu@...opsys.com" <joabreu@...opsys.com>, "justinstitt@...gle.com"
<justinstitt@...gle.com>, "kory.maincent@...tlin.com"
<kory.maincent@...tlin.com>, "leon@...nel.org" <leon@...nel.org>,
"liuhangbin@...il.com" <liuhangbin@...il.com>,
"maxime.chevallier@...tlin.com" <maxime.chevallier@...tlin.com>,
"pabeni@...hat.com" <pabeni@...hat.com>, "Greenwalt, Paul"
<paul.greenwalt@...el.com>, "Kitszel, Przemyslaw"
<przemyslaw.kitszel@...el.com>, "rdunlap@...radead.org"
<rdunlap@...radead.org>, "richardcochran@...il.com"
<richardcochran@...il.com>, "saeed@...nel.org" <saeed@...nel.org>,
"tariqt@...dia.com" <tariqt@...dia.com>, "vadim.fedorenko@...ux.dev"
<vadim.fedorenko@...ux.dev>, "vladimir.oltean@....com"
<vladimir.oltean@....com>, "Drewek, Wojciech" <wojciech.drewek@...el.com>
Subject: Re: [PATCH net-next v1 1/6] ethtool: add interface to read Tx
hardware timestamping statistics
On Wed, 03 Apr, 2024 18:44:52 +0000 "Keller, Jacob E" <jacob.e.keller@...el.com> wrote:
>> On Tue, 02 Apr, 2024 19:18:42 -0700 Jakub Kicinski <kuba@...nel.org> 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
Powered by blists - more mailing lists