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

[email protected]
Newsgroups dev.linux.lists.dtrace
Message-ID <[email protected]>
From: Eugene Loh <[email protected]>

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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.