Re: [RESEND][PATCH] tracing: gfp: Remove duplication of recording GFP flags
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2025-03-13 16:53:20
Also in:
linux-mm, lkml
Subsystem:
the rest, tracing · Maintainers:
Linus Torvalds, Steven Rostedt, Masami Hiramatsu
On Thu, 13 Mar 2025 16:26:22 +0100 Petr Mladek [off-list ref] wrote:
quoted hunk ↗ jump to hunk
This causes regression in the printf selftest: # modprobe test_printf modprobe: ERROR: could not insert 'test_printf': Invalid argument # dmesg | tail [ 46.206779] test_printf: vsnprintf(buf, 256, "%pGg", ...) returned 15, expected 10 [ 46.208192] test_printf: vsnprintf(buf, 3, "%pGg", ...) returned 15, expected 10 [ 46.208196] test_printf: vsnprintf(buf, 0, "%pGg", ...) returned 15, expected 10 [ 46.208199] test_printf: kvasprintf(..., "%pGg", ...) returned 'none|0xfc000000', expected '0xfc000000' [ 46.208202] test_printf: vsnprintf(buf, 256, "%pGg", ...) returned 26, expected 21 [ 46.208204] test_printf: vsnprintf(buf, 17, "%pGg", ...) returned 26, expected 21 [ 46.208206] test_printf: vsnprintf(buf, 0, "%pGg", ...) returned 26, expected 21 [ 46.208209] test_printf: kvasprintf(..., "%pGg", ...) returned '__GFP_HIGH|none|0xfc000000', expected '__GFP_HIGH|0xfc000000' [ 46.208865] test_printf: failed 8 out of 448 tests => vprintf() started printing the "none|" string. It seems to me that "{ 0, "none" }" was added as an "innocent" entry to avoid the trailing "," generated by TRACE_GFP_FLAGS. So, it is not really needed. In fact, I think that it probably causes similar regression in the trace output because the logic in trace_print_flags_seq() seems to be the same as in format_flags() in lib/vsprintf.c. The following worked for me:diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h index 82371177ef79..15aae955a10b 100644 --- a/include/trace/events/mmflags.h +++ b/include/trace/events/mmflags.h@@ -101,7 +101,7 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); gfpflag_string(GFP_DMA32), \ gfpflag_string(__GFP_RECLAIM), \ TRACE_GFP_FLAGS \ - { 0, "none" } + { 0, NULL } #define show_gfp_flags(flags) \ (flags) ? __print_flags(flags, "|", __def_gfpflag_names \It seems to be safe because the callers end up the cycle when .name == NULL. I think that it actually allows to remove similar trailing {} but I am not sure if we want it.
Hmm, I could get rid of that last one with this patch. What do you think?
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index 82371177ef79..74af49c33d14 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h@@ -37,26 +37,26 @@ TRACE_GFP_EM(HARDWALL) \ TRACE_GFP_EM(THISNODE) \ TRACE_GFP_EM(ACCOUNT) \ - TRACE_GFP_EM(ZEROTAGS) + TRACE_GFP_EME(ZEROTAGS) #ifdef CONFIG_KASAN_HW_TAGS # define TRACE_GFP_FLAGS_KASAN \ - TRACE_GFP_EM(SKIP_ZERO) \ - TRACE_GFP_EM(SKIP_KASAN) + TRACE_COMMA TRACE_GFP_EM(SKIP_ZERO) \ + TRACE_GFP_EME(SKIP_KASAN) #else # define TRACE_GFP_FLAGS_KASAN #endif #ifdef CONFIG_LOCKDEP # define TRACE_GFP_FLAGS_LOCKDEP \ - TRACE_GFP_EM(NOLOCKDEP) + TRACE_COMMA TRACE_GFP_EME(NOLOCKDEP) #else # define TRACE_GFP_FLAGS_LOCKDEP #endif #ifdef CONFIG_SLAB_OBJ_EXT # define TRACE_GFP_FLAGS_SLAB \ - TRACE_GFP_EM(NO_OBJ_EXT) + TRACE_COMMA TRACE_GFP_EME(NO_OBJ_EXT) #else # define TRACE_GFP_FLAGS_SLAB #endif
@@ -69,6 +69,10 @@ #undef TRACE_GFP_EM #define TRACE_GFP_EM(a) TRACE_DEFINE_ENUM(___GFP_##a##_BIT); +#undef TRACE_GFP_EME +#define TRACE_GFP_EME(a) TRACE_DEFINE_ENUM(___GFP_##a##_BIT); +#undef TRACE_COMMA +#define TRACE_COMMA TRACE_GFP_FLAGS
@@ -84,6 +88,10 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); */ #undef TRACE_GFP_EM #define TRACE_GFP_EM(a) gfpflag_string(__GFP_##a), +#undef TRACE_GFP_EME +#define TRACE_GFP_EME(a) gfpflag_string(__GFP_##a) +#undef TRACE_COMMA +#define TRACE_COMMA , #define __def_gfpflag_names \ gfpflag_string(GFP_TRANSHUGE), \
@@ -100,8 +108,7 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); gfpflag_string(GFP_DMA), \ gfpflag_string(GFP_DMA32), \ gfpflag_string(__GFP_RECLAIM), \ - TRACE_GFP_FLAGS \ - { 0, "none" } + TRACE_GFP_FLAGS #define show_gfp_flags(flags) \ (flags) ? __print_flags(flags, "|", __def_gfpflag_names \ -- Steve