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 several 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 the names from that section, 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.

The last patch triggers warnings when a module defines a tracepoint but does not use it.

The fourth patch updates the tracepoint-update program to handle modules.

Instead of hiding this behind a config option, where allmodconfig can cause the warnings to trigger, and we don't want current warnings to suddenly appear. Have the warnings trigger by a new make command line:

make UT=1

This will enable the unused tracepoints warnings. Now this should not be an issue to upstream before all warnings are removed. When all current warnings are removed, we can then make this the default option where it will always cause the build to warn if there's a unused tracepoint defined. The UT=1 command line can then be removed.

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

- Add backslash to end of __tracepoint_check in vmlinux.lds.h (Nathan Chancellor)

- Put the enabling of this around ifdef CONFIG_TRACEPOINTS as the tracepoint-update is only compiled when that is enabled.

- Add comment in make help that the UT=1 option is only temporary and will be removed when all current unused tracepoints are eliminated.

- Make updating tracepoint-update.c to handle modules a separate patch. This also allowed to pass "--module" to it to denote that the object is a module so that it can handle it differently.

- Remove use of "" in Makefile script (Nathan Chancellor)

- Use $(objtree) instead of ${obtree} (Nathan Chancellor)

- Move the update to tracepoint-update.c to a separate patch (Nathan Chancellor)

- Remove unneeded "else" block for cmd_check_tracepoint define (Nicolas Schier)

Steven Rostedt (5):
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
tracing: Allow tracepoint-update.c to work with modules
tracing: Add warnings for unused tracepoints for modules

----

 Makefile                          |  21 ++
 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/tracepoint.h        |  13 ++
 scripts/Makefile                  |   6 +
 scripts/Makefile.modfinal         |   5 +
 scripts/elf-parse.c               | 198 ++++++++++++++++
 scripts/elf-parse.h               | 305 ++++++++++++++++++++++++
 scripts/link-vmlinux.sh           |   7 +
 scripts/sorttable.c               | 477 +++-----------------------------------
 scripts/tracepoint-update.c       | 261 +++++++++++++++++++++
 10 files changed, 851 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 | 305 ++++++++++++++++++++++++++++
 scripts/sorttable.c | 477 ++++----------------------------------------
 4 files changed, 540 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.51.0

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, its 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.

To enable this check, run the build with: make UT=1

Note, when all the existing unused tracepoints are removed from the build, the "UT=1" will be removed and this will always be enabled when tracepoints are configured to warn on any new tracepoints. The reason this isn't always enabled now is because it will introduce a lot of warnings for the current unused tracepoints, and all bisects would end at this commit for those warnings.

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

- Add backslash to end of __tracepoint_check in vmlinux.lds.h (Nathan Chancellor)

- Put the enabling of this around ifdef CONFIG_TRACEPOINTS as the tracepoint-update is only compiled when that is enabled.

- Add comment in make help that the UT=1 option is only temporary and will be removed when all current unused tracepoints are eliminated.

 Makefile                          |  21 +++
 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/tracepoint.h        |  11 ++
 scripts/Makefile                  |   3 +
 scripts/link-vmlinux.sh           |   7 +
 scripts/tracepoint-update.c       | 232 ++++++++++++++++++++++++++++++
 6 files changed, 275 insertions(+)
 create mode 100644 scripts/tracepoint-update.c

index d14824792227..9823c20c4278 100644
@@ -810,6 +810,25 @@ ifdef CONFIG_FUNCTION_TRACER
   CC_FLAGS_FTRACE := -pg
 endif
 
+ifdef CONFIG_TRACEPOINTS
+# To check for unused tracepoints (tracepoints that are defined but never
+# called), run with:
+#
+# make UT=1
+#
+# Each unused tracepoints can take up to 5KB of memory in the running kernel.
+# It is best to remove any that are not used.
+#
+# This command line option will be removed when all current unused
+# tracepoints are removed.
+
+ifeq ("$(origin UT)", "command line")
+  WARN_ON_UNUSED_TRACEPOINTS := $(UT)
+endif
+endif # CONFIG_TRACEPOINTS
+
+export WARN_ON_UNUSED_TRACEPOINTS
+
 include $(srctree)/arch/$(SRCARCH)/Makefile
 
 ifdef need-config

@@ -1772,6 +1791,8 @@ help:
 	@echo  '		c: extra checks in the configuration stage (Kconfig)'
 	@echo  '		e: warnings are being treated as errors'
 	@echo  '		Multiple levels can be combined with W=12 or W=123'
+	@echo  '  make UT=1   [targets] Warn if a tracepoint is defined but not used.'
+	@echo  '          [ This will be removed when all current unused tracepoints are eliminated. ]'
 	@$(if $(dtstree), \
 		echo '  make CHECK_DTBS=1 [targets] Check all generated dtb files against schema'; \
 		echo '         This can be applied both to "dtbs" and to individual "foo.dtb" targets' ; \

index 8a9a2e732a65..c510fb097a8c 100644
@@ -1048,6 +1048,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 	*(.no_trim_symbol)						\
 	/* ld.bfd warns about .gnu.version* even when not emitted */	\
 	*(.gnu.version*)						\
+	*(__tracepoint_check)						\
 
 #define DISCARDS							\
 	/DISCARD/ : {							\

index 826ce3f8e1f8..1e53d3626c78 100644
@@ -221,6 +221,15 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 		__do_trace_##name(args);				\
 	}
 
+/*
+ * When a tracepoint is used, it's name is added to the __tracepoint_check
+ * section. This section is only used at build time to make sure all
+ * defined tracepoints are used. It is discarded after the build.
+ */
+# define TRACEPOINT_CHECK(name)						\
+	static const char __used __section("__tracepoint_check") __trace_check[] = \
+		#name;
+
 /*
  * 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 +279,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 +299,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 f19624b3ed92..0941e5ce7b57 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_TRACEPOINTS)			+= 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 433849ff7529..d304029fa6da 100755
@@ -208,6 +208,13 @@ kallsymso=
 strip_debug=
 generate_map=
 
+# Use "make UT=1" to trigger warnings on unused tracepoints
+case "${WARN_ON_UNUSED_TRACEPOINTS}" in
+*1*)
+	${objtree}/scripts/tracepoint-update vmlinux.o
+	;;
+esac
+
 if is_enabled CONFIG_KALLSYMS; then
 	true > .tmp_vmlinux0.syms
 	kallsyms .tmp_vmlinux0.syms .tmp_vmlinux0.kallsyms

2.51.0

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 1e53d3626c78..8a56f3278b1b 100644
@@ -227,8 +227,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
  * defined tracepoints are used. It is discarded after the build.
  */
 # 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;
 
 /*
  * Make sure the alignment of the structure in the __tracepoints section will

@@ -382,10 +382,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.51.0

From: Steven Rostedt <[email protected]>

In order for tracepoint-update.c to work with modules, it cannot error out if both "__tracepoint_check" and "__tracepoints_strings" are not found. When enabled, the vmlinux.o may be required to have both, but modules only have these sections if they have tracepoints. Modules without tracepoints will not have either. They should not fail to build because of that.

If one section exists the other one should too. Note, if a module defines a tracepoint but doesn't use any, it can cause this to fail.

Add a new "--module" parameter to tracepoint-update to be used when running on module code. It will not error out if this is set and both sections are missing. If this is set, and only the "__tracepoint_check" section is missing, it means the module has defined tracepoints but none of them are used. In that case, it prints a warning that the module has only unused tracepoints and exits normally to not fail the build.

If the "__tracepoint_check" section exists but not the "__tracepoint_strings", then that is an error and should fail the build.

---
 scripts/tracepoint-update.c | 45 ++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 8 deletions(-)

index 6ec30f39d0ad..7f7d90df14ce 100644
@@ -112,7 +112,7 @@ static int find_event(const char *str, void *array, size_t size)
 	return bsearch(&str, array, size, sizeof(char *), compare_strings) != NULL;
 }
 
-static void check_tracepoints(struct elf_tracepoint *etrace)
+static void check_tracepoints(struct elf_tracepoint *etrace, const char *fname)
 {
 	Elf_Ehdr *ehdr = etrace->ehdr;
 	int len;

@@ -129,22 +129,26 @@ static void check_tracepoints(struct elf_tracepoint *etrace)
 		if (!len)
 			continue;
 		if (!find_event(str, etrace->array, etrace->count)) {
-			fprintf(stderr, "warning: tracepoint '%s' is unused.\n", str);
+			fprintf(stderr, "warning: tracepoint '%s' is unused", str);
+			if (fname)
+				fprintf(stderr, " in module %s\n", fname);
+			else
+				fprintf(stderr, "\n");
 		}
 	}
 
 	free(etrace->array);
 }
 
-static void *tracepoint_check(struct elf_tracepoint *etrace)
+static void *tracepoint_check(struct elf_tracepoint *etrace, const char *fname)
 {
 	make_trace_array(etrace);
-	check_tracepoints(etrace);
+	check_tracepoints(etrace, fname);
 
 	return NULL;
 }
 
-static int process_tracepoints(void *addr, char const *const fname)
+static int process_tracepoints(bool mod, void *addr, const char *fname)
 {
 	struct elf_tracepoint etrace = {0};
 	Elf_Ehdr *ehdr = addr;

@@ -188,7 +192,19 @@ static int process_tracepoints(void *addr, char const *const fname)
 		}
 	}
 
+	/*
+	 * Modules may not have either section. But if it has one section,
+	 * it should have both of them.
+	 */
+	if (mod && !check_data_sec && !tracepoint_data_sec)
+		return 0;
+
 	if (!check_data_sec) {
+		if (mod) {
+			fprintf(stderr, "warning: Module %s has only unused tracepoints\n", fname);
+			/* Do not fail build */
+			return 0;
+		}
 		fprintf(stderr,	"no __tracepoint_check in file: %s\n", fname);
 		return -1;
 	}

@@ -198,8 +214,11 @@ static int process_tracepoints(void *addr, char const *const fname)
 		return -1;
 	}
 
