On Mon, 22 Jun 2026 12:01:05 -0400
Yury Norov [off-list ref] wrote:

On Mon, Jun 22, 2026 at 09:07:41AM -0400, Steven Rostedt wrote:

quoted

From: Steven Rostedt <rostedt@goodmis.org>

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.

Link: https://lore.kernel.org/all/CAHk-=wikCBeVFjVXiY4o-oepdbjAoir5+TcAgtL12c4u1TpZLQ@mail.gmail.com/ (local)  

Link is nice, but can you explain in the commit message what those
complaints exactly are? There's enough opinions shared to make a nice
summary. I even think it's important enough to become a Documentation
rule.

What rule is that?

quoted

@@ -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>  

So, before it was included unconditionally, now it's included. It
looks technically correct, but conceptually - I'm not sure.

I'm not a developer of this driver, but ... here we need trace_printk.h
if TRACE_GTT is enabled, in the next header TRACE_GEM needs it. To me
it sounds like the whole driver simply needs trace_printk.h.

I just added it when trace_printk() is being used. Why else should it
be included when trace_printk() is not used. There's precedent to add
includes within #if blocks that contain code that requires the include
where nothing else needs it.

quoted

 #define GTT_TRACE(...) trace_printk(__VA_ARGS__)
 #else
 #define GTT_TRACE(...)

diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 1da8fb61c09e..f490052e8964 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h

@@ -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__);						\

diff --git a/drivers/hwtracing/stm/dummy_stm.c b/drivers/hwtracing/stm/dummy_stm.c
index 38528ffdc0b3..784f9af7ccba 100644
--- a/drivers/hwtracing/stm/dummy_stm.c
+++ b/drivers/hwtracing/stm/dummy_stm.c

@@ -14,6 +14,10 @@
 #include <linux/stm.h>
 #include <uapi/linux/stm.h>
 
+#ifdef DEBUG
+#include <linux/trace_printk.h>
+#endif
+  

Same here. The cost of adding the header in a particular C file is
unmeasurable. But playing "#undef DEBUG #ifdef DEBUG" games looks
weird.

This one I'll agree with you. I didn't like the if conditional, and
looking at it now, I think it's not needed.

But the first instance above, if it is possible to add the include
within the #if conditional where trace_printk() is used then why not add
the include then?

-- Steve