tracepoints: Add warnings for unused tracepoints and trace events

Every trace event can take up to 5K of memory in text and metadata regardless if they are used or not. Trace events should not be created if they are not used. Currently there's over a hundred events in the kernel that are defined but unused, either because their callers were removed without removing the trace event with it, or a config hides the trace event caller but not the trace event itself. And in some cases, trace events were simply added but were never called for whatever reason. The number of unused trace events continues to grow.

This patch series aims to fix this.

The first patch moves the elf parsing out of sorttable.c so that it can be used by other tooling.

The second patch creates a new program to run during build called tracepoint-update (note this may be extended to do other tracepoint modifications in the future). It also creates a new section called __tracepoint_check, where all callers of a tracepoint creates a variable that is placed in this section with the name of the tracepoint they use. The scripts/tracepoint-update.c is used to find tracepoints that are defined but not used which would mean they would not be in the __tracepoint_check section. It sorts it, and then reads the __tracepoint_strings section that has all compiled in tracepoint names. It makes sure that every tracepoint is found in the check section and if not, it prints a warning message about it. This lists the missing tracepoints at build time.

The third patch adds EXPORT_TRACEPOINT() to the __tracepoint_check section as well. There was several locations that adds tracepoints in the kernel proper that are only used in modules. It was getting quite complex trying to move things around that I just decided to make any tracepoint in a EXPORT_TRACEPOINT "used". I'm using the analogy of static and global functions. An unused static function gets a warning but an unused global one does not.

Changes since v4: https://lore.kernel.org/linux-trace-kernel/[email protected]/

- Took out the last patch that updated trace_ftrace_test_filter boot up self test as it was agnostic to the rest of the patches. I just queued it up by itself.

- Added elf-parse.c that takes the elf parsing code out of sorttable.c so that it can be used with tracepoint-update.c.

- Use the tracepoint name instead of the pointer to map the checks to the tracepoints (Mathieu Desnoyers)

- Create a new tool "tracepoint-update" instead of hacking sorttable

- Move the __tracepoint_check into the discard section so it is never loaded into memory. (Linus Torvalds)

- Parse the vmlinux.o instead of vmlinux as the discard section is removed between those two steps.

Steven Rostedt (3):
sorttable: Move ELF parsing into scripts/elf-parse.[ch]
tracing: Add a tracepoint verification check at build time
tracepoint: Do not warn for unused event that is exported

----

 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/tracepoint.h        |  12 +
 kernel/trace/Kconfig              |  10 +
 scripts/Makefile                  |   6 +
 scripts/elf-parse.c               | 198 ++++++++++++++++
 scripts/elf-parse.h               | 306 ++++++++++++++++++++++++
 scripts/link-vmlinux.sh           |   4 +
 scripts/sorttable.c               | 477 +++-----------------------------------
 scripts/tracepoint-update.c       | 217 +++++++++++++++++
 9 files changed, 788 insertions(+), 443 deletions(-)
 create mode 100644 scripts/elf-parse.c
 create mode 100644 scripts/elf-parse.h
 create mode 100644 scripts/tracepoint-update.c

From: Steven Rostedt <[email protected]>

In order to share the elf parsing that is in sorttable.c so that other programs could use the same code, move it into elf-parse.c and elf-parse.h.

---
 scripts/Makefile    |   3 +
 scripts/elf-parse.c | 198 ++++++++++++++++++
 scripts/elf-parse.h | 306 ++++++++++++++++++++++++++++
 scripts/sorttable.c | 477 ++++----------------------------------------
 4 files changed, 541 insertions(+), 443 deletions(-)
 create mode 100644 scripts/elf-parse.c
 create mode 100644 scripts/elf-parse.h

index 46f860529df5..f19624b3ed92 100644
@@ -12,6 +12,8 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE)	+= insert-sys-cert
 hostprogs-always-$(CONFIG_RUST_KERNEL_DOCTESTS)		+= rustdoc_test_builder
 hostprogs-always-$(CONFIG_RUST_KERNEL_DOCTESTS)		+= rustdoc_test_gen
 
+sorttable-objs := sorttable.o elf-parse.o
+
 ifneq ($(or $(CONFIG_X86_64),$(CONFIG_X86_32)),)
 always-$(CONFIG_RUST)					+= target.json
 filechk_rust_target = $< < include/config/auto.conf

@@ -25,6 +27,7 @@ generate_rust_target-rust := y
 rustdoc_test_builder-rust := y
 rustdoc_test_gen-rust := y
 
+HOSTCFLAGS_elf-parse.o = -I$(srctree)/tools/include
 HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
 HOSTLDLIBS_sorttable = -lpthread
 HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include

2.47.2

From: Steven Rostedt <[email protected]>

If a tracepoint is defined via DECLARE_TRACE() or TRACE_EVENT() but never called (via the trace_<tracepoint>() function), its metadata is still around in memory and not discarded.

When created via TRACE_EVENT() the situation is worse because the TRACE_EVENT() creates metadata that can be around 5k per trace event. Having unused trace events causes several thousand of wasted bytes.

