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

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

[Randy Dunlap]

Add missing kernel-doc for function or struct members.
Use the correct enum names for enum members to prevent kernel-doc
warnings.

trace.h:302: warning: Function parameter or struct member 'cond_data' not described in 'cond_snapshot'
trace.h:618: warning: Excess struct member 'print_headers' description in 'tracer'

trace.h:1978: warning: Enum value 'EVENT_CMD_FL_POST_TRIGGER' not described in enum 'event_command_flags'
trace.h:1978: warning: Enum value 'EVENT_CMD_FL_NEEDS_REC' not described in enum 'event_command_flags'
trace.h:1978: warning: Excess enum value 'NEEDS_REC' description in 'event_command_flags'
trace.h:1978: warning: Excess enum value 'POST_TRIGGER' description in 'event_command_flags'

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: linux-trace-kernel@vger.kernel.org
---
 kernel/trace/trace.h |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- linux-next-20250108.orig/kernel/trace/trace.h
+++ linux-next-20250108/kernel/trace/trace.h
@@ -285,6 +285,8 @@ typedef bool (*cond_update_fn_t)(struct
  * associated with the trace instance by
  * tracing_cond_snapshot_disable().
  *
+ * @cond_data: the conditional snapshot's associated data
+ *
  * The method below is required.
  *
  * @update: When a conditional snapshot is invoked, the update()
@@ -568,7 +570,7 @@ struct trace_option_dentry {
  * @read: override the default read callback on trace_pipe
  * @splice_read: override the default splice_read callback on trace_pipe
  * @selftest: selftest to run on boot (see trace_selftest.c)
- * @print_headers: override the first lines that describe your columns
+ * @print_header: override the first lines that describe your columns
  * @print_line: callback that prints a trace
  * @set_flag: signals one of your private flags changed (trace_options file)
  * @flags: your private flags
@@ -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

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

[Steven Rostedt]

On Fri, 10 Jan 2025 22:32:10 -0800
Randy Dunlap <rdunlap@infradead.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