From: Johannes Berg <johannes@sipsolutions.net>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Ilya Maximets <i.maximets@ovn.org>,
Matthieu Baerts <matttbe@kernel.org>,
Aaron Conole <aconole@redhat.com>,
Eelco Chaudron <echaudro@redhat.com>,
dev@openvswitch.org, Adrian Moreno <amorenoz@redhat.com>,
Antoine Tenart <atenart@kernel.org>
Subject: Re: [PATCH v7 0/4] tracing: improve symbolic printing
Date: Thu, 05 Nov 2026 18:09:00 +0100 [thread overview]
Message-ID: <69c7cb8d0567f96dd03c2f35abb12b41ffa09f59.camel@sipsolutions.net> (raw)
In-Reply-To: <20261105113857.4737cffb@gandalf.local.home>
On Thu, 2026-11-05 at 11:38 -0500, Steven Rostedt wrote:
> On Thu, 05 Nov 2026 17:26:58 +0100
> Johannes Berg <johannes@sipsolutions.net> wrote:
>
> > On Thu, 2026-11-05 at 11:24 -0500, Steven Rostedt wrote:
> > > > Any objection to simply converting the __print_symbolic() sample? It
> > > > would look something like this:
> > >
> > > Actually, keeping them both would be very useful. That way one can see how
> > > to do it the other way. We can say, the old way is being deprecated so
> > > please use the new way.
> >
> > What's the point? But OK, sure.
>
> I thought I mentioned the point? "That way one can see how to do it the
> other way". Basically, seeing the equivalent way.
You did say that, but I guess I don't really understand it - I see the
point of the sample as the status quo ("how to do it"), not the
evolutionary history ("how to convert it").
But sure, whatever you say, I don't really care.
> > > > +const char *sample_names2_lookup(unsigned long long value);
> > > > +void sample_names2_show(struct seq_file *m);
> > >
> > > Are the above going to be defined somewhere?
> >
> > Well, it's a sample, and I wasn't even using "sample_names2" in the
> > below, so I didn't actually define it. I can flesh that out even more,
> > but I'm not sure it's worth it?
>
> It is, otherwise it is confusing. If it is in the sample code, people will
> think it's required.
All of the sample code is optional though, you don't have to use any one
of the methods, either this, or enum, or ... if you want to use the
dynamic way of __print_sym() then it indeed is required.
Anyway I'll flesh this out more.
> Right, but if we want to deprecate the old way, we need to have the
> simplest sample of it. That is, have one event that shows the simple way to
> do this (one that simply replaces the old way), and then have another
> example of how to do it in a more complex and featureful way.
>
> Honestly, this is why I asked for this. Your updates use the complex
> method, I wasn't sure from the code how to do it for the simple case. That
> is, what wasn't needed for just replacing static __print_symbolic() lists.
The simple way changes from
__print_symbolic(foo, BRACED_LIST)
to
TRACE_DEFINE_SYM_LIST(foo_names, BRACED_LIST);
...
__print_sym(foo, foo_names)
so I'm not sure I see how it's really that much more complex?
Anyway, how about below sample update?
johannes
--- a/samples/trace_events/trace-events-sample.c
+++ b/samples/trace_events/trace-events-sample.c
@@ -11,6 +11,36 @@
#define CREATE_TRACE_POINTS
#include "trace-events-sample.h"
+/* functions for the dynamic __print_sym() example */
+static const struct {
+ unsigned long val;
+ char *name;
+} print_sym_dynamic_names[] = {
+ { 0, "zero" },
+ { TRACE_SAMPLE_FOO, "TWO" },
+ { TRACE_SAMPLE_BAR, "FOUR" },
+ { TRACE_SAMPLE_ZOO, "EIGHT" },
+ { 10, "TEN" }
+};
+
+const char *sample_names_dynamic_lookup(unsigned long long value)
+{
+ for (int i = 0; i < ARRAY_SIZE(print_sym_dynamic_names); i++) {
+ if (print_sym_dynamic_names[i].val == value)
+ return print_sym_dynamic_names[i].name;
+ }
+
+ return NULL;
+}
+
+void sample_names_dynamic_show(struct seq_file *m)
+{
+ for (int i = 0; i < ARRAY_SIZE(print_sym_dynamic_names); i++)
+ seq_printf(m, ", { %lu, \"%s\" }",
+ print_sym_dynamic_names[i].val,
+ print_sym_dynamic_names[i].name);
+}
+
static const char *random_strings[] = {
"Mother Goose",
"Snoopy",
diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index 816848a456a2..38d6bf808de6 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -297,6 +297,33 @@ TRACE_DEFINE_ENUM(TRACE_SAMPLE_FOO);
TRACE_DEFINE_ENUM(TRACE_SAMPLE_BAR);
TRACE_DEFINE_ENUM(TRACE_SAMPLE_ZOO);
+/*
+ * define the names as { value, "name" } tuples for __print_sym() below,
+ * same as the inline list for __print_dynamic()
+ */
+TRACE_DEFINE_SYM_LIST(sample_names_static,
+ { 0, "zero" },
+ { TRACE_SAMPLE_FOO, "TWO" },
+ { TRACE_SAMPLE_BAR, "FOUR" },
+ { TRACE_SAMPLE_ZOO, "EIGHT" },
+ { 10, "TEN" }
+);
+
+/*
+ * Alternatively, you could define a function for the names:
+ *
+ * The _lookup() function returns the string for the given value.
+ *
+ * The _show() function must
+ * seq_printf(m, ", { %u, \"%s\" }", value, name);
+ * for each value that userspace should be able to parse.
+ */
+const char *sample_names_dynamic_lookup(unsigned long long value);
+void sample_names_dynamic_show(struct seq_file *m);
+TRACE_DEFINE_SYM_FNS(sample_names_dynamic,
+ sample_names_dynamic_lookup,
+ sample_names_dynamic_show);
+
TRACE_EVENT(foo_bar,
TP_PROTO(const char *foo, int bar, const int *lst,
@@ -328,7 +355,7 @@ TRACE_EVENT(foo_bar,
__assign_cpumask(cpum, cpumask_bits(mask));
),
- TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s [%d] %*pbl",
+ TP_printk("foo %s %d %s %s %s %s %s %s %s %s (%s) (%s) %s [%d] %*pbl",
__entry->foo, __entry->bar,
/*
@@ -340,6 +367,9 @@ TRACE_EVENT(foo_bar,
* the variable matches one of the values, then it will print the
* string in that pair. If non are matched, it returns a string
* version of the number (if __entry->bar == 7 then "7" is returned).
+ *
+ * NOTE: This variant is deprecated, it uses more space than the below
+ * __print_sym() which is identical to this for userspace.
*/
__print_symbolic(__entry->bar,
{ 0, "zero" },
@@ -349,6 +379,21 @@ TRACE_EVENT(foo_bar,
{ 10, "TEN" }
),
+/*
+ * __print_sym( variable, symbol_list_definition ),
+ *
+ * The variable is tested against each value of the symbol list.
+ * The symbol list can be built statically (sample_names_static) or
+ * dynamically (sample_names_dynamic). If the variable matches one
+ * of the values, then it will print the string in that pair. If
+ * no entry matches, the number itself is printed (e.g. here if
+ * if __entry->bar == 7 then "7" is shown).
+ *
+ * See above for the declaration of sample_names_{static,dynamic}.
+ */
+ __print_sym(__entry->bar, sample_names_static),
+ __print_sym(__entry->bar, sample_names_dynamic),
+
/*
* __print_flags( variable, "delim", { value, "flag" }, ... ),
*
prev parent reply other threads:[~2026-11-05 17:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-22 11:08 Johannes Berg
2026-09-22 11:08 ` [PATCH v7 1/4] tracing: add __print_sym() to replace __print_symbolic() Johannes Berg
2026-09-22 11:08 ` [PATCH v7 2/4] tracing/timer: use __print_sym() Johannes Berg
2026-09-22 11:08 ` [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing Johannes Berg
2026-09-24 15:31 ` Ilya Maximets
2026-09-28 13:34 ` Aaron Conole
2026-09-22 11:08 ` [PATCH v7 4/4] net: drop_monitor: use drop_reason_lookup() Johannes Berg
2026-11-05 14:12 ` [PATCH v7 0/4] tracing: improve symbolic printing Steven Rostedt
2026-11-05 15:04 ` Johannes Berg
2026-11-05 16:24 ` Steven Rostedt
2026-11-05 16:26 ` Johannes Berg
2026-11-05 16:38 ` Steven Rostedt
2026-11-05 17:09 ` Johannes Berg [this message]
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=69c7cb8d0567f96dd03c2f35abb12b41ffa09f59.camel@sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=aconole@redhat.com \
--cc=amorenoz@redhat.com \
--cc=atenart@kernel.org \
--cc=dev@openvswitch.org \
--cc=echaudro@redhat.com \
--cc=i.maximets@ovn.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=matttbe@kernel.org \
--cc=mhiramat@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=rostedt@goodmis.org \
/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
all inboxes | Powered by JetHome®