[DTrace-devel] [PATCH] Add more robust mechanism to skip tracepoint common fields

[Kris Van Hees]

On Thu, Sep 11, 2025 at 02:11:39PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> In dt_tp_event_info(), we parse tracepoint format files -- e.g.,
> /sys/kernel/debug/tracing/events/*/*/format.  Specifically, we
> are interested in the argument descriptions found in the file,
> but we are interested in skipping over "common fields."
> 
> The mechanism we used for this purpose was, in dt_provider_tp.c,
> to hardwire the number of fields to skip to SKIP_FIELDS_COUNT=4,
> assuming the common fields would always be type, flags, pid, and
> preempt_count.
> 
> This is hardly a robust mechanism.  For example, consider
>     https://kernel.googlesource.com/pub/scm/linux/kernel/git/rt/
>         linux-rt-devel/+/refs/tags/v5.9.1-rt20-patches/patches/
>         preempt-lazy-support.patch
> which introduces a preempt_lazy_count common field (on top of
> others).  Recent dtrace testing on RHCK 5.14 indicates widespread
> test failures due to this problem.
> 
> Implement a more robust mechanism.
> 
> Specifically, instead of skipping a hardwired (SKIP_FIELDS_COUNT=4)
> number of common fields, look for "common_" names.  E.g., in
> kernel/trace/trace_events.c in trace_define_common_fields(), we
> see the macro __common_field() is used to define common fields,
> and the names are prepended with "common_".

Comment below....

> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_provider_tp.c | 25 ++++++++-----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/libdtrace/dt_provider_tp.c b/libdtrace/dt_provider_tp.c
> index 4531a88a8..d0e980234 100644
> --- a/libdtrace/dt_provider_tp.c
> +++ b/libdtrace/dt_provider_tp.c
> @@ -17,13 +17,6 @@
>  #include "dt_probe.h"
>  #include "dt_impl.h"
>  
> -/*
> - * All tracing events (tracepoints) include a number of fields that we need to
> - * skip in the tracepoint format description.  These fields are: common_type,
> - * common_flags, common_preempt_coint, and common_pid.
> - */
> -#define SKIP_FIELDS_COUNT	4
> -
>  /*
>   * Tracepoint-specific probe data.  This is allocated for every tracepoint
>   * based probe.  Since 0 is not a valid tracepoint event id, and given that BTF
> @@ -129,10 +122,10 @@ dt_tp_attach_raw(dtrace_hdl_t *dtp, tp_probe_t *tpp, const char *name,
>   * the identifier isn't as easy because it may be suffixed by one or more
>   * array dimension specifiers (and those are part of the type).
>   *
> - * All events include a number of fields that we are not interested in and that
> - * need to be skipped (SKIP_FIELDS_COUNT).  Callers of this function can
> - * specify an additional number of fields to skip (using the 'skip' parameter)
> - * before we get to the actual arguments.
> + * All events include a number of common fields that we are not interested
> + * in and that need to be skipped.  Callers of this function can specify an
> + * additional number of fields to skip (using the 'skip' parameter) before
> + * we get to the actual arguments.
>   */
>  int
>  dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> @@ -146,11 +139,6 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
>  
>  	tpp->id = 0;
>  
> -	/*
> -	 * Let skip be the total number of fields to skip.
> -	 */
> -	skip += SKIP_FIELDS_COUNT;
> -
>  	/*
>  	 * Pass 1:
>  	 * Determine the event id and the number of arguments (along with the
> @@ -162,9 +150,10 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
>  
>  		if (sscanf(buf, "ID: %u\n", &tpp->id) == 1)
>  			continue;
> -
>  		if (sscanf(buf, " field:%[^;]", p) <= 0)
>  			continue;
> +		if (strstr(buf, " common_"))
> +			continue;

This would also skip any field that might have a type name like struct common_*
which is certainly not what we want to do.  You need to make sure that you match
on the field name only.  (And same below.)

You might be able to simplify by assuming (a safe bet) that all common_* fields
are at the beginning, and have the first loop count the number of common fields
to skip, i.e. increase skip for any common_* field found, and then the 2nd loop
can simply use that skip counter without needing to parse anything we are not
interested in.

>  		sscanf(p, "__data_loc %[^;]", p);
>  
>  		/* We found a field: description - see if we should skip it. */
> @@ -216,6 +205,8 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
>  		p = buf;
>  		if (sscanf(buf, " field:%[^;]", p) <= 0)
>  			continue;
> +		if (strstr(buf, " common_"))
> +			continue;
>  		sscanf(p, "__data_loc %[^;]", p);
>  
>  		/* We found a field: description - see if we should skip it. */
> -- 
> 2.47.3
>