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

[eugene.loh at oracle.com]

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_".

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;
 		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