Message-ID: <20251230114608.4d016ad3@gandalf.local.home>
Date: Tue, 30 Dec 2025 11:46:08 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Yury Norov <yury.norov@...il.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Masami Hiramatsu <mhiramat@...nel.org>,
 Christophe Leroy <chleroy@...nel.org>, Randy Dunlap
 <rdunlap@...radead.org>, Ingo Molnar <mingo@...nel.org>, Jani Nikula
 <jani.nikula@...ux.intel.com>, Joonas Lahtinen
 <joonas.lahtinen@...ux.intel.com>, David Laight <david.laight@...box.com>,
 Petr Pavlu <petr.pavlu@...e.com>, Andi Shyti <andi.shyti@...nel.org>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>, Tvrtko Ursulin
 <tursulin@...ulin.net>, Daniel Gomez <da.gomez@...nel.org>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
 <rafael@...nel.org>, Danilo Krummrich <dakr@...nel.org>,
 linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-modules@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h

On Tue, 30 Dec 2025 11:18:37 -0500
Yury Norov <yury.norov@...il.com> wrote:

> On Tue, Dec 30, 2025 at 09:21:00AM -0500, Mathieu Desnoyers wrote:
> > On 2025-12-30 03:55, Andy Shevchenko wrote:  
> > > On Mon, Dec 29, 2025 at 05:25:08PM -0500, Mathieu Desnoyers wrote:
> > > 
> > > ...
> > >   
> > > > One possible compromise would be to move it to its own header file,
> > > > and introduce a CONFIG_TRACE_PRINTK Kconfig option (default Y) that
> > > > would surround an include from linux/kernel.h with a preprocessor
> > > > conditional.  
> 
> We already have CONFIG_TRACING, and everything in the new
> trace_printk.h is conditional on it. We can protect the header in
> kernel.h with the same config.

Tracing is used in production all the time. So I think we can have a new
config just for trace_printk(). I was actually thinking of adding a
CONFIG_HIDE_TRACE_PRINTK, with the description of:

  trace_printk() is an extremely powerful utility to debug and develop
  kernel code. It is defined in kernel.h so that it can be easily accessed
  during development or having to debug existing code.

  But trace_printk() is not to be included in the final result, and having
  it in kernel.h during normal builds where the builder has no plans of
  debugging the kernel causes wasted cycles and time in compiling the kernel.

  By saying yes here, the include of trace_printk() macros will be hidden
  from kernel.h and help speed up the compile.

  If you do not plan on debugging this kernel, say Y

And then have in kernel.h:

#ifndef CONFIG_HIDE_TRACE_PRINTK
# include <linux/trace_printk.h>
#endif

This also means it gets set for allyesconfig builds, which I doubt anyone
wants to debug anyway.

> 
> > > > But please make sure the default stays as it is today:
> > > > include the trace printk header by default.  
> > > 
> > > "by default" where exactly?  
> 
> Seemingly nowhere.
> 
> > > The problem is that kernel.h is a total mess and
> > > it's included in a lot of mysterious ways (indirectly),  
> 
> Yes!
> 
> > > and in C you _must_
> > > include a header anyway for a custom API, just define *which* one.  
> >
> > This patch series moves the guts of trace_printk into its own header
> > file, which reduces clutter. So that's already progress. :)
> >   
> > > 
> > > Based on the Steven's first replies I see a compromise in having it inside
> > > printk.h. If you want to debug something with printf() (in general) the same
> > > header should provide all species. Do you agree?  
>  
> It may sound logical, but I don't like this idea. Printk() is used
> for debugging by everyone, but its main goal is to communicate to
> userspace and between different parts of the kernel. Notice how all
> debugging and development API in linux/pritnk.h is protected with the
> corresponding ifdefery. 
> 
> Contrary to that, trace_printk() is a purely debugging feature. There's
> no use for it after the debugging is done. (Or I missed something?)

I actually agree with you here. I don't think adding trace_printk.h into
printk.h is appropriate. I only said that anywhere you can add a printk()
for debugging, you should also be able to add trace_printk(). I believe
kernel.h is the appropriate place for both.

> 
> Everyone admits that kernel.h is a mess. Particularly, it's a mess of
> development and production features. So, moving trace_printk() from an
> already messy kernel.h to a less messy printk.h - to me it looks like
> spreading the mess.
> 
> > I don't have a strong opinion about including trace_printk.h from either
> > kernel.h or printk.h. As long as it's still included by a default kernel
> > config the same way it has been documented/used since 2009.  
> 
> Can you please point to the documentation and quote the exact piece
> stating that? Git history points to the commit 40ada30f9621f from Ingo
> that decouples tracers from DEBUG_KERNEL, and the following 422d3c7a577
> from Kosaki that force-enables the new TRACING_SUPPORT regardless of
> the DEBUG_KERNEL state.
> 
> To me, decoupling tracing from DEBUG_KERNEL looks accidental rather than
> intentional. So maybe simply restore that dependency?

Absolutely not. Tracing is used to debug production kernels, and things
like live kernel patching also depend on it, not to mention BPF.

> 
> Currently, even with tinyconfig, DEBUG_KERNEL is enabled (via EXPERT).
> And even if EXPERT and DEBUG_KERNEL are off, tracers are still enabled.
> This doesn't look right...

Looks fine to me.

-- Steve

Powered by blists - more mailing lists