tracing: Remove trace_printk.h from kernel.h

Remove trace_printk.h by creating a trace_controls.h for those places that need access to tracing prototypes like tracing_off() and for the places that need trace_printk() directly, to have it included directly.

Changse since v2: https://lore.kernel.org/all/[email protected]/

- Update change log in patch 1

- Remove #ifdef DEBUG and always include trace_printk.h in patch 2.

Steven Rostedt (2):
tracing: Move non-trace_printk prototypes into trace_controls.h
tracing: Remove trace_printk.h from kernel.h

----

 arch/powerpc/kvm/book3s_xics.c         |  1 +
 arch/powerpc/xmon/xmon.c               |  1 +
 arch/s390/kernel/ipl.c                 |  1 +
 arch/s390/kernel/machine_kexec.c       |  1 +
 drivers/gpu/drm/i915/gt/intel_gtt.h    |  1 +
 drivers/gpu/drm/i915/i915_gem.h        |  2 ++
 drivers/hwtracing/stm/dummy_stm.c      |  1 +
 drivers/infiniband/hw/hfi1/trace_dbg.h |  1 +
 drivers/tty/sysrq.c                    |  1 +
 drivers/usb/early/xhci-dbc.c           |  1 +
 fs/ext4/inline.c                       |  1 +
 include/linux/ftrace.h                 |  2 ++
 include/linux/kernel.h                 |  1 -
 include/linux/sunrpc/debug.h           |  1 +
 include/linux/trace_controls.h         | 54 ++++++++++++++++++++++++++++++++
 include/linux/trace_printk.h           | 56 ++--------------------------------
 kernel/debug/debug_core.c              |  1 +
 kernel/panic.c                         |  1 +
 kernel/rcu/rcu.h                       |  2 ++
 kernel/rcu/rcutorture.c                |  1 +
 kernel/trace/ring_buffer_benchmark.c   |  1 +
 kernel/trace/trace.h                   |  1 +
 kernel/trace/trace_benchmark.c         |  1 +
 lib/sys_info.c                         |  1 +
 samples/fprobe/fprobe_example.c        |  1 +
 samples/ftrace/ftrace-direct-too.c     |  1 -
 samples/trace_printk/trace-printk.c    |  1 +
 27 files changed, 83 insertions(+), 55 deletions(-)
 create mode 100644 include/linux/trace_controls.h

From: Steven Rostedt <[email protected]>

In order to remove the include to trace_printk.h from kernel.h the tracing control prototypes need to be separated into their own header file as they are used in other common header files like rcu.h. There's no point in removing trace_printk.h from kernel.h if it just gets added back to other common headers.

Prototypes are very cheap for the compiler and should not be an issue.

ftrace_dump() and trace_dump_stack() are also moved into trace_controls.h, as they are used in cases where things go wrong. The main use case is to do a trace_dump_stack(); tracing_off(); ftrace_dump(); in a place that detected that something went wrong, whereas, trace_printk() is added to normal code during debugging and removed before committing upstream. The dump code is fine to keep in production.

---
Changes since v2: https://patch.msgid.link/[email protected]

- Updated the change log

 arch/powerpc/xmon/xmon.c         |  1 +
 arch/s390/kernel/ipl.c           |  1 +
 arch/s390/kernel/machine_kexec.c |  1 +
 drivers/gpu/drm/i915/i915_gem.h  |  1 +
 drivers/tty/sysrq.c              |  1 +
 include/linux/trace_controls.h   | 54 ++++++++++++++++++++++++++++++++
 include/linux/trace_printk.h     | 51 ------------------------------
 kernel/debug/debug_core.c        |  1 +
 kernel/panic.c                   |  1 +
 kernel/rcu/rcu.h                 |  2 ++
 kernel/rcu/rcutorture.c          |  1 +
 kernel/trace/trace.h             |  1 +
 kernel/trace/trace_benchmark.c   |  1 +
 lib/sys_info.c                   |  1 +
 14 files changed, 67 insertions(+), 51 deletions(-)
 create mode 100644 include/linux/trace_controls.h