+	if (!mod)
+		fname = NULL;
+
 	etrace.ehdr = ehdr;
-	tracepoint_check(&etrace);
+	tracepoint_check(&etrace, fname);
 	return 0;
 }
 

@@ -208,9 +227,19 @@ int main(int argc, char *argv[])
 	int n_error = 0;
 	size_t size = 0;
 	void *addr = NULL;
+	bool mod = false;
+
+	if (argc > 1 && strcmp(argv[1], "--module") == 0) {
+		mod = true;
+		argc--;
+		argv++;
+	}
 
 	if (argc < 2) {
-		fprintf(stderr, "usage: tracepoint-update vmlinux...\n");
+		if (mod)
+			fprintf(stderr, "usage: tracepoint-update --module module...\n");
+		else
+			fprintf(stderr, "usage: tracepoint-update vmlinux...\n");
 		return 0;
 	}
 

@@ -222,7 +251,7 @@ int main(int argc, char *argv[])
 			continue;
 		}
 
-		if (process_tracepoints(addr, argv[i]))
+		if (process_tracepoints(mod, addr, argv[i]))
 			++n_error;
 
 		elf_unmap(addr, size);

2.51.0

From: Steven Rostedt <[email protected]>

If a modules has TRACE_EVENT() but does not use it, add a warning about it at build time.

Currently, the build must be made by adding "UT=1" to the make command line in order for this to trigger.

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

- Remove use of "" in Makefile script (Nathan Chancellor)

- Use $(objtree) instead of ${obtree} (Nathan Chancellor)

- Move the update to tracepoint-update.c to a separate patch (Nathan Chancellor)

- Remove unneeded "else" block for cmd_check_tracepoint define (Nicolas Schier)

- Added use of "--module" parameter.

 scripts/Makefile.modfinal | 5 +++++
 1 file changed, 5 insertions(+)

index 542ba462ed3e..149e12ff5700 100644
@@ -28,6 +28,10 @@ ccflags-remove-y := $(CC_FLAGS_CFI)
 .module-common.o: $(srctree)/scripts/module-common.c FORCE
 	$(call if_changed_rule,cc_o_c)
 
+ifneq ($(WARN_ON_UNUSED_TRACEPOINTS),)
+cmd_check_tracepoint = $(objtree)/scripts/tracepoint-update --module $<;
+endif
+
 quiet_cmd_ld_ko_o = LD [M]  $@
       cmd_ld_ko_o =							\
 	$(LD) -r $(KBUILD_LDFLAGS)					\

@@ -57,6 +61,7 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 	+$(if $(newer-prereqs),$(call cmd,btf_ko))
 endif
+	+$(call cmd,check_tracepoint)
 
 targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) .module-common.o
 

2.51.0