* [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
@ 2026-04-29 16:57 Qian-Yu Lin
2026-04-29 17:42 ` Steven Rostedt
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Qian-Yu Lin @ 2026-04-29 16:57 UTC (permalink / raw)
To: rostedt, mhiramat
Cc: mathieu.desnoyers, linux-kernel, linux-trace-kernel, Qian-Yu Lin
The macro trace_printk() uses a hardcoded identifier _______STR
within a statement expression, which can lead to variable name
shadowing if a caller happens to use the same name in its scope.
Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
("min/max: remove sparse warnings when they're nested"), replace the
hardcoded identifier with __UNIQUE_ID(STR).
Since __UNIQUE_ID() must be expanded once to remain consistent across
declaration and sizeof() within the statement expression, introduce a
nested helper macro ___trace_printk.
Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
---
include/linux/trace_printk.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 2670ec7f4262..060eccb40838 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_TRACE_PRINTK_H
#define _LINUX_TRACE_PRINTK_H
+#include <linux/compiler.h>
#include <linux/compiler_attributes.h>
#include <linux/instruction_pointer.h>
#include <linux/stddef.h>
@@ -84,15 +85,18 @@ do { \
* let gcc optimize the rest.
*/
-#define trace_printk(fmt, ...) \
+#define ___trace_printk(fmt, str, ...) \
do { \
- char _______STR[] = __stringify((__VA_ARGS__)); \
- if (sizeof(_______STR) > 3) \
+ char str[] = __stringify((__VA_ARGS__)); \
+ if (sizeof(str) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__); \
else \
trace_puts(fmt); \
} while (0)
+#define trace_printk(fmt, ...) \
+ ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
+
#define do_trace_printk(fmt, args...) \
do { \
static const char *trace_printk_fmt __used \
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-04-29 16:57 [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR) Qian-Yu Lin
@ 2026-04-29 17:42 ` Steven Rostedt
2026-04-29 21:47 ` David Laight
2026-05-01 14:40 ` Qian-Yu Lin
2026-05-01 16:28 ` [PATCH v2] trace_printk: replace ___STR with compound literal Qian-Yu Lin
2026-05-02 7:55 ` [PATCH v3] trace_printk: remove local variable for argument detection Qian-Yu Lin
2 siblings, 2 replies; 12+ messages in thread
From: Steven Rostedt @ 2026-04-29 17:42 UTC (permalink / raw)
To: Qian-Yu Lin; +Cc: mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel
On Thu, 30 Apr 2026 00:57:07 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> The macro trace_printk() uses a hardcoded identifier _______STR
> within a statement expression, which can lead to variable name
> shadowing if a caller happens to use the same name in its scope.
Has this ever been a problem?
>
> Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> ("min/max: remove sparse warnings when they're nested"), replace the
> hardcoded identifier with __UNIQUE_ID(STR).
>
> Since __UNIQUE_ID() must be expanded once to remain consistent across
> declaration and sizeof() within the statement expression, introduce a
> nested helper macro ___trace_printk.
Hmm, so we are replacing one name with underscores with another name
with underscores?
>
> Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> ---
> include/linux/trace_printk.h | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> index 2670ec7f4262..060eccb40838 100644
> --- a/include/linux/trace_printk.h
> +++ b/include/linux/trace_printk.h
> @@ -2,6 +2,7 @@
> #ifndef _LINUX_TRACE_PRINTK_H
> #define _LINUX_TRACE_PRINTK_H
>
> +#include <linux/compiler.h>
People are already saying that trace_printk.h slows down the compile.
Does this add any overhead to the compile?
-- Steve
> #include <linux/compiler_attributes.h>
> #include <linux/instruction_pointer.h>
> #include <linux/stddef.h>
> @@ -84,15 +85,18 @@ do { \
> * let gcc optimize the rest.
> */
>
> -#define trace_printk(fmt, ...) \
> +#define ___trace_printk(fmt, str, ...) \
> do { \
> - char _______STR[] = __stringify((__VA_ARGS__)); \
> - if (sizeof(_______STR) > 3) \
> + char str[] = __stringify((__VA_ARGS__)); \
> + if (sizeof(str) > 3) \
> do_trace_printk(fmt, ##__VA_ARGS__); \
> else \
> trace_puts(fmt); \
> } while (0)
>
> +#define trace_printk(fmt, ...) \
> + ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> +
> #define do_trace_printk(fmt, args...) \
> do { \
> static const char *trace_printk_fmt __used \
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-04-29 17:42 ` Steven Rostedt
@ 2026-04-29 21:47 ` David Laight
2026-05-01 14:51 ` Qian-Yu Lin
2026-05-01 14:40 ` Qian-Yu Lin
1 sibling, 1 reply; 12+ messages in thread
From: David Laight @ 2026-04-29 21:47 UTC (permalink / raw)
To: Steven Rostedt
Cc: Qian-Yu Lin, mhiramat, mathieu.desnoyers, linux-kernel,
linux-trace-kernel
On Wed, 29 Apr 2026 13:42:26 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 30 Apr 2026 00:57:07 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
>
> > The macro trace_printk() uses a hardcoded identifier _______STR
> > within a statement expression, which can lead to variable name
> > shadowing if a caller happens to use the same name in its scope.
>
> Has this ever been a problem?
>
> >
> > Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> > and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> > ("min/max: remove sparse warnings when they're nested"), replace the
> > hardcoded identifier with __UNIQUE_ID(STR).
min and max do get nested - so it is important that the 'local'
variables have different names - otherwise you can get invalid expansions.
No one is going to have: trace_printk(fmt1, trace_printk(ftm2, ...), ...)
it just doesn't make sense.
There is a slight problem the ____________________________STR might
be used by an entirely different #define.
Just using _trace_printk_str[] would make this pretty unlikely.
It would also have the advantage of making the .i file a bit easier
to read, all the UNIQUE names in min/max output make it really hard
to see what the output actually means.
> >
> > Since __UNIQUE_ID() must be expanded once to remain consistent across
> > declaration and sizeof() within the statement expression, introduce a
> > nested helper macro ___trace_printk.
>
> Hmm, so we are replacing one name with underscores with another name
> with underscores?
>
> >
> > Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> > ---
> > include/linux/trace_printk.h | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> > index 2670ec7f4262..060eccb40838 100644
> > --- a/include/linux/trace_printk.h
> > +++ b/include/linux/trace_printk.h
> > @@ -2,6 +2,7 @@
> > #ifndef _LINUX_TRACE_PRINTK_H
> > #define _LINUX_TRACE_PRINTK_H
> >
> > +#include <linux/compiler.h>
>
> People are already saying that trace_printk.h slows down the compile.
> Does this add any overhead to the compile?
A little - nothing is free.
David
>
> -- Steve
>
>
> > #include <linux/compiler_attributes.h>
> > #include <linux/instruction_pointer.h>
> > #include <linux/stddef.h>
> > @@ -84,15 +85,18 @@ do { \
> > * let gcc optimize the rest.
> > */
> >
> > -#define trace_printk(fmt, ...) \
> > +#define ___trace_printk(fmt, str, ...) \
> > do { \
> > - char _______STR[] = __stringify((__VA_ARGS__)); \
> > - if (sizeof(_______STR) > 3) \
> > + char str[] = __stringify((__VA_ARGS__)); \
> > + if (sizeof(str) > 3) \
> > do_trace_printk(fmt, ##__VA_ARGS__); \
> > else \
> > trace_puts(fmt); \
> > } while (0)
> >
> > +#define trace_printk(fmt, ...) \
> > + ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > +
> > #define do_trace_printk(fmt, args...) \
> > do { \
> > static const char *trace_printk_fmt __used \
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-04-29 17:42 ` Steven Rostedt
2026-04-29 21:47 ` David Laight
@ 2026-05-01 14:40 ` Qian-Yu Lin
2026-05-01 15:19 ` Steven Rostedt
2026-05-01 21:13 ` David Laight
1 sibling, 2 replies; 12+ messages in thread
From: Qian-Yu Lin @ 2026-05-01 14:40 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, david.laight.linux, linux-kernel, linux-trace-kernel
On Wed, Apr 29, 2026 at 01:42:26PM -0400, Steven Rostedt wrote:
> On Thu, 30 Apr 2026 00:57:07 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
>
> > The macro trace_printk() uses a hardcoded identifier _______STR
> > within a statement expression, which can lead to variable name
> > shadowing if a caller happens to use the same name in its scope.
>
> Has this ever been a problem?
>
No, this has not been a practical problem. However, the long-underscore
name ___STR is an indication of a potential shadowing risk.
> >
> > Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> > and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> > ("min/max: remove sparse warnings when they're nested"), replace the
> > hardcoded identifier with __UNIQUE_ID(STR).
> >
> > Since __UNIQUE_ID() must be expanded once to remain consistent across
> > declaration and sizeof() within the statement expression, introduce a
> > nested helper macro ___trace_printk.
>
> Hmm, so we are replacing one name with underscores with another name
> with underscores?
>
> >
> > Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> > ---
> > include/linux/trace_printk.h | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> > index 2670ec7f4262..060eccb40838 100644
> > --- a/include/linux/trace_printk.h
> > +++ b/include/linux/trace_printk.h
> > @@ -2,6 +2,7 @@
> > #ifndef _LINUX_TRACE_PRINTK_H
> > #define _LINUX_TRACE_PRINTK_H
> >
> > +#include <linux/compiler.h>
>
> People are already saying that trace_printk.h slows down the compile.
> Does this add any overhead to the compile?
>
> -- Steve
Yes. I measured compile time of kernel/trace/ring_buffer_benchmark.o
after make clean on an x86_64 machine running Ubuntu 24.04 LTS:
- Original _______STR: 49.8s
- v1 with __UNIQUE_ID (compiler.h): 53.5s
- compound literal (no extra include): 33.2s
I propose using a compound literal in v2, which eliminates the local
variable entirely and requires no extra include:
#define trace_printk(fmt, ...) \
do { \
if (sizeof((char[]) \
{__stringify((__VA_ARGS__))}) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__); \
else \
trace_puts(fmt); \
} while (0)
This fully eliminates the shadowing risk without any compile overhead.
Qian-Yu
>
>
> > #include <linux/compiler_attributes.h>
> > #include <linux/instruction_pointer.h>
> > #include <linux/stddef.h>
> > @@ -84,15 +85,18 @@ do { \
> > * let gcc optimize the rest.
> > */
> >
> > -#define trace_printk(fmt, ...) \
> > +#define ___trace_printk(fmt, str, ...) \
> > do { \
> > - char _______STR[] = __stringify((__VA_ARGS__)); \
> > - if (sizeof(_______STR) > 3) \
> > + char str[] = __stringify((__VA_ARGS__)); \
> > + if (sizeof(str) > 3) \
> > do_trace_printk(fmt, ##__VA_ARGS__); \
> > else \
> > trace_puts(fmt); \
> > } while (0)
> >
> > +#define trace_printk(fmt, ...) \
> > + ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > +
> > #define do_trace_printk(fmt, args...) \
> > do { \
> > static const char *trace_printk_fmt __used \
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-04-29 21:47 ` David Laight
@ 2026-05-01 14:51 ` Qian-Yu Lin
0 siblings, 0 replies; 12+ messages in thread
From: Qian-Yu Lin @ 2026-05-01 14:51 UTC (permalink / raw)
To: David Laight; +Cc: rostedt, mhiramat, linux-kernel, linux-trace-kernel
On Wed, Apr 29, 2026 at 10:47:28PM +0100, David Laight wrote:
> On Wed, 29 Apr 2026 13:42:26 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Thu, 30 Apr 2026 00:57:07 +0800
> > Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> >
> > > The macro trace_printk() uses a hardcoded identifier _______STR
> > > within a statement expression, which can lead to variable name
> > > shadowing if a caller happens to use the same name in its scope.
> >
> > Has this ever been a problem?
> >
> > >
> > > Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> > > and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> > > ("min/max: remove sparse warnings when they're nested"), replace the
> > > hardcoded identifier with __UNIQUE_ID(STR).
>
> min and max do get nested - so it is important that the 'local'
> variables have different names - otherwise you can get invalid expansions.
>
> No one is going to have: trace_printk(fmt1, trace_printk(ftm2, ...), ...)
> it just doesn't make sense.
>
> There is a slight problem the ____________________________STR might
> be used by an entirely different #define.
> Just using _trace_printk_str[] would make this pretty unlikely.
> It would also have the advantage of making the .i file a bit easier
> to read, all the UNIQUE names in min/max output make it really hard
> to see what the output actually means.
>
Thank you for the suggestion. However, _trace_printk_str still carries
a theoretical shadowing risk if a caller happens to use the same name.
> > >
> > > Since __UNIQUE_ID() must be expanded once to remain consistent across
> > > declaration and sizeof() within the statement expression, introduce a
> > > nested helper macro ___trace_printk.
> >
> > Hmm, so we are replacing one name with underscores with another name
> > with underscores?
> >
> > >
> > > Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> > > ---
> > > include/linux/trace_printk.h | 10 +++++++---
> > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> > > index 2670ec7f4262..060eccb40838 100644
> > > --- a/include/linux/trace_printk.h
> > > +++ b/include/linux/trace_printk.h
> > > @@ -2,6 +2,7 @@
> > > #ifndef _LINUX_TRACE_PRINTK_H
> > > #define _LINUX_TRACE_PRINTK_H
> > >
> > > +#include <linux/compiler.h>
> >
> > People are already saying that trace_printk.h slows down the compile.
> > Does this add any overhead to the compile?
>
> A little - nothing is free.
>
> David
>
Indeed. I measured compile time of kernel/trace/ring_buffer_benchmark.o
after make clean on an x86_64 machine running Ubuntu 24.04 LTS:
- Original _______STR: 49.8s
- v1 with __UNIQUE_ID (compiler.h): 53.5s
- compound literal (no extra include): 33.2s
A compound literal eliminates the local variable entirely, removing
the shadowing risk without any extra include or compile overhead:
#define trace_printk(fmt, ...) \
do { \
if (sizeof((char[]) \
{__stringify((__VA_ARGS__))}) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__); \
else \
trace_puts(fmt); \
} while (0)
Qian-Yu
> >
> > -- Steve
> >
> >
> > > #include <linux/compiler_attributes.h>
> > > #include <linux/instruction_pointer.h>
> > > #include <linux/stddef.h>
> > > @@ -84,15 +85,18 @@ do { \
> > > * let gcc optimize the rest.
> > > */
> > >
> > > -#define trace_printk(fmt, ...) \
> > > +#define ___trace_printk(fmt, str, ...) \
> > > do { \
> > > - char _______STR[] = __stringify((__VA_ARGS__)); \
> > > - if (sizeof(_______STR) > 3) \
> > > + char str[] = __stringify((__VA_ARGS__)); \
> > > + if (sizeof(str) > 3) \
> > > do_trace_printk(fmt, ##__VA_ARGS__); \
> > > else \
> > > trace_puts(fmt); \
> > > } while (0)
> > >
> > > +#define trace_printk(fmt, ...) \
> > > + ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > > +
> > > #define do_trace_printk(fmt, args...) \
> > > do { \
> > > static const char *trace_printk_fmt __used \
> >
> >
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-05-01 14:40 ` Qian-Yu Lin
@ 2026-05-01 15:19 ` Steven Rostedt
2026-05-01 16:17 ` Qian-Yu Lin
2026-05-01 21:13 ` David Laight
1 sibling, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2026-05-01 15:19 UTC (permalink / raw)
To: Qian-Yu Lin
Cc: mhiramat, david.laight.linux, linux-kernel, linux-trace-kernel
On Fri, 1 May 2026 22:40:17 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> I propose using a compound literal in v2, which eliminates the local
> variable entirely and requires no extra include:
>
> #define trace_printk(fmt, ...) \
> do { \
> if (sizeof((char[]) \
> {__stringify((__VA_ARGS__))}) > 3) \
> do_trace_printk(fmt, ##__VA_ARGS__); \
> else \
> trace_puts(fmt); \
> } while (0)
>
> This fully eliminates the shadowing risk without any compile overhead.
Have you tested to make sure a string with no arguments still produces the
trace_puts() and one that has arguments calls do_trace_printk()?
I'm fine with that one if it still works.
-- Steve
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-05-01 15:19 ` Steven Rostedt
@ 2026-05-01 16:17 ` Qian-Yu Lin
2026-05-01 16:21 ` Steven Rostedt
0 siblings, 1 reply; 12+ messages in thread
From: Qian-Yu Lin @ 2026-05-01 16:17 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, david.laight.linux, linux-kernel, linux-trace-kernel
On Fri, May 01, 2026 at 11:19:39AM -0400, Steven Rostedt wrote:
> On Fri, 1 May 2026 22:40:17 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
>
> > I propose using a compound literal in v2, which eliminates the local
> > variable entirely and requires no extra include:
> >
> > #define trace_printk(fmt, ...) \
> > do { \
> > if (sizeof((char[]) \
> > {__stringify((__VA_ARGS__))}) > 3) \
> > do_trace_printk(fmt, ##__VA_ARGS__); \
> > else \
> > trace_puts(fmt); \
> > } while (0)
> >
> > This fully eliminates the shadowing risk without any compile overhead.
>
> Have you tested to make sure a string with no arguments still produces the
> trace_puts() and one that has arguments calls do_trace_printk()?
>
> I'm fine with that one if it still works.
>
> -- Steve
Yes, I verified it with the preprocessor output. I created a minimal
test file:
// kernel/trace/test_trace_printk.c
#include <linux/trace_printk.h>
void test(void) {
trace_printk("no args\n");
trace_printk("with arg %d\n", 42);
}
Then ran make kernel/trace/test_trace_printk.i.
The no-args case has sizeof((char[]){"()"}) which is 3, so 3 > 3 is
false and it falls through to trace_bputs/trace_puts.
The args case has sizeof((char[]){"(42)"}) which is 5, so 5 > 3 is
true and it goes to trace_bprintk/trace_printk with the argument
42 correctly passed through.
Qian-Yu
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-05-01 16:17 ` Qian-Yu Lin
@ 2026-05-01 16:21 ` Steven Rostedt
0 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2026-05-01 16:21 UTC (permalink / raw)
To: Qian-Yu Lin
Cc: mhiramat, david.laight.linux, linux-kernel, linux-trace-kernel
On Sat, 2 May 2026 00:17:50 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> Yes, I verified it with the preprocessor output. I created a minimal
> test file:
>
> // kernel/trace/test_trace_printk.c
> #include <linux/trace_printk.h>
>
> void test(void) {
> trace_printk("no args\n");
> trace_printk("with arg %d\n", 42);
> }
>
> Then ran make kernel/trace/test_trace_printk.i.
>
> The no-args case has sizeof((char[]){"()"}) which is 3, so 3 > 3 is
> false and it falls through to trace_bputs/trace_puts.
>
> The args case has sizeof((char[]){"(42)"}) which is 5, so 5 > 3 is
> true and it goes to trace_bprintk/trace_printk with the argument
> 42 correctly passed through.
Thanks. When I test, I do the same but just compile down to an object file
and run objdump and look at which functions were called.
-- Steve
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] trace_printk: replace ___STR with compound literal
2026-04-29 16:57 [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR) Qian-Yu Lin
2026-04-29 17:42 ` Steven Rostedt
@ 2026-05-01 16:28 ` Qian-Yu Lin
2026-05-02 7:55 ` [PATCH v3] trace_printk: remove local variable for argument detection Qian-Yu Lin
2 siblings, 0 replies; 12+ messages in thread
From: Qian-Yu Lin @ 2026-05-01 16:28 UTC (permalink / raw)
To: rostedt, mhiramat
Cc: mathieu.desnoyers, david.laight.linux, linux-kernel,
linux-trace-kernel, Qian-Yu Lin
The macro trace_printk() uses a hardcoded identifier ___STR
within a statement expression, which can lead to variable name
shadowing if a caller happens to use the same name in its scope.
Replace the hardcoded identifier with a compound literal, which
eliminates the local variable entirely and removes any shadowing
risk without requiring an extra include or helper macro.
Since trace_printk() is never nested, __UNIQUE_ID() provides no
practical benefit here. The compound literal approach is simpler
and has no compile time overhead, unlike the v1 approach which
added #include <linux/compiler.h>.
Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
---
include/linux/trace_printk.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 2670ec7f4262..7a4f6711834f 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -86,8 +86,7 @@ do { \
#define trace_printk(fmt, ...) \
do { \
- char _______STR[] = __stringify((__VA_ARGS__)); \
- if (sizeof(_______STR) > 3) \
+ if (sizeof((char[]){__stringify((__VA_ARGS__))}) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__); \
else \
trace_puts(fmt); \
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-05-01 14:40 ` Qian-Yu Lin
2026-05-01 15:19 ` Steven Rostedt
@ 2026-05-01 21:13 ` David Laight
2026-05-02 7:37 ` Qian-Yu Lin
1 sibling, 1 reply; 12+ messages in thread
From: David Laight @ 2026-05-01 21:13 UTC (permalink / raw)
To: Qian-Yu Lin; +Cc: Steven Rostedt, mhiramat, linux-kernel, linux-trace-kernel
On Fri, 1 May 2026 22:40:17 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:
...
> Yes. I measured compile time of kernel/trace/ring_buffer_benchmark.o
> after make clean on an x86_64 machine running Ubuntu 24.04 LTS:
>
> - Original _______STR: 49.8s
> - v1 with __UNIQUE_ID (compiler.h): 53.5s
> - compound literal (no extra include): 33.2s
That difference looks far to big to me.
And the times are far too large to be measuring the actual compile time.
>
> I propose using a compound literal in v2, which eliminates the local
> variable entirely and requires no extra include:
>
> #define trace_printk(fmt, ...) \
> do { \
> if (sizeof((char[]) \
> {__stringify((__VA_ARGS__))}) > 3) \
> do_trace_printk(fmt, ##__VA_ARGS__); \
There has to be a better way to align that code.
Although you should be able to use:
if (sizeof __stringify((__VA_ARGS__)) > 3)
(I've omitted one set of parenthesis for clarity)
You could change __stringify() to work with __VA_ARGS__ the you don't need
the extra (); this works fine:
#define _x(...) #__VA_ARGS__
#define x(...) _x(__VA_ARGS__)
#define z abcd
int a = sizeof x(z, v); /* 8 */
See: https://godbolt.org/z/zo4h4nr9b
-- David
> else \
> trace_puts(fmt); \
> } while (0)
>
> This fully eliminates the shadowing risk without any compile overhead.
>
> Qian-Yu
>
> >
> >
> > > #include <linux/compiler_attributes.h>
> > > #include <linux/instruction_pointer.h>
> > > #include <linux/stddef.h>
> > > @@ -84,15 +85,18 @@ do { \
> > > * let gcc optimize the rest.
> > > */
> > >
> > > -#define trace_printk(fmt, ...) \
> > > +#define ___trace_printk(fmt, str, ...) \
> > > do { \
> > > - char _______STR[] = __stringify((__VA_ARGS__)); \
> > > - if (sizeof(_______STR) > 3) \
> > > + char str[] = __stringify((__VA_ARGS__)); \
> > > + if (sizeof(str) > 3) \
> > > do_trace_printk(fmt, ##__VA_ARGS__); \
> > > else \
> > > trace_puts(fmt); \
> > > } while (0)
> > >
> > > +#define trace_printk(fmt, ...) \
> > > + ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > > +
> > > #define do_trace_printk(fmt, args...) \
> > > do { \
> > > static const char *trace_printk_fmt __used \
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)
2026-05-01 21:13 ` David Laight
@ 2026-05-02 7:37 ` Qian-Yu Lin
0 siblings, 0 replies; 12+ messages in thread
From: Qian-Yu Lin @ 2026-05-02 7:37 UTC (permalink / raw)
To: David Laight; +Cc: rostedt, mhiramat, linux-kernel, linux-trace-kernel
On Fri, May 01, 2026 at 10:13:15PM +0100, David Laight wrote:
> On Fri, 1 May 2026 22:40:17 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
>
> ...
> > Yes. I measured compile time of kernel/trace/ring_buffer_benchmark.o
> > after make clean on an x86_64 machine running Ubuntu 24.04 LTS:
> >
> > - Original _______STR: 49.8s
> > - v1 with __UNIQUE_ID (compiler.h): 53.5s
> > - compound literal (no extra include): 33.2s
>
> That difference looks far to big to me.
> And the times are far too large to be measuring the actual compile time.
>
You're right, my earlier measurements included dependency rebuilds
after make clean. I re-measured using touch to isolate the actual
compile time of ring_buffer_benchmark.o on x86_64:
- Original ___STR: 1.757s
- v1 with __UNIQUE_ID (compiler.h): 1.836s
- sizeof __stringify (your suggestion): 1.781s
> >
> > I propose using a compound literal in v2, which eliminates the local
> > variable entirely and requires no extra include:
> >
> > #define trace_printk(fmt, ...) \
> > do { \
> > if (sizeof((char[]) \
> > {__stringify((__VA_ARGS__))}) > 3) \
> > do_trace_printk(fmt, ##__VA_ARGS__); \
>
> There has to be a better way to align that code.
> Although you should be able to use:
> if (sizeof __stringify((__VA_ARGS__)) > 3)
> (I've omitted one set of parenthesis for clarity)
>
> You could change __stringify() to work with __VA_ARGS__ the you don't need
> the extra (); this works fine:
> #define _x(...) #__VA_ARGS__
> #define x(...) _x(__VA_ARGS__)
> #define z abcd
> int a = sizeof x(z, v); /* 8 */
> See: https://godbolt.org/z/zo4h4nr9b
>
> -- David
>
Yes, this works. I verified with objdump on the
samples/trace_printk module that all four cases branch correctly:
__trace_bputs, __trace_puts, __trace_bprintk, and __trace_printk.
I'll use this form in v3 since it's simpler than the compound literal.
> > else \
> > trace_puts(fmt); \
> > } while (0)
> >
> > This fully eliminates the shadowing risk without any compile overhead.
> >
> > Qian-Yu
> >
> > >
> > >
> > > > #include <linux/compiler_attributes.h>
> > > > #include <linux/instruction_pointer.h>
> > > > #include <linux/stddef.h>
> > > > @@ -84,15 +85,18 @@ do { \
> > > > * let gcc optimize the rest.
> > > > */
> > > >
> > > > -#define trace_printk(fmt, ...) \
> > > > +#define ___trace_printk(fmt, str, ...) \
> > > > do { \
> > > > - char _______STR[] = __stringify((__VA_ARGS__)); \
> > > > - if (sizeof(_______STR) > 3) \
> > > > + char str[] = __stringify((__VA_ARGS__)); \
> > > > + if (sizeof(str) > 3) \
> > > > do_trace_printk(fmt, ##__VA_ARGS__); \
> > > > else \
> > > > trace_puts(fmt); \
> > > > } while (0)
> > > >
> > > > +#define trace_printk(fmt, ...) \
> > > > + ___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > > > +
> > > > #define do_trace_printk(fmt, args...) \
> > > > do { \
> > > > static const char *trace_printk_fmt __used \
> > >
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] trace_printk: remove local variable for argument detection
2026-04-29 16:57 [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR) Qian-Yu Lin
2026-04-29 17:42 ` Steven Rostedt
2026-05-01 16:28 ` [PATCH v2] trace_printk: replace ___STR with compound literal Qian-Yu Lin
@ 2026-05-02 7:55 ` Qian-Yu Lin
2 siblings, 0 replies; 12+ messages in thread
From: Qian-Yu Lin @ 2026-05-02 7:55 UTC (permalink / raw)
To: rostedt
Cc: mhiramat, david.laight.linux, linux-kernel, linux-trace-kernel,
Qian-Yu Lin
The trace_printk() macro uses a local variable _______STR to detect
whether variadic arguments are present. This name can shadow outer
variables.
Replace the local variable with sizeof applied directly to the
stringified arguments:
if (sizeof __stringify((__VA_ARGS__)) > 3)
This eliminates the shadowing risk entirely without introducing
any additional includes or local variables.
Verified with objdump on samples/trace_printk that all four cases
branch correctly: __trace_bputs, __trace_puts, __trace_bprintk,
and __trace_printk.
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
---
include/linux/trace_printk.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 2670ec7f4262..3d54f440dccf 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -86,8 +86,7 @@ do { \
#define trace_printk(fmt, ...) \
do { \
- char _______STR[] = __stringify((__VA_ARGS__)); \
- if (sizeof(_______STR) > 3) \
+ if (sizeof __stringify((__VA_ARGS__)) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__); \
else \
trace_puts(fmt); \
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-05-02 7:56 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-29 16:57 [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR) Qian-Yu Lin
2026-04-29 17:42 ` Steven Rostedt
2026-04-29 21:47 ` David Laight
2026-05-01 14:51 ` Qian-Yu Lin
2026-05-01 14:40 ` Qian-Yu Lin
2026-05-01 15:19 ` Steven Rostedt
2026-05-01 16:17 ` Qian-Yu Lin
2026-05-01 16:21 ` Steven Rostedt
2026-05-01 21:13 ` David Laight
2026-05-02 7:37 ` Qian-Yu Lin
2026-05-01 16:28 ` [PATCH v2] trace_printk: replace ___STR with compound literal Qian-Yu Lin
2026-05-02 7:55 ` [PATCH v3] trace_printk: remove local variable for argument detection Qian-Yu Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox