* [PATCH v7 1/4] tracing: add __print_sym() to replace __print_symbolic()
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
@ 2026-09-22 11:08 ` Johannes Berg
2026-09-22 11:08 ` [PATCH v7 2/4] tracing/timer: use __print_sym() Johannes Berg
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron,
dev, Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
The way __print_symbolic() works is limited and inefficient
in multiple ways:
- you can only use it with a static list of symbols, but
e.g. the SKB dropreasons are now a dynamic list
- it builds the list in memory _three_ times, so it takes
a lot of memory:
- The print_fmt contains the list (since it's passed to
the macro there). This actually contains the names
_twice_, which is fixed up at runtime.
- TRACE_DEFINE_ENUM() puts a 24-byte struct trace_eval_map
for every entry, plus the string pointed to by it, which
cannot be deduplicated with the strings in the print_fmt
- The in-kernel symbolic printing creates yet another list
of struct trace_print_flags for trace_print_symbols_seq()
- it also requires runtime fixup during init, which is a lot
of string parsing due to the print_fmt fixup
Introduce __print_sym() to - over time - replace the old one.
We can easily extend this also to __print_flags later, but I
cared only about the SKB dropreasons for now, which has only
__print_symbolic().
This new __print_sym() requires only a single list of items,
created by TRACE_DEFINE_SYM_LIST(), or can even use another
already existing list by using TRACE_DEFINE_SYM_FNS() with
lookup and show methods.
Then, instead of doing an init-time fixup, just do this at the
time when userspace reads the print_fmt. This way, dynamically
updated lists are possible.
For userspace, nothing actually changes, because the print_fmt
is shown exactly the same way the old __print_symbolic() was.
This adds about 4k .text in my test builds, but that'll be
more than paid for by the actual conversions.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v7: address sashiko's alignment concern
v6: don't place data needed at runtime in .init sections,
fixing the crash Steven reported with v1:
https://lore.kernel.org/r/20240819182340.3bd23d67@gandalf.local.home/
(yes, it's been over 2 years ... my bad)
---
include/asm-generic/vmlinux.lds.h | 4 +
include/linux/module.h | 2 +
include/linux/trace_events.h | 7 ++
include/linux/tracepoint.h | 20 +++++
include/trace/stages/init.h | 55 ++++++++++++
include/trace/stages/stage2_data_offsets.h | 6 ++
include/trace/stages/stage3_trace_output.h | 9 ++
include/trace/stages/stage7_class_define.h | 3 +
kernel/module/main.c | 3 +
kernel/trace/trace_events.c | 99 +++++++++++++++++++++-
kernel/trace/trace_output.c | 45 ++++++++++
11 files changed, 251 insertions(+), 2 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b2988aa12f66..632eafd0f496 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -275,9 +275,12 @@
#define FTRACE_EVENTS() \
. = ALIGN(8); \
BOUNDED_SECTION(_ftrace_events) \
+ . = ALIGN(8); \
BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
+#define FTRACE_SYM_DEFS() BOUNDED_SECTION(_ftrace_sym_defs)
#else
#define FTRACE_EVENTS()
+#define FTRACE_SYM_DEFS()
#endif
#ifdef CONFIG_TRACING
@@ -391,6 +394,7 @@
TRACE_PRINTKS() \
BPF_RAW_TP() \
TRACEPOINT_STR() \
+ FTRACE_SYM_DEFS() \
KUNIT_TABLE()
/*
diff --git a/include/linux/module.h b/include/linux/module.h
index 96cc98568eea..a02833ac421b 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -516,6 +516,8 @@ struct module {
unsigned int num_trace_events;
struct trace_eval_map **trace_evals;
unsigned int num_trace_evals;
+ struct trace_sym_def **trace_sym_defs;
+ unsigned int num_trace_sym_defs;
#endif
#ifdef CONFIG_DYNAMIC_FTRACE
unsigned int num_ftrace_callsites;
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5cbd09c8be8d..80c34ef89feb 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -29,6 +29,13 @@ const char *trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
const struct trace_print_flags *symbol_array,
size_t symbol_array_size);
+const char *trace_print_sym_seq(struct trace_seq *p, unsigned long long val,
+ const char *(*lookup)(unsigned long long val));
+const char *trace_sym_lookup(const struct trace_sym_entry *list,
+ size_t len, unsigned long long value);
+void trace_sym_show(struct seq_file *m,
+ const struct trace_sym_entry *list, size_t len);
+
#if BITS_PER_LONG == 32
const char *trace_print_flags_seq_u64(struct trace_seq *p, const char *delim,
unsigned long long flags,
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index e0d838c9ce93..05c45a72756c 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -32,6 +32,24 @@ struct trace_eval_map {
unsigned long eval_value;
};
+struct trace_sym_def {
+ const char *system;
+ const char *symbol_id;
+ /* may return NULL, called under rcu_read_lock() */
+ const char * (*lookup)(unsigned long long);
+ /*
+ * Must print the list: ', { val, "name"}, ...'
+ * with no trailing comma, but with the leading ', '
+ * to simplify things:
+ */
+ void (*show)(struct seq_file *);
+};
+
+struct trace_sym_entry {
+ unsigned long long value;
+ const char *name;
+};
+
#define TRACEPOINT_DEFAULT_PRIO 10
extern int
@@ -163,6 +181,8 @@ extern void syscall_unregfunc(void);
#define TRACE_DEFINE_ENUM(x)
#define TRACE_DEFINE_SIZEOF(x)
+#define TRACE_DEFINE_SYM_FNS(...)
+#define TRACE_DEFINE_SYM_LIST(...)
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
diff --git a/include/trace/stages/init.h b/include/trace/stages/init.h
index 000bcfc8dd2e..6285a03a52b2 100644
--- a/include/trace/stages/init.h
+++ b/include/trace/stages/init.h
@@ -23,6 +23,61 @@ TRACE_MAKE_SYSTEM_STR();
__section("_ftrace_eval_map") \
*TRACE_SYSTEM##_##a = &__##TRACE_SYSTEM##_##a
+/*
+ * Define a symbol for __print_sym by giving lookup and
+ * show functions. See &struct trace_sym_def.
+ */
+#undef TRACE_DEFINE_SYM_FNS
+#define TRACE_DEFINE_SYM_FNS(_symbol_id, _lookup, _show) \
+ _TRACE_DEFINE_SYM_FNS(TRACE_SYSTEM, _symbol_id, _lookup, _show)
+#define _TRACE_DEFINE_SYM_FNS(_system, _symbol_id, _lookup, _show) \
+ __TRACE_DEFINE_SYM_FNS(_system, _symbol_id, _lookup, _show)
+#define __TRACE_DEFINE_SYM_FNS(_system, _symbol_id, _lookup, _show) \
+ ___TRACE_DEFINE_SYM_FNS(_system ## _ ## _symbol_id, _symbol_id, \
+ _lookup, _show)
+#define ___TRACE_DEFINE_SYM_FNS(_name, _symbol_id, _lookup, _show) \
+ static struct trace_sym_def \
+ __trace_sym_def_ ## _name = { \
+ .system = TRACE_SYSTEM_STRING, \
+ .symbol_id = #_symbol_id, \
+ .lookup = _lookup, \
+ .show = _show, \
+ }; \
+ static struct trace_sym_def __used \
+ __section("_ftrace_sym_defs") \
+ *__trace_sym_def_p_ ## _name = &__trace_sym_def_ ## _name
+
+/*
+ * Define a symbol for __print_sym by giving lookup and
+ * show functions. See &struct trace_sym_def.
+ */
+#undef TRACE_DEFINE_SYM_LIST
+#define TRACE_DEFINE_SYM_LIST(_symbol_id, ...) \
+ _TRACE_DEFINE_SYM_LIST(TRACE_SYSTEM, _symbol_id, __VA_ARGS__)
+#define _TRACE_DEFINE_SYM_LIST(_system, _symbol_id, ...) \
+ __TRACE_DEFINE_SYM_LIST(_system, _symbol_id, __VA_ARGS__)
+#define __TRACE_DEFINE_SYM_LIST(_system, _symbol_id, ...) \
+ ___TRACE_DEFINE_SYM_LIST(_system ## _ ## _symbol_id, _symbol_id,\
+ __VA_ARGS__)
+#define ___TRACE_DEFINE_SYM_LIST(_name, _symbol_id, ...) \
+ static struct trace_sym_entry \
+ __trace_sym_list_ ## _name[] = { __VA_ARGS__ }; \
+ static const char * \
+ __trace_sym_lookup_ ## _name(unsigned long long value) \
+ { \
+ return trace_sym_lookup(__trace_sym_list_ ## _name, \
+ ARRAY_SIZE(__trace_sym_list_ ## _name), value); \
+ } \
+ static void \
+ __trace_sym_show_ ## _name(struct seq_file *m) \
+ { \
+ trace_sym_show(m, __trace_sym_list_ ## _name, \
+ ARRAY_SIZE(__trace_sym_list_ ## _name)); \
+ } \
+ ___TRACE_DEFINE_SYM_FNS(_name, _symbol_id, \
+ __trace_sym_lookup_ ## _name, \
+ __trace_sym_show_ ## _name)
+
#undef TRACE_DEFINE_SIZEOF
#define TRACE_DEFINE_SIZEOF(a) \
static struct trace_eval_map __used __initdata \
diff --git a/include/trace/stages/stage2_data_offsets.h b/include/trace/stages/stage2_data_offsets.h
index 8b0cff06d346..5afd9de7deb3 100644
--- a/include/trace/stages/stage2_data_offsets.h
+++ b/include/trace/stages/stage2_data_offsets.h
@@ -5,6 +5,12 @@
#undef TRACE_DEFINE_ENUM
#define TRACE_DEFINE_ENUM(a)
+#undef TRACE_DEFINE_SYM_FNS
+#define TRACE_DEFINE_SYM_FNS(_symbol_id, _lookup, _show)
+
+#undef TRACE_DEFINE_SYM_LIST
+#define TRACE_DEFINE_SYM_LIST(_symbol_id, ...)
+
#undef TRACE_DEFINE_SIZEOF
#define TRACE_DEFINE_SIZEOF(a)
diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index 181b81335781..09304259a8c1 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -79,6 +79,15 @@
trace_print_symbols_seq(p, value, symbols, ARRAY_SIZE(symbols)); \
})
+#undef __print_sym
+#define __print_sym(value, symbol_id) \
+ ___print_sym(TRACE_SYSTEM, value, symbol_id)
+#define ___print_sym(sys, value, symbol_id) \
+ ____print_sym(sys, value, symbol_id)
+#define ____print_sym(sys, value, symbol_id) \
+ trace_print_sym_seq(p, value, \
+ __trace_sym_def_p_ ## sys ## _ ## symbol_id->lookup)
+
#undef __print_flags_u64
#undef __print_symbolic_u64
#if BITS_PER_LONG == 32
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index 47008897a795..936b9fa08cea 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -45,6 +45,9 @@
#define __event_in_softirq() (REC->common_flags & 0x10)
#define __event_in_irq() (REC->common_flags & 0x18)
+#undef __print_sym
+#define __print_sym(value, symbol_id) __print_sym(value:symbol_id)
+
/*
* The below is not executed in the kernel. It is only what is
* displayed in the print format for userspace to parse.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index d0e1e0bd2ad0..8ab386a37376 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2735,6 +2735,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->trace_evals = section_objs(info, "_ftrace_eval_map",
sizeof(*mod->trace_evals),
&mod->num_trace_evals);
+ mod->trace_sym_defs = section_objs(info, "_ftrace_sym_defs",
+ sizeof(*mod->trace_sym_defs),
+ &mod->num_trace_sym_defs);
#endif
#ifdef CONFIG_TRACING
mod->trace_bprintk_fmt_start = section_objs(info, "__trace_printk_fmt",
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 30c0ddf90887..53849baae291 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2111,6 +2111,102 @@ static void *f_next(struct seq_file *m, void *v, loff_t *pos)
return node;
}
+extern struct trace_sym_def *__start_ftrace_sym_defs[];
+extern struct trace_sym_def *__stop_ftrace_sym_defs[];
+
+/* note: @name is not NUL-terminated */
+static void show_sym_list(struct seq_file *m, struct trace_event_call *call,
+ const char *name, unsigned int name_len)
+{
+ struct trace_sym_def **sym_defs;
+ unsigned int n_sym_defs, i;
+
+ if ((call->flags & TRACE_EVENT_FL_DYNAMIC) || !call->module) {
+ sym_defs = __start_ftrace_sym_defs;
+ n_sym_defs = __stop_ftrace_sym_defs - __start_ftrace_sym_defs;
+ } else {
+#ifdef CONFIG_MODULES
+ struct module *mod = call->module;
+
+ sym_defs = mod->trace_sym_defs;
+ n_sym_defs = mod->num_trace_sym_defs;
+#else
+ return;
+#endif /* CONFIG_MODULES */
+ }
+
+ for (i = 0; i < n_sym_defs; i++) {
+ unsigned int sym_len;
+
+ if (!sym_defs[i])
+ continue;
+ if (sym_defs[i]->system != call->class->system)
+ continue;
+ sym_len = strlen(sym_defs[i]->symbol_id);
+ if (name_len != sym_len)
+ continue;
+ if (strncmp(sym_defs[i]->symbol_id, name, sym_len))
+ continue;
+ if (sym_defs[i]->show)
+ sym_defs[i]->show(m);
+ break;
+ }
+}
+
+static void show_print_fmt(struct seq_file *m, struct trace_event_call *call)
+{
+ char *ptr = call->print_fmt;
+ bool in_print_sym = false;
+ int quote = 0;
+
+ seq_puts(m, "\nprint fmt: ");
+ while (*ptr) {
+ if (*ptr == '\\') {
+ seq_putc(m, *ptr);
+ ptr++;
+ /* paranoid */
+ if (!*ptr)
+ break;
+ goto next;
+ }
+ if (*ptr == '"') {
+ quote ^= 1;
+ goto next;
+ }
+ if (quote)
+ goto next;
+
+ if (in_print_sym && *ptr != ':')
+ goto next;
+
+ if (in_print_sym && *ptr == ':') {
+ const char *name;
+
+ ptr++;
+ name = ptr;
+ /* skip the name */
+ while (*ptr && *ptr != ')')
+ ptr++;
+ /* and show the actual list inline now */
+ show_sym_list(m, call, name, ptr - name);
+ in_print_sym = false;
+ continue;
+ }
+
+ if (strncmp(ptr, "__print_sym(", 12) == 0) {
+ ptr += 12;
+ seq_puts(m, "__print_symbolic(");
+ in_print_sym = true;
+ continue;
+ }
+next:
+ seq_putc(m, *ptr);
+ ptr++;
+ }
+
+ seq_putc(m, '\n');
+}
+
static int f_show(struct seq_file *m, void *v)
{
struct trace_event_file *file = event_file_data(m->private);
@@ -2130,8 +2226,7 @@ static int f_show(struct seq_file *m, void *v)
return 0;
case FORMAT_PRINTFMT:
- seq_printf(m, "\nprint fmt: %s\n",
- call->print_fmt);
+ show_print_fmt(m, call);
return 0;
}
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index a5ad76175d10..0850b3ad5707 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -131,6 +131,51 @@ trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
}
EXPORT_SYMBOL(trace_print_symbols_seq);
+const char *trace_sym_lookup(const struct trace_sym_entry *list,
+ size_t len, unsigned long long value)
+{
+ size_t i;
+
+ for (i = 0; i < len; i++) {
+ if (list[i].value == value)
+ return list[i].name;
+ }
+ return NULL;
+}
+EXPORT_SYMBOL(trace_sym_lookup);
+
+void trace_sym_show(struct seq_file *m,
+ const struct trace_sym_entry *list, size_t len)
+{
+ size_t i;
+
+ for (i = 0; i < len; i++)
+ seq_printf(m, ", { %lld, \"%s\" }",
+ list[i].value, list[i].name);
+}
+EXPORT_SYMBOL(trace_sym_show);
+
+const char *
+trace_print_sym_seq(struct trace_seq *p, unsigned long long val,
+ const char *(*lookup)(unsigned long long val))
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ const char *name;
+
+ rcu_read_lock();
+ name = lookup(val);
+ if (name)
+ trace_seq_puts(p, name);
+ else
+ trace_seq_printf(p, "0x%llx", val);
+ rcu_read_unlock();
+
+ trace_seq_putc(p, 0);
+
+ return ret;
+}
+EXPORT_SYMBOL(trace_print_sym_seq);
+
#if BITS_PER_LONG == 32
const char *
trace_print_flags_seq_u64(struct trace_seq *p, const char *delim,
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v7 2/4] tracing/timer: use __print_sym()
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing 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 ` Johannes Berg
2026-09-22 11:08 ` [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing Johannes Berg
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron,
dev, Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Use the new __print_sym() in the timer tracing, just to show
how to convert something. This adds ~80 bytes of .text for a
saving of ~1.5K of data in my builds.
Note the format changes from
print fmt: "success=%d dependency=%s", REC->success, __print_symbolic(REC->dependency, { 0, "NONE" }, { (1 << 0), "POSIX_TIMER" }, { (1 << 1), "PERF_EVENTS" }, { (1 << 2), "SCHED" }, { (1 << 3), "CLOCK_UNSTABLE" }, { (1 << 4), "RCU" }, { (1 << 5), "RCU_EXP" })
to
print fmt: "success=%d dependency=%s", REC->success, __print_symbolic(REC->dependency, { 0, "NONE" }, { 1, "POSIX_TIMER" }, { 2, "PERF_EVENTS" }, { 4, "SCHED" }, { 8, "CLOCK_UNSTABLE" }, { 16, "RCU" }, { 32, "RCU_EXP" })
since the values are now just printed in the show function as
pure decimal values.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/trace/events/timer.h | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index 3aa0608c6361..d45259d96069 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -441,26 +441,18 @@ TRACE_EVENT(itimer_expire,
#undef tick_dep_mask_name
#undef tick_dep_name_end
-/* The MASK will convert to their bits and they need to be processed too */
-#define tick_dep_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-#define tick_dep_name_end(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-/* NONE only has a mask defined for it */
-#define tick_dep_mask_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
+#define tick_dep_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
+#define tick_dep_mask_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
+#define tick_dep_name_end(sdep) { TICK_DEP_MASK_##sdep, #sdep }
-TICK_DEP_NAMES
+TRACE_DEFINE_SYM_LIST(tick_dep_names, TICK_DEP_NAMES);
#undef tick_dep_name
#undef tick_dep_mask_name
#undef tick_dep_name_end
-#define tick_dep_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
-#define tick_dep_mask_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
-#define tick_dep_name_end(sdep) { TICK_DEP_MASK_##sdep, #sdep }
-
#define show_tick_dep_name(val) \
- __print_symbolic(val, TICK_DEP_NAMES)
+ __print_sym(val, tick_dep_names)
TRACE_EVENT(tick_stop,
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing 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 ` 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
4 siblings, 2 replies; 13+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron,
dev, Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
The __print_symbolic() could only ever print the core
drop reasons, since that's the way the infrastructure
works. Now that we have __print_sym() with all the
advantages mentioned in that commit, convert to that
and get all the drop reasons from all subsystems. As
we already have a list of them, that's really easy.
This is a little bit of .text (~100 bytes in my build)
and saves a lot of .data (~17k).
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v7: update the selftest accordingly
---
include/net/dropreason.h | 5 +++
include/trace/events/skb.h | 13 ++----
net/core/skbuff.c | 43 +++++++++++++++++++
.../selftests/net/openvswitch/openvswitch.sh | 17 ++------
4 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index 1df60645fb27..dc4a60130c09 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -42,6 +42,11 @@ struct drop_reason_list {
extern const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value);
+void drop_reason_show(struct seq_file *m);
+#endif
+
void drop_reasons_register_subsys(enum skb_drop_reason_subsys subsys,
const struct drop_reason_list *list);
void drop_reasons_unregister_subsys(enum skb_drop_reason_subsys subsys);
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 2945aa7fe9a7..991bf172a6ea 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -8,15 +8,9 @@
#include <linux/skbuff.h>
#include <linux/netdevice.h>
#include <linux/tracepoint.h>
+#include <net/dropreason.h>
-#undef FN
-#define FN(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason);
-DEFINE_DROP_REASON(FN, FN)
-
-#undef FN
-#undef FNe
-#define FN(reason) { SKB_DROP_REASON_##reason, #reason },
-#define FNe(reason) { SKB_DROP_REASON_##reason, #reason }
+TRACE_DEFINE_SYM_FNS(drop_reason, drop_reason_lookup, drop_reason_show);
/*
* Tracepoint for free an sk_buff:
@@ -47,8 +41,7 @@ TRACE_EVENT(kfree_skb,
TP_printk("skbaddr=%p rx_sk=%p protocol=%u location=%pS reason: %s",
__entry->skbaddr, __entry->rx_sk, __entry->protocol,
__entry->location,
- __print_symbolic(__entry->reason,
- DEFINE_DROP_REASON(FN, FNe)))
+ __print_sym(__entry->reason, drop_reason))
);
#undef FN
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index dbbe10277d51..8c8f7a05dea4 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -154,6 +154,49 @@ drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
};
EXPORT_SYMBOL(drop_reasons_by_subsys);
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value)
+{
+ unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
+ u32 reason = value & ~SKB_DROP_REASON_SUBSYS_MASK;
+ const struct drop_reason_list *subsys;
+
+ if (subsys_id >= SKB_DROP_REASON_SUBSYS_NUM)
+ return NULL;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ return NULL;
+ if (reason >= subsys->n_reasons)
+ return NULL;
+ return subsys->reasons[reason];
+}
+
+void drop_reason_show(struct seq_file *m)
+{
+ u32 subsys_id;
+
+ rcu_read_lock();
+ for (subsys_id = 0; subsys_id < SKB_DROP_REASON_SUBSYS_NUM; subsys_id++) {
+ const struct drop_reason_list *subsys;
+ u32 i;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ continue;
+
+ for (i = 0; i < subsys->n_reasons; i++) {
+ if (!subsys->reasons[i])
+ continue;
+ seq_printf(m, ", { %u, \"%s\" }",
+ (subsys_id << SKB_DROP_REASON_SUBSYS_SHIFT) | i,
+ subsys->reasons[i]);
+ }
+ }
+ rcu_read_unlock();
+}
+#endif
+
/**
* drop_reasons_register_subsys - register another drop reason subsystem
* @subsys: the subsystem to register, must not be the core
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index a31f7fb6882d..9b8edfcd2d1a 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -234,7 +234,7 @@ ovs_drop_reason_count()
local reason=$1
local perf_output=`perf script -i ${ovs_dir}/perf.data -F trace:event,trace`
- local pattern="skb:kfree_skb:.*reason: $reason"
+ local pattern="skb:kfree_skb:.*reason: $reason$"
return `echo "$perf_output" | grep "$pattern" | wc -l`
}
@@ -790,15 +790,6 @@ test_psample() {
# - drop packets and verify the right drop reason is reported
test_drop_reason() {
which perf >/dev/null 2>&1 || return $ksft_skip
- which pahole >/dev/null 2>&1 || return $ksft_skip
-
- ovs_drop_subsys=$(pahole -C skb_drop_reason_subsys |
- awk '/OPENVSWITCH/ { print $3; }' |
- tr -d ,)
- if [ -z "$ovs_drop_subsys" ]; then
- info "failed to get OVS drop subsys ID"
- return $ksft_skip
- fi
sbx_add "test_drop_reason" || return $?
@@ -842,7 +833,7 @@ test_drop_reason() {
"in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20,proto=1),icmp()" 'drop'
ovs_drop_record_and_run "test_drop_reason" ip netns exec client ping -c 2 172.31.110.20
- ovs_drop_reason_count 0x${ovs_drop_subsys}0001 # OVS_DROP_FLOW_ACTION
+ ovs_drop_reason_count OVS_DROP_LAST_ACTION
if [[ "$?" -ne "2" ]]; then
info "Did not detect expected drops: $?"
return 1
@@ -859,7 +850,7 @@ test_drop_reason() {
ovs_drop_record_and_run \
"test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 6000
- ovs_drop_reason_count 0x${ovs_drop_subsys}0004 # OVS_DROP_EXPLICIT_ACTION_ERROR
+ ovs_drop_reason_count OVS_DROP_EXPLICIT_WITH_ERROR
if [[ "$?" -ne "1" ]]; then
info "Did not detect expected explicit error drops: $?"
return 1
@@ -867,7 +858,7 @@ test_drop_reason() {
ovs_drop_record_and_run \
"test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 7000
- ovs_drop_reason_count 0x${ovs_drop_subsys}0003 # OVS_DROP_EXPLICIT_ACTION
+ ovs_drop_reason_count OVS_DROP_EXPLICIT
if [[ "$?" -ne "1" ]]; then
info "Did not detect expected explicit drops: $?"
return 1
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing
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
1 sibling, 0 replies; 13+ messages in thread
From: Ilya Maximets @ 2026-09-24 15:31 UTC (permalink / raw)
To: Johannes Berg, linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron,
dev, Adrian Moreno, Antoine Tenart, Johannes Berg
On 9/22/26 1:08 PM, Johannes Berg wrote:
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index a31f7fb6882d..9b8edfcd2d1a 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -234,7 +234,7 @@ ovs_drop_reason_count()
> local reason=$1
>
> local perf_output=`perf script -i ${ovs_dir}/perf.data -F trace:event,trace`
> - local pattern="skb:kfree_skb:.*reason: $reason"
> + local pattern="skb:kfree_skb:.*reason: $reason$"
>
> return `echo "$perf_output" | grep "$pattern" | wc -l`
> }
> @@ -790,15 +790,6 @@ test_psample() {
> # - drop packets and verify the right drop reason is reported
> test_drop_reason() {
> which perf >/dev/null 2>&1 || return $ksft_skip
> - which pahole >/dev/null 2>&1 || return $ksft_skip
> -
> - ovs_drop_subsys=$(pahole -C skb_drop_reason_subsys |
> - awk '/OPENVSWITCH/ { print $3; }' |
> - tr -d ,)
> - if [ -z "$ovs_drop_subsys" ]; then
> - info "failed to get OVS drop subsys ID"
> - return $ksft_skip
> - fi
>
> sbx_add "test_drop_reason" || return $?
>
> @@ -842,7 +833,7 @@ test_drop_reason() {
> "in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20,proto=1),icmp()" 'drop'
>
> ovs_drop_record_and_run "test_drop_reason" ip netns exec client ping -c 2 172.31.110.20
> - ovs_drop_reason_count 0x${ovs_drop_subsys}0001 # OVS_DROP_FLOW_ACTION
> + ovs_drop_reason_count OVS_DROP_LAST_ACTION
> if [[ "$?" -ne "2" ]]; then
> info "Did not detect expected drops: $?"
> return 1
> @@ -859,7 +850,7 @@ test_drop_reason() {
>
> ovs_drop_record_and_run \
> "test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 6000
> - ovs_drop_reason_count 0x${ovs_drop_subsys}0004 # OVS_DROP_EXPLICIT_ACTION_ERROR
> + ovs_drop_reason_count OVS_DROP_EXPLICIT_WITH_ERROR
> if [[ "$?" -ne "1" ]]; then
> info "Did not detect expected explicit error drops: $?"
> return 1
> @@ -867,7 +858,7 @@ test_drop_reason() {
>
> ovs_drop_record_and_run \
> "test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 7000
> - ovs_drop_reason_count 0x${ovs_drop_subsys}0003 # OVS_DROP_EXPLICIT_ACTION
> + ovs_drop_reason_count OVS_DROP_EXPLICIT
> if [[ "$?" -ne "1" ]]; then
> info "Did not detect expected explicit drops: $?"
> return 1
For the test:
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing
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
1 sibling, 0 replies; 13+ messages in thread
From: Aaron Conole @ 2026-09-28 13:34 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-trace-kernel, linux-kernel, netdev, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Ilya Maximets,
Matthieu Baerts, Eelco Chaudron, dev, Adrian Moreno,
Antoine Tenart, Johannes Berg
Johannes Berg <johannes@sipsolutions.net> writes:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The __print_symbolic() could only ever print the core
> drop reasons, since that's the way the infrastructure
> works. Now that we have __print_sym() with all the
> advantages mentioned in that commit, convert to that
> and get all the drop reasons from all subsystems. As
> we already have a list of them, that's really easy.
>
> This is a little bit of .text (~100 bytes in my build)
> and saves a lot of .data (~17k).
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> v7: update the selftest accordingly
Sorry for the late reply - this series was caught in a Spam filter for
some reason.
> ---
[ ... ]
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index a31f7fb6882d..9b8edfcd2d1a 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -234,7 +234,7 @@ ovs_drop_reason_count()
For the OVS part (also nice to see the line count reduction) -
Reviewed-by: Aaron Conole <aconole@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v7 4/4] net: drop_monitor: use drop_reason_lookup()
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
` (2 preceding siblings ...)
2026-09-22 11:08 ` [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing Johannes Berg
@ 2026-09-22 11:08 ` Johannes Berg
2026-11-05 14:12 ` [PATCH v7 0/4] tracing: improve symbolic printing Steven Rostedt
4 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron,
dev, Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Now that we have drop_reason_lookup(), we can just use it for
drop_monitor as well, rather than exporting the list itself.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v7: don't hide drop_reason_show() prototype (sashiko,
but it got it the wrong way around)
---
include/net/dropreason.h | 5 +----
net/core/drop_monitor.c | 20 +++++---------------
net/core/skbuff.c | 6 +++---
3 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index dc4a60130c09..537428c72642 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -38,12 +38,9 @@ struct drop_reason_list {
size_t n_reasons;
};
-/* Note: due to dynamic registrations, access must be under RCU */
-extern const struct drop_reason_list __rcu *
-drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
+const char *drop_reason_lookup(unsigned long long value);
#ifdef CONFIG_TRACEPOINTS
-const char *drop_reason_lookup(unsigned long long value);
void drop_reason_show(struct seq_file *m);
#endif
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index edc660778408..5aa7692c5df2 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -611,9 +611,8 @@ static int net_dm_packet_report_fill(struct sk_buff *msg, struct sk_buff *skb,
size_t payload_len)
{
struct net_dm_skb_cb *cb = NET_DM_SKB_CB(skb);
- const struct drop_reason_list *list = NULL;
- unsigned int subsys, subsys_reason;
char buf[NET_DM_MAX_SYMBOL_LEN];
+ const char *reason_str;
struct nlattr *attr;
void *hdr;
int rc;
@@ -631,19 +630,10 @@ static int net_dm_packet_report_fill(struct sk_buff *msg, struct sk_buff *skb,
goto nla_put_failure;
rcu_read_lock();
- subsys = u32_get_bits(cb->reason, SKB_DROP_REASON_SUBSYS_MASK);
- if (subsys < SKB_DROP_REASON_SUBSYS_NUM)
- list = rcu_dereference(drop_reasons_by_subsys[subsys]);
- subsys_reason = cb->reason & ~SKB_DROP_REASON_SUBSYS_MASK;
- if (!list ||
- subsys_reason >= list->n_reasons ||
- !list->reasons[subsys_reason] ||
- strlen(list->reasons[subsys_reason]) > NET_DM_MAX_REASON_LEN) {
- list = rcu_dereference(drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_CORE]);
- subsys_reason = SKB_DROP_REASON_NOT_SPECIFIED;
- }
- if (nla_put_string(msg, NET_DM_ATTR_REASON,
- list->reasons[subsys_reason])) {
+ reason_str = drop_reason_lookup(cb->reason);
+ if (unlikely(!reason_str))
+ reason_str = drop_reason_lookup(SKB_DROP_REASON_NOT_SPECIFIED);
+ if (nla_put_string(msg, NET_DM_ATTR_REASON, reason_str)) {
rcu_read_unlock();
goto nla_put_failure;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8c8f7a05dea4..c675d3330937 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -148,13 +148,11 @@ static const struct drop_reason_list drop_reasons_core = {
.n_reasons = ARRAY_SIZE(drop_reasons),
};
-const struct drop_reason_list __rcu *
+static const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
[SKB_DROP_REASON_SUBSYS_CORE] = RCU_INITIALIZER(&drop_reasons_core),
};
-EXPORT_SYMBOL(drop_reasons_by_subsys);
-#ifdef CONFIG_TRACEPOINTS
const char *drop_reason_lookup(unsigned long long value)
{
unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
@@ -171,7 +169,9 @@ const char *drop_reason_lookup(unsigned long long value)
return NULL;
return subsys->reasons[reason];
}
+EXPORT_SYMBOL(drop_reason_lookup);
+#ifdef CONFIG_TRACEPOINTS
void drop_reason_show(struct seq_file *m)
{
u32 subsys_id;
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 0/4] tracing: improve symbolic printing
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
` (3 preceding siblings ...)
2026-09-22 11:08 ` [PATCH v7 4/4] net: drop_monitor: use drop_reason_lookup() Johannes Berg
@ 2026-11-05 14:12 ` Steven Rostedt
2026-11-05 15:04 ` Johannes Berg
4 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2026-11-05 14:12 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-trace-kernel, linux-kernel, netdev, Masami Hiramatsu,
Mathieu Desnoyers, Ilya Maximets, Matthieu Baerts, Aaron Conole,
Eelco Chaudron, dev, Adrian Moreno, Antoine Tenart
On Tue, 22 Sep 2026 13:08:39 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:
> Hi,
>
> Resend with the selftests and the two sashiko findings.
Hi Johannes,
Could you also add code in the samples/trace_events directory. Perhaps add
a new event?
This will make it easier to understand exactly how to add new __print_sym()s.
FYI, this series did pass all my tests.
Thanks,
-- Steve
>
> v2 was:
> - rebased on 6.9-rc1
> - always search for __print_sym() and get rid of the DYNPRINT flag
> and associated code; I think ideally we'll just remove the older
> __print_symbolic() entirely
> - use ':' as the separator instead of "//" since that makes searching
> for it much easier and it's still not a valid char in an identifier
> - fix RCU
>
> v3:
> - fix #undef issues
> - fix drop_monitor default
> - rebase on linux-trace/for-next (there were no conflicts)
> - move net patches to 3/4
> - clarify symbol name matching logic (and remove ")" from it)
>
> v4:
> - fix non-module build and possibly dynamic event handling
>
> v5: (https://lore.kernel.org/all/20240614081956.19832-6-johannes@sipsolutions.net/)
> - fix build warning in non-module build
>
> v6: (https://lore.kernel.org/linux-trace-kernel/20260921101538.97286-6-johannes@sipsolutions.net/)
> - rebase
> - fix nit from Paolo (I hope, I don't remember changing it
> but it looks fixed!)
> - fix crash due to wrong data placement
>
> v7: (this version)
> - update ovs selftest
> - don't ifdef drop_reason_lookup() prototype
> - fix section alignment
>
>
> To recap, it's annoying to have
>
> irq/65-iwlwifi:-401 [000] 22.790000: kfree_skb: ... reason: 0x20000
>
> and much nicer to see
>
> irq/65-iwlwifi:-401 [000] 22.790000: kfree_skb: ... reason: RX_DROP_MONITOR
>
> but this doesn't work now because __print_symbolic() can only
> deal with a hard-coded list (which is actually really big.)
>
> So here's __print_sym() which doesn't build the list into the
> kernel image, but creates it at runtime. For userspace, it
> will look the same as __print_symbolic() (it literally shows
> __print_symbolic() to userspace) so no changes are needed,
> but the actual list of values exposed to userspace in there
> is built dynamically. For SKB drop reasons, this then has all
> the reasons known when userspace queries the trace format.
>
> I guess patches 3/4 should go through net-next, so not sure
> how to handle this patch series. Or perhaps, as this will likely
> not cause conflicts (in fact I've been rebasing it for years now),
> go through tracing anyway with an Ack from netdev? But I can
> also just wait for the trace patch(es) to land and resubmit the
> net patches after. Assuming this looks good at all :-)
>
> Thanks,
> johannes
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 0/4] tracing: improve symbolic printing
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
0 siblings, 1 reply; 13+ messages in thread
From: Johannes Berg @ 2026-11-05 15:04 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, netdev, Masami Hiramatsu,
Mathieu Desnoyers, Ilya Maximets, Matthieu Baerts, Aaron Conole,
Eelco Chaudron, dev, Adrian Moreno, Antoine Tenart
On Thu, 2026-11-05 at 09:12 -0500, Steven Rostedt wrote:
> Could you also add code in the samples/trace_events directory. Perhaps add
> a new event?
>
> This will make it easier to understand exactly how to add new __print_sym()s.
I guess so. I was kinda hoping to simply convert all the others - it
clearly has advantages this way.
Any objection to simply converting the __print_symbolic() sample? It
would look something like this:
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -297,6 +297,28 @@ 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 */
+TRACE_DEFINE_SYM_LIST(sample_names,
+ { 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_names2_lookup(unsigned long long value);
+void sample_names2_show(struct seq_file *m);
+TRACE_DEFINE_SYM_FNS(sample_names2, sample_names2_lookup, sample_names2_show);
+
TRACE_EVENT(foo_bar,
TP_PROTO(const char *foo, int bar, const int *lst,
@@ -340,14 +362,10 @@ 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).
+ *
+ * See above for the declaration of "sample_names".
*/
- __print_symbolic(__entry->bar,
- { 0, "zero" },
- { TRACE_SAMPLE_FOO, "TWO" },
- { TRACE_SAMPLE_BAR, "FOUR" },
- { TRACE_SAMPLE_ZOO, "EIGHT" },
- { 10, "TEN" }
- ),
+ __print_sym(__entry->bar, sample_names),
/*
* __print_flags( variable, "delim", { value, "flag" }, ... ),
It did show a bug because TRACE_SYSTEM isn't a C var in that file (and
perhaps only in that file?), so I'll have to resend with that fixed
anyway.
> FYI, this series did pass all my tests.
Cool. Sorry the earlier (long time ago) version had the __init_data
issue, I'd mostly tested on UML and that wasn't protected there.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 0/4] tracing: improve symbolic printing
2026-11-05 15:04 ` Johannes Berg
@ 2026-11-05 16:24 ` Steven Rostedt
2026-11-05 16:26 ` Johannes Berg
0 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2026-11-05 16:24 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-trace-kernel, linux-kernel, netdev, Masami Hiramatsu,
Mathieu Desnoyers, Ilya Maximets, Matthieu Baerts, Aaron Conole,
Eelco Chaudron, dev, Adrian Moreno, Antoine Tenart
On Thu, 05 Nov 2026 16:04:21 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2026-11-05 at 09:12 -0500, Steven Rostedt wrote:
> > Could you also add code in the samples/trace_events directory. Perhaps add
> > a new event?
> >
> > This will make it easier to understand exactly how to add new __print_sym()s.
>
> I guess so. I was kinda hoping to simply convert all the others - it
> clearly has advantages this way.
>
> 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.
>
>
> --- a/samples/trace_events/trace-events-sample.h
> +++ b/samples/trace_events/trace-events-sample.h
> @@ -297,6 +297,28 @@ 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 */
> +TRACE_DEFINE_SYM_LIST(sample_names,
> + { 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_names2_lookup(unsigned long long value);
> +void sample_names2_show(struct seq_file *m);
Are the above going to be defined somewhere?
> +TRACE_DEFINE_SYM_FNS(sample_names2, sample_names2_lookup, sample_names2_show);
> +
> TRACE_EVENT(foo_bar,
>
> TP_PROTO(const char *foo, int bar, const int *lst,
> @@ -340,14 +362,10 @@ 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).
> + *
> + * See above for the declaration of "sample_names".
> */
> - __print_symbolic(__entry->bar,
> - { 0, "zero" },
> - { TRACE_SAMPLE_FOO, "TWO" },
> - { TRACE_SAMPLE_BAR, "FOUR" },
> - { TRACE_SAMPLE_ZOO, "EIGHT" },
> - { 10, "TEN" }
> - ),
> + __print_sym(__entry->bar, sample_names),
>
> /*
> * __print_flags( variable, "delim", { value, "flag" }, ... ),
>
>
>
> It did show a bug because TRACE_SYSTEM isn't a C var in that file (and
> perhaps only in that file?), so I'll have to resend with that fixed
> anyway.
>
>
> > FYI, this series did pass all my tests.
>
> Cool. Sorry the earlier (long time ago) version had the __init_data
> issue, I'd mostly tested on UML and that wasn't protected there.
>
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 0/4] tracing: improve symbolic printing
2026-11-05 16:24 ` Steven Rostedt
@ 2026-11-05 16:26 ` Johannes Berg
2026-11-05 16:38 ` Steven Rostedt
0 siblings, 1 reply; 13+ messages in thread
From: Johannes Berg @ 2026-11-05 16:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, netdev, Masami Hiramatsu,
Mathieu Desnoyers, Ilya Maximets, Matthieu Baerts, Aaron Conole,
Eelco Chaudron, dev, Adrian Moreno, Antoine Tenart
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.
> > +/*
> > + * 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_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?
I reckon most users won't even need the dynamic way, but obviously part
of the reason I'm doing this is that I _do_ need the dynamic way for the
drop reasons, which are registered/built at runtime.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 0/4] tracing: improve symbolic printing
2026-11-05 16:26 ` Johannes Berg
@ 2026-11-05 16:38 ` Steven Rostedt
2026-11-05 17:09 ` Johannes Berg
0 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2026-11-05 16:38 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-trace-kernel, linux-kernel, netdev, Masami Hiramatsu,
Mathieu Desnoyers, Ilya Maximets, Matthieu Baerts, Aaron Conole,
Eelco Chaudron, dev, Adrian Moreno, Antoine Tenart
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.
>
> > > +/*
> > > + * 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_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.
>
> I reckon most users won't even need the dynamic way, but obviously part
> of the reason I'm doing this is that I _do_ need the dynamic way for the
> drop reasons, which are registered/built at runtime.
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.
-- Steve
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 0/4] tracing: improve symbolic printing
2026-11-05 16:38 ` Steven Rostedt
@ 2026-11-05 17:09 ` Johannes Berg
0 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2026-11-05 17:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, netdev, Masami Hiramatsu,
Mathieu Desnoyers, Ilya Maximets, Matthieu Baerts, Aaron Conole,
Eelco Chaudron, dev, Adrian Moreno, Antoine Tenart
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" }, ... ),
*
^ permalink raw reply [flat|nested] 13+ messages in thread