Add a verifier that injects a string of the name of the tracepoint it calls that is added to the discarded section "__tracepoint_check". For every builtin tracepoint, it's name (which is saved in the in-memory section "__tracepoint_strings") will have its name also in the "__tracepoint_check" section if it is used.

Add a new program that is run on build called tracepoint-update. This is executed on the vmlinux.o before the __tracepoint_check section is discarded (the section is discarded before vmlinux is created). This program will create an array of each string in the __tracepoint_check section and then sort it. Then it will walk the strings in the __tracepoint_strings section and do a binary search to check if its name is in the __tracepoint_check section. If it is not, then it is unused and a warning is printed.

Note, this currently only handles tracepoints that are builtin and not in modules.

Enabling this currently with a given config produces:

warning: tracepoint 'sched_move_numa' is unused.
warning: tracepoint 'sched_stick_numa' is unused.
warning: tracepoint 'sched_swap_numa' is unused.
warning: tracepoint 'pelt_hw_tp' is unused.
warning: tracepoint 'pelt_irq_tp' is unused.
warning: tracepoint 'rcu_preempt_task' is unused.
warning: tracepoint 'rcu_unlock_preempted_task' is unused.
warning: tracepoint 'xdp_bulk_tx' is unused.
warning: tracepoint 'xdp_redirect_map' is unused.
warning: tracepoint 'xdp_redirect_map_err' is unused.
warning: tracepoint 'vma_mas_szero' is unused.
warning: tracepoint 'vma_store' is unused.
warning: tracepoint 'hugepage_set_pmd' is unused.
warning: tracepoint 'hugepage_set_pud' is unused.
warning: tracepoint 'hugepage_update_pmd' is unused.
warning: tracepoint 'hugepage_update_pud' is unused.
warning: tracepoint 'block_rq_remap' is unused.
warning: tracepoint 'xhci_dbc_handle_event' is unused.
warning: tracepoint 'xhci_dbc_handle_transfer' is unused.
warning: tracepoint 'xhci_dbc_gadget_ep_queue' is unused.
warning: tracepoint 'xhci_dbc_alloc_request' is unused.
warning: tracepoint 'xhci_dbc_free_request' is unused.
warning: tracepoint 'xhci_dbc_queue_request' is unused.
warning: tracepoint 'xhci_dbc_giveback_request' is unused.
warning: tracepoint 'tcp_ao_wrong_maclen' is unused.
warning: tracepoint 'tcp_ao_mismatch' is unused.
warning: tracepoint 'tcp_ao_key_not_found' is unused.
warning: tracepoint 'tcp_ao_rnext_request' is unused.
warning: tracepoint 'tcp_ao_synack_no_key' is unused.
warning: tracepoint 'tcp_ao_snd_sne_update' is unused.
warning: tracepoint 'tcp_ao_rcv_sne_update' is unused.

Some of the above is totally unused but others are not used due to their "trace_" functions being inside configs, in which case, the defined tracepoints should also be inside those same configs. Others are architecture specific but defined in generic code, where they should either be moved to the architecture or be surrounded by #ifdef for the architectures they are for.

This tool could be updated to process modules in the future.

I'd like to thank Mathieu Desnoyers for suggesting using strings instead of pointers, as using pointers in vmlinux.o required handling relocations and it required implementing almost a full feature linker to do so.

---
Changes since v4: https://lore.kernel.org/[email protected]

- Use the tracepoint name instead of the pointer to map the checks to the tracepoints (Mathieu Desnoyers)

- Create a new tool "tracepoint-update" instead of hacking sorttable

- Move the __tracepoint_check into the discard section so it is never loaded into memory. (Linus Torvalds)

- Parse the vmlinux.o instead of vmlinux as the discard section is removed between those two steps.

 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/tracepoint.h        |  10 ++
 kernel/trace/Kconfig              |  10 ++
 scripts/Makefile                  |   3 +
 scripts/link-vmlinux.sh           |   4 +
 scripts/tracepoint-update.c       | 217 ++++++++++++++++++++++++++++++
 6 files changed, 245 insertions(+)
 create mode 100644 scripts/tracepoint-update.c

index fa5f19b8d53a..867ecdb16b5d 100644
@@ -1047,6 +1047,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 	*(.modinfo)							\
 	/* ld.bfd warns about .gnu.version* even when not emitted */	\
 	*(.gnu.version*)						\
+	*(__tracepoint_check)
 
 #define DISCARDS							\
 	/DISCARD/ : {							\

index 826ce3f8e1f8..71d2e085c49e 100644
@@ -221,6 +221,14 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 		__do_trace_##name(args);				\
 	}
 
