[DTrace-devel] [PATCH v2 3/3] 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 | 83 ++++++++++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 22 deletions(-)

diff --git a/libdtrace/dt_provider_tp.c b/libdtrace/dt_provider_tp.c
index a6f978e3f..13f4528fc 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,
@@ -140,19 +133,16 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
 {
 	char		*buf = NULL;
 	size_t		bufsz;
-	int		argc;
+	int		argc, common = 1;
 	dt_argdesc_t	*argv = NULL;
 
 	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.
+	 * Skip over how ever many arguments the caller asks us to skip.
+	 * We will skip initial "common fields" as well.
 	 */
 	argc = -skip;
 	while (getline(&buf, &bufsz, f) >= 0) {
@@ -160,14 +150,63 @@ 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;
-		sscanf(p, "__data_loc %[^;]", p);
 
-		/* We found a field: description - see if we should skip it. */
-		if (argc++ < 0)
-			continue;
+		/*
+		 * If we have only seen common fields to date, keep
+		 * looking for a non-common field.
+		 */
+		if (common == 1) {
+			char	*s = p + strlen(p) - 1;
+
+			/*
+			 * Strip off any [] array size specifications at the end.
+			 */
+			while (*s == ']') {
+				/* From ']' hunt back to '['.  They are not nested. */
+				while (s > p && *(--s) != '[') ;
+
+				/* Then remove any spaces. */
+				while (s > p && *(--s) == ' ') ;
+			}
+			*(++s) = '\0';
+
+			/*
+			 * Go to the beginning of the identifier.
+			 */
+			p = strrchr(p, ' ');
+			if (p == NULL)
+				return -EINVAL;
+			p++;
+
+			/*
+			 * Check if it is a common field.
+			 *
+			 * In kernel source file kernel/trace/trace_events.c
+			 * in trace_define_common_fields(), the macro
+			 * __common_field() is used to define common fields,
+			 * prepending names with "common_".
+			 */
+			if (strncmp(p, "common_", 7) == 0) {
+				/*
+				 * For Pass 2, we will not bother checking
+				 * for "common" fields;  we will just pretend
+				 * the caller asked us to skip more arguments.
+				 */
+				skip++;
+
+				continue;
+			}
+
+			/*
+			 * No more need to check for common fields.
+			 */
+			common = 0;
+		}
+
+		/* We found a non-common "field:" description. */
+		argc++;
 	}
 	free(buf);
 	buf = NULL;
-- 
2.47.3