public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: eugene.loh@oracle.com
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [DTrace-devel] [PATCH v2 3/3] Add more robust mechanism to skip tracepoint common fields
Date: Sun, 14 Sep 2025 14:26:04 +0100	[thread overview]
Message-ID: <87frcpw4gj.fsf@esperi.org.uk> (raw)
In-Reply-To: <20250913155619.22299-3-eugene.loh@oracle.com> (eugene loh's message of "Sat, 13 Sep 2025 11:56:19 -0400")

On 13 Sep 2025, eugene loh stated:
> 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_".

(If this turns out not to work, maybe we can hunt for the blank line
that always seems to follow common_* fields?)

>  	while (getline(&buf, &bufsz, f) >= 0) {

This is much clearer than it was before!

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

> @@ -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) {

... ok, I take back the thing I said in the previous mail about moving
things to a separate loop: the ID/field scan is necessary anyway, given
the first two lines:

ID: 94
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;

Given the overall format of e.g.

name: sys_enter_iopl
ID: 94
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;

	field:int __syscall_nr;	offset:8;	size:4;	signed:1;

an alternative approach might be to exploit the format, and always skip
all lines that do not start with a tab, and/or simply all lines until
the first entirely blank line? Maybe? (Looking at the kernel source, the
common fields always seem to be followed by a blank one.)

But this approach will work for now, though it appears to be pure
coincidence that all common fields start with common_. We definitely
have workable alternative approaches if this stops working :)

Hmm...

+			/*
+			 * 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';

Why is this necessary? We never look at the end of the identifier in the
common_ search, only the start...

  reply	other threads:[~2025-09-14 13:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-13 15:56 [PATCH 1/3] Clean up unused argsz eugene.loh
2025-09-13 15:56 ` [PATCH 2/3] Defer stripping out "__data_loc" until we know that is useful eugene.loh
2025-09-14 13:17   ` Nick Alcock
2025-09-15  4:50     ` Eugene Loh
2025-09-13 15:56 ` [PATCH v2 3/3] Add more robust mechanism to skip tracepoint common fields eugene.loh
2025-09-14 13:26   ` Nick Alcock [this message]
2025-09-15  4:56     ` [DTrace-devel] " Eugene Loh
2025-09-14 13:11 ` [DTrace-devel] [PATCH 1/3] Clean up unused argsz Nick Alcock

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87frcpw4gj.fsf@esperi.org.uk \
    --to=nick.alcock@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox