diff mbox series

[RESEND] tracing: gfp: Remove duplication of recording GFP flags

Message ID 20250225135611.1942b65c@gandalf.local.home (mailing list archive)
State New
Headers show
Series [RESEND] tracing: gfp: Remove duplication of recording GFP flags | expand

Commit Message

Steven Rostedt Feb. 25, 2025, 6:56 p.m. UTC 
From: Steven Rostedt <rostedt@goodmis.org>

The gfp_flags when recorded in the trace require being converted from
their numbers to values. Various macros are used to help facilitate this,
but there's two sets of macros that need to keep track of the same GFP
flags to stay in sync.

Commit 60295b944ff68 ("tracing: gfp: Fix the GFP enum values shown for
user space tracing tools") added a TRACE_GFP_FLAGS macro that holds the
enum ___GFP_*_BIT defined bits, and creates the TRACE_DEFINE_ENUM()
wrapper around them.

The __def_gfpflag_names() macro creates the mapping of various flags or
multiple flags to give them human readable names via the __print_flags()
tracing helper macro.

As the TRACE_GFP_FLAGS is a subset of the __def_gfpflags_names(), it can
be used to cover the individual bit names, by redefining the internal
macro TRACE_GFP_EM():

  #undef TRACE_GFP_EM
  #define TRACE_GFP_EM(a) gfpflag_string(__GFP_##a),

This will remove the bits that are duplicate between the two macros. If a
new bit is created, only the TRACE_GFP_FLAGS needs to be updated and that
will also update the __def_gfpflags_names() macro.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Last version: https://lore.kernel.org/20250116214439.046082618@goodmis.org

  This was originally sent with a patch that fixed the output of gfp flags
  in trace events to show human readable flags and not hex numbers.

  This patch on the other hand is a clean up as the there's now two macros
  that define the bits to print. This makes the one macro use the other
  macro that is a subset of the first.

  Can someone in the memory management subsystem either give me an acked-by
  and I can take this through my tree, or you can just take this through
  the memory management tree. Either way works for me.

 include/trace/events/mmflags.h | 41 +++++++++-------------------------
 1 file changed, 10 insertions(+), 31 deletions(-)

Comments

Steven Rostedt Feb. 25, 2025, 7:56 p.m. UTC | #1 
On Tue, 25 Feb 2025 13:56:11 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Last version: https://lore.kernel.org/20250116214439.046082618@goodmis.org
> 
>   This was originally sent with a patch that fixed the output of gfp flags
>   in trace events to show human readable flags and not hex numbers.
> 
>   This patch on the other hand is a clean up as the there's now two macros
>   that define the bits to print. This makes the one macro use the other
>   macro that is a subset of the first.
> 
>   Can someone in the memory management subsystem either give me an acked-by
>   and I can take this through my tree, or you can just take this through
>   the memory management tree. Either way works for me.

Interesting, I even ran a before and after of this patch by doing the following:

 # trace-cmd start -e dma -e vmscan -e percpu -e kmem
 [ wait a few minutes ]

 # trace-cmd show |grep gfp > ~/gfp-before

 [ apply patch, compile, install, reboot ]

 # trace-cmd start -e dma -e vmscan -e percpu -e kmem
 [ wait a few minutes ]

 # trace-cmd show |grep gfp > ~/gfp-after

 # perl -e 'while (<>) { if (/gfp_flags=(\S+)/) { print "$1\n"; } }' < gfp-before  | sort -u  > /tmp/before.sort
 # perl -e 'while (<>) { if (/gfp_flags=(\S+)/) { print "$1\n"; } }' < gfp-after  | sort -u  > /tmp/after.sort

 # diff -u /tmp/before.sort /tmp/after.sort 
--- /tmp/before.sort	2025-02-25 14:41:49.799742048 -0500
+++ /tmp/after.sort	2025-02-25 14:41:41.247636893 -0500
@@ -4,38 +4,39 @@
 GFP_ATOMIC|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP
 GFP_ATOMIC|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC
 GFP_ATOMIC|__GFP_NOWARN|__GFP_NORETRY|__GFP_NOMEMALLOC
-GFP_ATOMIC|__GFP_ZERO|0x2000000
+GFP_ATOMIC|__GFP_ZERO|__GFP_COMP|__GFP_NO_OBJ_EXT
 GFP_HIGHUSER|__GFP_ACCOUNT
 GFP_HIGHUSER_MOVABLE|__GFP_COMP
-GFP_HIGHUSER_MOVABLE|__GFP_COMP|__GFP_WRITE
-GFP_HIGHUSER_MOVABLE|__GFP_COMP|__GFP_ZERO
+GFP_HIGHUSER_MOVABLE|__GFP_WRITE|__GFP_COMP
+GFP_HIGHUSER_MOVABLE|__GFP_ZERO|__GFP_COMP
 __GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC
-__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_RECLAIMABLE
-__GFP_IO|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_RECLAIMABLE
 GFP_KERNEL
 GFP_KERNEL_ACCOUNT
-GFP_KERNEL_ACCOUNT|__GFP_COMP|__GFP_ZERO
+GFP_KERNEL_ACCOUNT|__GFP_NOWARN|__GFP_NOMEMALLOC
 GFP_KERNEL_ACCOUNT|__GFP_ZERO
-GFP_KERNEL|__GFP_COMP|__GFP_ZERO|0x2000000
+GFP_KERNEL_ACCOUNT|__GFP_ZERO|__GFP_COMP
 GFP_KERNEL|__GFP_NOWARN|__GFP_NOMEMALLOC
-GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP
 GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY|__GFP_NOMEMALLOC
 GFP_KERNEL|__GFP_ZERO
+GFP_KERNEL|__GFP_ZERO|__GFP_COMP|__GFP_NO_OBJ_EXT
+GFP_KERNEL|__GFP_ZERO|__GFP_NO_OBJ_EXT
 GFP_NOFS
-GFP_NOFS|__GFP_COMP|__GFP_ZERO|0x2000000
-GFP_NOFS|__GFP_NOFAIL|__GFP_COMP|__GFP_HARDWALL|__GFP_MOVABLE
-GFP_NOFS|__GFP_NOFAIL|__GFP_ZERO
-GFP_NOFS|__GFP_NOFAIL|__GFP_ZERO|__GFP_ACCOUNT
-GFP_NOFS|__GFP_NOFAIL|__GFP_ZERO|__GFP_HARDWALL|__GFP_MOVABLE|__GFP_ACCOUNT
+GFP_NOFS|__GFP_MOVABLE|__GFP_NOFAIL|__GFP_COMP|__GFP_HARDWALL
+GFP_NOFS|__GFP_MOVABLE|__GFP_ZERO|__GFP_NOFAIL|__GFP_HARDWALL|__GFP_ACCOUNT
+GFP_NOFS|__GFP_NOFAIL
 GFP_NOFS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP
-GFP_NOFS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_RECLAIMABLE
 GFP_NOFS|__GFP_NOWARN|__GFP_NORETRY|__GFP_NOMEMALLOC
+GFP_NOFS|__GFP_RECLAIMABLE|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP
 GFP_NOFS|__GFP_ZERO
-GFP_NOFS|__GFP_ZERO|0x2000000
+GFP_NOFS|__GFP_ZERO|__GFP_NOFAIL
+GFP_NOFS|__GFP_ZERO|__GFP_NOFAIL|__GFP_ACCOUNT
+GFP_NOFS|__GFP_ZERO|__GFP_NO_OBJ_EXT
 GFP_NOWAIT
 GFP_NOWAIT|__GFP_ACCOUNT
-GFP_NOWAIT|__GFP_IO|__GFP_FS|__GFP_NORETRY|__GFP_COMP
+GFP_NOWAIT|__GFP_COMP
 GFP_NOWAIT|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC
-GFP_NOWAIT|__GFP_NORETRY|__GFP_COMP|__GFP_RECLAIMABLE
 GFP_NOWAIT|__GFP_NORETRY|__GFP_NOMEMALLOC
-GFP_NOWAIT|__GFP_ZERO|0x2000000
+GFP_NOWAIT|__GFP_RECLAIMABLE|__GFP_NORETRY|__GFP_COMP
+GFP_NOWAIT|__GFP_ZERO|__GFP_ACCOUNT
+GFP_NOWAIT|__GFP_ZERO|__GFP_NO_OBJ_EXT
+__GFP_RECLAIMABLE|__GFP_IO|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC

Notice that the old way has:

-GFP_NOFS|__GFP_COMP|__GFP_ZERO|0x2000000

And I looked at what that 0x2000000 is, and for my current config, it is:

  __GFP_NO_OBJ_EXT

Which was completely missing from the old way, and this patch actually
picks it up!

That's because the TRACE_GFP_FLAGS has it, but the __def_gfpflag_names
macro was missing it. Again, it's better to remove having to maintain two
lists instead of just one.

-- Steve
diff mbox series

Patch

diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index 72fbfe3caeaf..82371177ef79 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -78,6 +78,13 @@  TRACE_DEFINE_ENUM(___GFP_LAST_BIT);
 
 #define gfpflag_string(flag) {(__force unsigned long)flag, #flag}
 
+/*
+ * For the values that match the bits, use the TRACE_GFP_FLAGS
+ * which will allow any updates to be included automatically.
+ */
+#undef TRACE_GFP_EM
+#define TRACE_GFP_EM(a) gfpflag_string(__GFP_##a),
+
 #define __def_gfpflag_names			\
 	gfpflag_string(GFP_TRANSHUGE),		\
 	gfpflag_string(GFP_TRANSHUGE_LIGHT),	\
@@ -91,41 +98,13 @@  TRACE_DEFINE_ENUM(___GFP_LAST_BIT);
 	gfpflag_string(GFP_NOIO),		\
 	gfpflag_string(GFP_NOWAIT),		\
 	gfpflag_string(GFP_DMA),		\
-	gfpflag_string(__GFP_HIGHMEM),		\
 	gfpflag_string(GFP_DMA32),		\
-	gfpflag_string(__GFP_HIGH),		\
-	gfpflag_string(__GFP_IO),		\
-	gfpflag_string(__GFP_FS),		\
-	gfpflag_string(__GFP_NOWARN),		\
-	gfpflag_string(__GFP_RETRY_MAYFAIL),	\
-	gfpflag_string(__GFP_NOFAIL),		\
-	gfpflag_string(__GFP_NORETRY),		\
-	gfpflag_string(__GFP_COMP),		\
-	gfpflag_string(__GFP_ZERO),		\
-	gfpflag_string(__GFP_NOMEMALLOC),	\
-	gfpflag_string(__GFP_MEMALLOC),		\
-	gfpflag_string(__GFP_HARDWALL),		\
-	gfpflag_string(__GFP_THISNODE),		\
-	gfpflag_string(__GFP_RECLAIMABLE),	\
-	gfpflag_string(__GFP_MOVABLE),		\
-	gfpflag_string(__GFP_ACCOUNT),		\
-	gfpflag_string(__GFP_WRITE),		\
 	gfpflag_string(__GFP_RECLAIM),		\
-	gfpflag_string(__GFP_DIRECT_RECLAIM),	\
-	gfpflag_string(__GFP_KSWAPD_RECLAIM),	\
-	gfpflag_string(__GFP_ZEROTAGS)
-
-#ifdef CONFIG_KASAN_HW_TAGS
-#define __def_gfpflag_names_kasan ,			\
-	gfpflag_string(__GFP_SKIP_ZERO),		\
-	gfpflag_string(__GFP_SKIP_KASAN)
-#else
-#define __def_gfpflag_names_kasan
-#endif
+	TRACE_GFP_FLAGS				\
+	{ 0, "none" }
 
 #define show_gfp_flags(flags)						\
-	(flags) ? __print_flags(flags, "|",				\
-	__def_gfpflag_names __def_gfpflag_names_kasan			\
+	(flags) ? __print_flags(flags, "|", __def_gfpflag_names		\
 	) : "none"
 
 #ifdef CONFIG_MMU