index cb3a3244ae6f..2135f319e0dd 100644
@@ -27,6 +27,7 @@
 #include <linux/highmem.h>
 #include <linux/security.h>
 #include <linux/debugfs.h>
+#include <linux/trace_controls.h>
 
 #include <asm/ptrace.h>
 #include <asm/smp.h>

index 3c346b02ceb9..baac66cc4de4 100644
@@ -22,6 +22,7 @@
 #include <linux/debug_locks.h>
 #include <linux/vmalloc.h>
 #include <linux/secure_boot.h>
+#include <linux/trace_controls.h>
 #include <asm/asm-extable.h>
 #include <asm/machine.h>
 #include <asm/diag.h>

index baeb3dcfc1c8..33f9a89eb3ad 100644
@@ -12,6 +12,7 @@
 #include <linux/delay.h>
 #include <linux/reboot.h>
 #include <linux/ftrace.h>
+#include <linux/trace_controls.h>
 #include <linux/debug_locks.h>
 #include <linux/cpufeature.h>
 #include <asm/guarded_storage.h>

index 20b3cb29cfff..1da8fb61c09e 100644
@@ -116,6 +116,7 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
 #endif
 
 #if IS_ENABLED(CONFIG_DRM_I915_TRACE_GEM)
+#include <linux/trace_controls.h>
 #define GEM_TRACE(...) trace_printk(__VA_ARGS__)
 #define GEM_TRACE_ERR(...) do {						\
 	pr_err(__VA_ARGS__);						\

index c2e4b31b699a..d3f72dc430b8 100644
@@ -324,6 +324,7 @@ static const struct sysrq_key_op sysrq_showstate_blocked_op = {
 };
 
 #ifdef CONFIG_TRACING
+#include <linux/trace_controls.h>
 #include <linux/ftrace.h>
 
 static void sysrq_ftrace_dump(u8 key)

index b276504c1c6b..f9c83a470c98 100644
@@ -27,6 +27,7 @@
 
 #define pr_fmt(fmt) "KGDB: " fmt
 
+#include <linux/trace_controls.h>
 #include <linux/pid_namespace.h>
 #include <linux/clocksource.h>
 #include <linux/serial_core.h>

index 213725b612aa..1415e910371d 100644
@@ -9,6 +9,7 @@
  * This function is used through-out the kernel (including mm and fs)
  * to indicate a major problem.
  */
+#include <linux/trace_controls.h>
 #include <linux/debug_locks.h>
 #include <linux/sched/debug.h>
 #include <linux/interrupt.h>

index fa6d30ce73d1..b3e2c8f25a4f 100644
@@ -280,6 +280,8 @@ extern int rcu_cpu_stall_notifiers;
 
 #ifdef CONFIG_RCU_STALL_COMMON
 
+#include <linux/trace_controls.h>
+
 extern int rcu_cpu_stall_ftrace_dump;
 extern int rcu_cpu_stall_suppress;
 extern int rcu_cpu_stall_timeout;

index 882a158ada7b..76bf0184b267 100644
@@ -39,6 +39,7 @@
 #include <linux/srcu.h>
 #include <linux/slab.h>
 #include <linux/trace_clock.h>
+#include <linux/trace_controls.h>
 #include <asm/byteorder.h>
 #include <linux/torture.h>
 #include <linux/vmalloc.h>

index 80fe152af1dd..2537c33ddd49 100644
@@ -22,6 +22,7 @@
 #include <linux/ctype.h>
 #include <linux/once_lite.h>
 #include <linux/ftrace_regs.h>
+#include <linux/trace_controls.h>
 #include <linux/llist.h>
 
 #include "pid_list.h"

index e19c32f2a938..69cc39008c36 100644
@@ -3,6 +3,7 @@
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/trace_clock.h>
+#include <linux/trace_controls.h>
 
 #define CREATE_TRACE_POINTS
 #include "trace_benchmark.h"

index f32a06ec9ed4..e3c9ca05601b 100644
@@ -8,6 +8,7 @@
 #include <linux/ftrace.h>
 #include <linux/nmi.h>
 #include <linux/sched/debug.h>
+#include <linux/trace_controls.h>
 #include <linux/string.h>
 #include <linux/sysctl.h>
 

2.53.0

From: Steven Rostedt <[email protected]>

There have been complaints about trace_printk.h causing more build time for being in kernel.h. Move it out of kernel.h and place it in the headers and C files that use it.

---
Changes since v2: https://patch.msgid.link/[email protected]

- Remove #ifdef DEBUG and just always include trace_printk.h in dummy_stm.c.

 arch/powerpc/kvm/book3s_xics.c         | 1 +
 drivers/gpu/drm/i915/gt/intel_gtt.h    | 1 +
 drivers/gpu/drm/i915/i915_gem.h        | 1 +
 drivers/hwtracing/stm/dummy_stm.c      | 1 +
 drivers/infiniband/hw/hfi1/trace_dbg.h | 1 +
 drivers/usb/early/xhci-dbc.c           | 1 +
 fs/ext4/inline.c                       | 1 +
 include/linux/ftrace.h                 | 2 ++
 include/linux/kernel.h                 | 1 -
 include/linux/sunrpc/debug.h           | 1 +
 include/linux/trace_printk.h           | 5 +++--
 kernel/trace/ring_buffer_benchmark.c   | 1 +
 samples/fprobe/fprobe_example.c        | 1 +
 samples/ftrace/ftrace-direct-too.c     | 1 -
 samples/trace_printk/trace-printk.c    | 1 +
 15 files changed, 16 insertions(+), 4 deletions(-)

index 74a44fa702b0..ef5eb596a56e 100644
@@ -26,6 +26,7 @@
 #if 1
 #define XICS_DBG(fmt...) do { } while (0)
 #else
+#include <linux/trace_printk.h>
 #define XICS_DBG(fmt...) trace_printk(fmt)
 #endif
 

index b54ee4f25af1..f6f223090760 100644
@@ -35,6 +35,7 @@
 #define I915_GFP_ALLOW_FAIL (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
 
 #if IS_ENABLED(CONFIG_DRM_I915_TRACE_GTT)
+#include <linux/trace_printk.h>
 #define GTT_TRACE(...) trace_printk(__VA_ARGS__)
 #else
 #define GTT_TRACE(...)

index 1da8fb61c09e..f490052e8964 100644
@@ -117,6 +117,7 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
 
 #if IS_ENABLED(CONFIG_DRM_I915_TRACE_GEM)
 #include <linux/trace_controls.h>
+#include <linux/trace_printk.h>
 #define GEM_TRACE(...) trace_printk(__VA_ARGS__)
 #define GEM_TRACE_ERR(...) do {						\
 	pr_err(__VA_ARGS__);						\

index 38528ffdc0b3..7c5e48ebfb9f 100644
@@ -8,6 +8,7 @@
  */
 
 #undef DEBUG
+#include <linux/trace_printk.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>

index 58304b91380f..30df5e246586 100644
@@ -103,6 +103,7 @@ __hfi1_trace_def(IOCTL);
  */
 
 #ifdef HFI1_EARLY_DBG
+#include <linux/trace_printk.h>
 #define hfi1_dbg_early(fmt, ...) \
 	trace_printk(fmt, ##__VA_ARGS__)
 #else

index 41118bba9197..955c73bd601f 100644
@@ -30,6 +30,7 @@ static struct xdbc_state xdbc;
 static bool early_console_keep;
 
 #ifdef XDBC_TRACE
+#include <linux/trace_printk.h>
 #define	xdbc_trace	trace_printk
 #else
 static inline void xdbc_trace(const char *fmt, ...) { }

index 8045e4ff270c..0eff4a0c6a6c 100644
@@ -934,6 +934,7 @@ static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
 }
 
 #ifdef INLINE_DIR_DEBUG
