linux-kernel - Re: [PATCH] tracing: fix trace.h kernel-doc func/struct/enum members

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250113161646.6889e310@gandalf.local.home>
Date: Mon, 13 Jan 2025 16:16:46 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: linux-kernel@...r.kernel.org, Masami Hiramatsu <mhiramat@...nel.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: fix trace.h kernel-doc func/struct/enum
 members

On Fri, 10 Jan 2025 22:32:10 -0800
Randy Dunlap <rdunlap@...radead.org> wrote:

> @@ -1947,7 +1949,7 @@ struct event_command {
>  /**
>   * enum event_command_flags - flags for struct event_command
>   *
> - * @POST_TRIGGER: A flag that says whether or not this command needs
> + * @EVENT_CMD_FL_POST_TRIGGER: Indicates whether or not this command needs
>   *	to have its action delayed until after the current event has
>   *	been closed.  Some triggers need to avoid being invoked while
>   *	an event is currently in the process of being logged, since
> @@ -1966,7 +1968,7 @@ struct event_command {
>   *	itself logs to the trace buffer, this flag should be set,
>   *	otherwise it can be left unspecified.
>   *
> - * @NEEDS_REC: A flag that says whether or not this command needs
> + * @EVENT_CMD_FL_NEEDS_REC: Indicates whether or not this command needs
>   *	access to the trace record in order to perform its function,
>   *	regardless of whether or not it has a filter associated with
>   *	it (filters make a trigger require access to the trace record

I really wish tools would handle the above and accept it as is. I hate
adding duplication as it makes it harder to see the difference between
events.

/**
 * enum event_command_flags - flags for struct event_command
 *
 * @POST_TRIGGER: A flag that says whether or not this command needs
 *	to have its action delayed until after the current event has
 *	been closed.  Some triggers need to avoid being invoked while
 *	an event is currently in the process of being logged, since
 *	the trigger may itself log data into the trace buffer.  Thus
 *	we make sure the current event is committed before invoking
 *	those triggers.  To do that, the trigger invocation is split
 *	in two - the first part checks the filter using the current
 *	trace record; if a command has the @post_trigger flag set, it
 *	sets a bit for itself in the return value, otherwise it
 *	directly invokes the trigger.  Once all commands have been
 *	either invoked or set their return flag, the current record is
 *	either committed or discarded.  At that point, if any commands
 *	have deferred their triggers, those commands are finally
 *	invoked following the close of the current event.  In other
 *	words, if the event_trigger_ops @func() probe implementation
 *	itself logs to the trace buffer, this flag should be set,
 *	otherwise it can be left unspecified.
 *
 * @NEEDS_REC: A flag that says whether or not this command needs
 *	access to the trace record in order to perform its function,
 *	regardless of whether or not it has a filter associated with
 *	it (filters make a trigger require access to the trace record
 *	but are not always present).
 */
enum event_command_flags {
	EVENT_CMD_FL_POST_TRIGGER	= 1,
	EVENT_CMD_FL_NEEDS_REC		= 2,
};

Looks much easier to see what is what than:

/**
 * enum event_command_flags - flags for struct event_command
 *
 * @EVENT_CMD_FL_POST_TRIGGER: A flag that says whether or not this command needs
 *	to have its action delayed until after the current event has
 *	been closed.  Some triggers need to avoid being invoked while
 *	an event is currently in the process of being logged, since
 *	the trigger may itself log data into the trace buffer.  Thus
 *	we make sure the current event is committed before invoking
 *	those triggers.  To do that, the trigger invocation is split
 *	in two - the first part checks the filter using the current
 *	trace record; if a command has the @post_trigger flag set, it
 *	sets a bit for itself in the return value, otherwise it
 *	directly invokes the trigger.  Once all commands have been
 *	either invoked or set their return flag, the current record is
 *	either committed or discarded.  At that point, if any commands
 *	have deferred their triggers, those commands are finally
 *	invoked following the close of the current event.  In other
 *	words, if the event_trigger_ops @func() probe implementation
 *	itself logs to the trace buffer, this flag should be set,
 *	otherwise it can be left unspecified.
 *
 * @EVENT_CMD_FL_NEEDS_REC: A flag that says whether or not this command needs
 *	access to the trace record in order to perform its function,
 *	regardless of whether or not it has a filter associated with
 *	it (filters make a trigger require access to the trace record
 *	but are not always present).
 */
enum event_command_flags {
	EVENT_CMD_FL_POST_TRIGGER	= 1,
	EVENT_CMD_FL_NEEDS_REC		= 2,
};


If tooling can't handle it, I'd rather replace the /** with /* as I find
the above much harder to read than the original.

It shouldn't be too hard. The tooling should accept the kerneldoc if all
the enums start with the same text, then the descriptions only need to show
the part that is different.

-- Steve