Re: [PATCH v2 2/2] tracing: Remove trace_printk.h from kernel.h

[Steven Rostedt <rostedt@kernel.org>]

On Mon, 22 Jun 2026 12:01:05 -0400
Yury Norov <yury.norov@gmail.com> wrote:

> On Mon, Jun 22, 2026 at 09:07:41AM -0400, Steven Rostedt wrote:
> > 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/  
> 
> 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?



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

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