+#include <linux/trace_printk.h>
 void ext4_show_inline_dir(struct inode *dir, struct buffer_head *bh,
 			  void *inline_start, int inline_size)
 {

index 02bc5027523a..b5336a81e619 100644
@@ -8,6 +8,8 @@
 #define _LINUX_FTRACE_H
 
 #include <linux/trace_recursion.h>
+#include <linux/trace_controls.h>
+#include <linux/trace_printk.h>
 #include <linux/trace_clock.h>
 #include <linux/jump_label.h>
 #include <linux/kallsyms.h>

index e5570a16cbb1..e87a40fbd152 100644
@@ -31,7 +31,6 @@
 #include <linux/build_bug.h>
 #include <linux/sprintf.h>
 #include <linux/static_call_types.h>
-#include <linux/trace_printk.h>
 #include <linux/util_macros.h>
 #include <linux/wordpart.h>
 

index ab61bed2f7af..7524f5d82fba 100644
@@ -29,6 +29,7 @@ extern unsigned int		nlm_debug;
 # define ifdebug(fac)		if (unlikely(rpc_debug & RPCDBG_##fac))
 
 # if IS_ENABLED(CONFIG_SUNRPC_DEBUG_TRACE)
+#  include <linux/trace_printk.h>
 #  define __sunrpc_printk(fmt, ...)	trace_printk(fmt, ##__VA_ARGS__)
 # else
 #  define __sunrpc_printk(fmt, ...)	printk(KERN_DEFAULT fmt, ##__VA_ARGS__)

index a488ea9e9f85..74ce4f8995c4 100644
@@ -1,11 +1,12 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef _LINUX_TRACE_PRINTK_H
 #define _LINUX_TRACE_PRINTK_H
+#if !defined(__ASSEMBLY__) && !defined(__GENKSYMS__) && !defined(BUILD_VDSO)
 
-#include <linux/compiler_attributes.h>
 #include <linux/instruction_pointer.h>
 #include <linux/stddef.h>
 #include <linux/stringify.h>
+#include <linux/stdarg.h>
 
 #ifdef CONFIG_TRACING
 static inline __printf(1, 2)

@@ -147,5 +148,5 @@ ftrace_vprintk(const char *fmt, va_list ap)
 	return 0;
 }
 #endif /* CONFIG_TRACING */
-
+#endif /* !defined(__ASSEMBLY__) && !defined(__GENKSYMS__) && !defined(BUILD_VDSO) */
 #endif

index 593e3b59e42e..2bb25caebb75 100644
@@ -5,6 +5,7 @@
  * Copyright (C) 2009 Steven Rostedt <[email protected]>
  */
 #include <linux/ring_buffer.h>
+#include <linux/trace_printk.h>
 #include <linux/completion.h>
 #include <linux/kthread.h>
 #include <uapi/linux/sched/types.h>

index bfe98ce826f3..de81b9b4ca7d 100644
@@ -12,6 +12,7 @@
 
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
+#include <linux/trace_printk.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/fprobe.h>

index bf2411aa6fd7..159190f4103f 100644
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-only
 #include <linux/module.h>
-
 #include <linux/mm.h> /* for handle_mm_fault() */
 #include <linux/ftrace.h>
 #if !defined(CONFIG_ARM64) && !defined(CONFIG_PPC32)

index cfc159580263..ff37aeb8523e 100644
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-only
+#include <linux/trace_printk.h>
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/irq_work.h>

2.53.0

That is all about changes to the file causing everything to be rebuilt, not the contents of the file slowing down builds.
The two are different.

The part you are moving out of normal builds is just a few #defines. They won't have a significant effect on build times either.

So there is no point splitting out trace_controls.h.

David

I guess I should say it better. It causes more build time if that file changes. That's what I meant. I update the wording to say:

There have been complaints about trace_printk.h causing more build time for being in kernel.h it if changes. There is also an effort to clean up kernel.h to have it not include unneeded header files. Move trace_printk.h out of kernel.h and place it in the headers and C files that use it.

That is a completely different reason. trace_printk.h is about trace_printk() usage. The stuff split out into trace_controls.h have nothing to do with trace_printk()s.

-- Steve

True, but every header file costs extra time to open.
That could easily be more that the cost of parsing it (ok hand waving!). With a long list of -I parameters just finding a file costs because of all the failed opens.