+#ifdef CONFIG_TRACEPOINT_WARN_ON_UNUSED
+# define TRACEPOINT_CHECK(name)						\
+	static const char __used __section("__tracepoint_check") __trace_check[] = \
+		#name;
+#else
+# define TRACEPOINT_CHECK(tname)
+#endif
+
 /*
  * Make sure the alignment of the structure in the __tracepoints section will
  * not add unwanted padding between the beginning of the section and the

@@ -270,6 +278,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 	__DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), PARAMS(data_proto)) \
 	static inline void __do_trace_##name(proto)			\
 	{								\
+		TRACEPOINT_CHECK(name)					\
 		if (cond) {						\
 			guard(preempt_notrace)();			\
 			__DO_TRACE_CALL(name, TP_ARGS(args));		\

@@ -289,6 +298,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 	__DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), PARAMS(data_proto)) \
 	static inline void __do_trace_##name(proto)			\
 	{								\
+		TRACEPOINT_CHECK(name)					\
 		guard(rcu_tasks_trace)();				\
 		__DO_TRACE_CALL(name, TP_ARGS(args));			\
 	}								\

index 35448f7233fe..c63e47ff8d94 100644
@@ -1050,6 +1050,16 @@ config GCOV_PROFILE_FTRACE
 	  Note that on a kernel compiled with this config, ftrace will
 	  run significantly slower.
 
+config TRACEPOINT_WARN_ON_UNUSED
+	bool "Warn on build if a tracepoint is defined but not used"
+	depends on TRACEPOINTS
+	help
+	  This option checks if every builtin defined tracepoint is
+	  used in the code. If a tracepoint is defined but not used,
+	  it will waste memory as its metadata is still created.
+	  This will cause a warning at build time if it detects a
+	  tracepoint created but never called.
+
 config FTRACE_SELFTEST
 	bool
 

index f19624b3ed92..11bf1bbd8c5d 100644
@@ -11,8 +11,10 @@ hostprogs-always-$(CONFIG_MODULE_SIG_FORMAT)		+= sign-file
 hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE)	+= insert-sys-cert
 hostprogs-always-$(CONFIG_RUST_KERNEL_DOCTESTS)		+= rustdoc_test_builder
 hostprogs-always-$(CONFIG_RUST_KERNEL_DOCTESTS)		+= rustdoc_test_gen
+hostprogs-always-$(CONFIG_TRACEPOINT_WARN_ON_UNUSED)	+= tracepoint-update
 
 sorttable-objs := sorttable.o elf-parse.o
+tracepoint-update-objs := tracepoint-update.o elf-parse.o
 
 ifneq ($(or $(CONFIG_X86_64),$(CONFIG_X86_32)),)
 always-$(CONFIG_RUST)					+= target.json

@@ -27,6 +29,7 @@ generate_rust_target-rust := y
 rustdoc_test_builder-rust := y
 rustdoc_test_gen-rust := y
 
+HOSTCFLAGS_tracepoint-update.o = -I$(srctree)/tools/include
 HOSTCFLAGS_elf-parse.o = -I$(srctree)/tools/include
 HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
 HOSTLDLIBS_sorttable = -lpthread

index 51367c2bfc21..4934c5e6c0b1 100755
@@ -211,6 +211,10 @@ kallsymso=
 strip_debug=
 generate_map=
 
+if is_enabled CONFIG_TRACEPOINT_WARN_ON_UNUSED; then
+	${objtree}/scripts/tracepoint-update vmlinux.o
+fi
+
 if is_enabled CONFIG_KALLSYMS; then
 	true > .tmp_vmlinux0.syms
 	kallsyms .tmp_vmlinux0.syms .tmp_vmlinux0.kallsyms

2.47.2

From: Steven Rostedt <[email protected]>

There are a few generic events that may only be used by modules. They are defined and then set with EXPORT_TRACEPOINT*(). Mark events that are exported as being used, even though they still waste memory in the kernel proper.

---
 include/linux/tracepoint.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 71d2e085c49e..ec6827d00d79 100644
@@ -223,8 +223,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 
 #ifdef CONFIG_TRACEPOINT_WARN_ON_UNUSED
 # define TRACEPOINT_CHECK(name)						\
-	static const char __used __section("__tracepoint_check") __trace_check[] = \
-		#name;
+	static const char __used __section("__tracepoint_check")	\
+	__trace_check_##name[] = #name;
 #else
 # define TRACEPOINT_CHECK(tname)
 #endif

@@ -381,10 +381,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 	__DEFINE_TRACE_EXT(_name, NULL, PARAMS(_proto), PARAMS(_args));
 
 #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)				\
+	TRACEPOINT_CHECK(name)						\
 	EXPORT_SYMBOL_GPL(__tracepoint_##name);				\
 	EXPORT_SYMBOL_GPL(__traceiter_##name);				\
 	EXPORT_STATIC_CALL_GPL(tp_func_##name)
 #define EXPORT_TRACEPOINT_SYMBOL(name)					\
+	TRACEPOINT_CHECK(name)						\
 	EXPORT_SYMBOL(__tracepoint_##name);				\
 	EXPORT_SYMBOL(__traceiter_##name);				\
 	EXPORT_STATIC_CALL(tp_func_##name)

2.47.2

its

[...]

Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Weird Al would be ashamed of me!

Although, you shouldn't delete the word that comes next in your reply, as I can't tell if you were correct or not from the context you kept.

I'm guessing. But when I walked through it in the debugger, there was more zero length strings than strings with content. But it had all the strings I was looking for.

-- Steve