On Mon, 22 Jun 2026 12:01:05 -0400 Yury Norov <[email protected]> wrote:
> On Mon, Jun 22, 2026 at 09:07:41AM -0400, Steven Rostedt wrote: > > 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. > > > > Link: > > https://lore.kernel.org/all/CAHk-=wikcbevfjvxiy4o-oepdbjaoir5+tcagtl12c4u1tp...@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