I've just knocked it out of kernel.h, had to fix:
rcu.h
linux/ftrace.h
to make my 'normal' kernel build.
Lots of stuff includes the latter.

David

Hello,

This patch became commit 9cbc3d9806d3 ("tracing: Remove trace_printk.h from kernel.h") that currently is in next via

https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git trace/fixes

.

It creates a build regression for an ARCH=arc allmodconfig build:

CC lib/test_context-analysis.o
In file included from include/linux/local_lock_internal.h:8,
from include/linux/local_lock.h:5,
from lib/test_context-analysis.c:9:
include/linux/local_lock_internal.h: In function ‘local_lock_acquire’:
include/linux/lockdep.h:541:87: error: ‘_THIS_IP_’ undeclared (first use in this function)
541 | l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
| ^~~~~~~~~
include/linux/lockdep.h:509:88: note: in definition of macro ‘lock_acquire_exclusive’
509 | re_exclusive(l, s, t, n, i) lock_acquire(l, s, t, 0, 1, n, i)
| ^
include/linux/local_lock_internal.h:46:9: note: in expansion of macro ‘lock_map_acquire’
46 | lock_map_acquire(&l->dep_map);
| ^~~~~~~~~~~~~~~~
include/linux/lockdep.h:541:87: note: each undeclared identifier is reported only once for each function it appears in
541 | l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
| ^~~~~~~~~
include/linux/lockdep.h:509:88: note: in definition of macro ‘lock_acquire_exclusive’
509 | re_exclusive(l, s, t, n, i) lock_acquire(l, s, t, 0, 1, n, i)
| ^
include/linux/local_lock_internal.h:46:9: note: in expansion of macro ‘lock_map_acquire’
46 | lock_map_acquire(&l->dep_map);
| ^~~~~~~~~~~~~~~~
include/linux/local_lock_internal.h: In function ‘local_trylock_acquire’:
include/linux/lockdep.h:542:87: error: ‘_THIS_IP_’ undeclared (first use in this function)
542 | try(l) lock_acquire_exclusive(l, 0, 1, NULL, _THIS_IP_)
| ^~~~~~~~~
include/linux/lockdep.h:509:88: note: in definition of macro ‘lock_acquire_exclusive’
509 | re_exclusive(l, s, t, n, i) lock_acquire(l, s, t, 0, 1, n, i)
| ^
include/linux/local_lock_internal.h:53:9: note: in expansion of macro ‘lock_map_acquire_try’
53 | lock_map_acquire_try(&l->dep_map);
| ^~~~~~~~~~~~~~~~~~~~
include/linux/local_lock_internal.h: In function ‘local_lock_release’:
include/linux/lockdep.h:545:65: error: ‘_THIS_IP_’ undeclared (first use in this function)
545 | fine lock_map_release(l) lock_release(l, _THIS_IP_)
| ^~~~~~~~~
include/linux/local_lock_internal.h:62:9: note: in expansion of macro ‘lock_map_release’
62 | lock_map_release(&l->dep_map);
| ^~~~~~~~~~~~~~~~
make[5]: *** [scripts/Makefile.build:289: lib/test_context-analysis.o] Error 1
make[4]: *** [scripts/Makefile.build:549: lib] Error 2
make[3]: *** [Makefile:2184: .] Error 2
make[2]: *** [Makefile:372: __build_one_by_one] Error 2
make[1]: *** [Makefile:248: __sub-make] Error 2
make[1]: Leaving directory '/home/uwe/work/kbuild/arc'
make: *** [Makefile:248: __sub-make] Error 2

#regzbot introduced: 9cbc3d9806d3

Best regards
Uwe

Thanks for letting me know. I'll remove it and push it up.

I forgot I had it in there :-p

-- Steve

That sounds reasonable. Thank you for doing it.

Sebastian

Hi Steven,

Any progress on it? I still see linux/trace_printk.h in 7.3-rc1.

Thanks,
Yury

The merge window just ended. I'll start working on it during this cycle. I had other issues come up that needed to be dealt with first.

-- Steve