[PATCH 0/6] Fix enum values print in tracepoints

[PATCH 0/6] Fix enum values print in tracepoints

[Nikolay Borisov]

While looking at tracepoint dumps with trace-cmd report I observed that
tracepoints that should have printed text instead of raw values weren't
doing so:

13 kworker/u8:1-61    [000]    66.299527: btrfs_flush_space:    5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0

In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e
the correct output should be:

6 fio-370   [002]    56.762402: btrfs_trigger_flush:  d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360

Investigating this turned out to be caused  because enum values weren't exported
to user space via TRACE_DEFINE_ENUM. This is required in order for user space
tools to correctly map the raw binary values to their textual representation.
More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space")

This series follows the approach taken by 190f0b76ca49 in defining the various
enum mapping structures.

Nikolay Borisov (6):
  btrfs: tracepoints: Fix btrfs_trigger_flush printout
  btrfs: tracepoints: Fix extent type symbolic name print
  btrfs: tracepoints: Move FLUSH_ACTIONS define
  btrfs: tracepoints: Fix qgroup reservation type printing
  btrfs: tracepoints: Switch extent_io_tree_owner to using EM macro
  btrfs: tracepoints: Convert flush states to using EM macros

 include/trace/events/btrfs.h | 128 +++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 60 deletions(-)

--
2.17.1

[PATCH 1/6] btrfs: tracepoints: Fix btrfs_trigger_flush printout

[Nikolay Borisov]

When tracepoints use __print_symbolic to print textual representation of
a value that comes from an ENUM each enum value needs to be exported
to user space so that user space tools can convert the binary value
data to the trings as user space does not know what those enums are
about.

Doing a trace-cmd record && trace-cmd report currently results in:

kworker/u8:1-61    [000]    66.299527: btrfs_flush_space:    5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0

I.e state is not translated to its symbolic counterpart. With this patch
applied the output is:

fio-370   [002]    56.762402: btrfs_trigger_flush:  d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 include/trace/events/btrfs.h | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index b4453fbbfa4b..2615c7181e8c 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -1042,11 +1042,24 @@ TRACE_EVENT(btrfs_space_reservation,
 			__entry->bytes)
 );
 
-#define show_flush_action(action)						\
-	__print_symbolic(action,						\
-		{ BTRFS_RESERVE_NO_FLUSH,	"BTRFS_RESERVE_NO_FLUSH"},	\
-		{ BTRFS_RESERVE_FLUSH_LIMIT,	"BTRFS_RESERVE_FLUSH_LIMIT"},	\
-		{ BTRFS_RESERVE_FLUSH_ALL,	"BTRFS_RESERVE_FLUSH_ALL"})
+#define FLUSH_ACTIONS								\
+	EM (BTRFS_RESERVE_NO_FLUSH,		"BTRFS_RESERVE_NO_FLUSH")	\
+	EM (BTRFS_RESERVE_FLUSH_LIMIT,		"BTRFS_RESERVE_FLUSH_LIMIT")	\
+	EM (BTRFS_RESERVE_FLUSH_ALL,		"BTRFS_RESERVE_FLUSH_ALL")	\
+	EMe (BTRFS_RESERVE_FLUSH_ALL_STEAL,	"BTRFS_RESERVE_FLUSH_ALL_STEAL")
+
+#undef EM
+#undef EMe
+#define EM(a, b) TRACE_DEFINE_ENUM(a);
+#define EMe(a, b) TRACE_DEFINE_ENUM(a);
+
+FLUSH_ACTIONS
+
+#undef EM
+#undef EMe
+
+#define EM(a, b)        {a, b},
+#define EMe(a, b)       {a, b}
 
 TRACE_EVENT(btrfs_trigger_flush,
 
@@ -1071,7 +1084,7 @@ TRACE_EVENT(btrfs_trigger_flush,
 
 	TP_printk_btrfs("%s: flush=%d(%s) flags=%llu(%s) bytes=%llu",
 		  __get_str(reason), __entry->flush,
-		  show_flush_action(__entry->flush),
+		  __print_symbolic(__entry->flush, FLUSH_ACTIONS),
 		  __entry->flags,
 		  __print_flags((unsigned long)__entry->flags, "|",
 				BTRFS_GROUP_FLAGS),
-- 
2.17.1

[PATCH 2/6] btrfs: tracepoints: Fix extent type symbolic name print

[Nikolay Borisov]

extent's type is an enum and this requires that the enum values be
exported to user space so that user space tools can correctly map raw
binary data to the symbolic name. Currently tracepoints using
btrfs__file_extent_item_regular or btrfs__file_extent_item_inline result
in the following output:

fio-443   [002]   586.609450: btrfs_get_extent_show_fi_regular: f0c3bf8e-0174-4bcc-92aa-6c2d62430420:i
root=5(FS_TREE) inode=258 size=2136457216 disk_isize=0 i
file extent range=[2126946304 2136457216] (num_bytes=9510912
ram_bytes=9510912 disk_bytenr=0 disk_num_bytes=0 extent_offset=0
type=0x1 compression=0

E.g type is 0x1 . With this patch applie the output is:

<ommitted for brevity>  disk_bytenr=141348864 disk_num_bytes=4096 extent_offset=0 type=REG compression=0

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 include/trace/events/btrfs.h | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 2615c7181e8c..937519399f30 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -67,11 +67,24 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 	      (obj >= BTRFS_ROOT_TREE_OBJECTID &&			\
 	       obj <= BTRFS_QUOTA_TREE_OBJECTID)) ? __show_root_type(obj) : "-"
 
-#define show_fi_type(type)						\
-	__print_symbolic(type,						\
-		 { BTRFS_FILE_EXTENT_INLINE,	"INLINE" },		\
-		 { BTRFS_FILE_EXTENT_REG,	"REG"	 },		\
-		 { BTRFS_FILE_EXTENT_PREALLOC,	"PREALLOC"})
+#define FI_TYPES							\
+	EM (BTRFS_FILE_EXTENT_INLINE,		"INLINE")	\
+	EM (BTRFS_FILE_EXTENT_REG,		"REG")		\
+	EMe (BTRFS_FILE_EXTENT_PREALLOC,	"PREALLOC")
+
+#undef EM
+#undef EMe
+#define EM(a, b) TRACE_DEFINE_ENUM(a);
+#define EMe(a, b) TRACE_DEFINE_ENUM(a);
+
+FI_TYPES
+
+#undef EM
+#undef EMe
+
+#define EM(a, b)        {a, b},
+#define EMe(a, b)       {a, b}
+
 
 #define show_qgroup_rsv_type(type)					\
 	__print_symbolic(type,						\
@@ -380,7 +393,7 @@ DECLARE_EVENT_CLASS(btrfs__file_extent_item_regular,
 		__entry->disk_isize, __entry->extent_start,
 		__entry->extent_end, __entry->num_bytes, __entry->ram_bytes,
 		__entry->disk_bytenr, __entry->disk_num_bytes,
-		__entry->extent_offset, show_fi_type(__entry->extent_type),
+		__entry->extent_offset, __print_symbolic(__entry->extent_type, FI_TYPES),
 		__entry->compression)
 );
 
@@ -421,7 +434,7 @@ DECLARE_EVENT_CLASS(
 		"extent_type=%s compression=%u",
 		show_root_type(__entry->root_obj), __entry->ino, __entry->isize,
 		__entry->disk_isize, __entry->extent_start,
-		__entry->extent_end, show_fi_type(__entry->extent_type),
+		__entry->extent_end, __print_symbolic(__entry->extent_type, FI_TYPES),
 		__entry->compression)
 );
 
-- 
2.17.1

[PATCH 3/6] btrfs: tracepoints: Move FLUSH_ACTIONS define

[Nikolay Borisov]

Since all enums used in btrfs' tracepoints are going to be redefined
to allow proper parsing of their values by userspace tools let's
rearrange when they are defined. This will allow to use only a single
set of #define EM/#undef EM sequence. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 include/trace/events/btrfs.h | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 937519399f30..e13c25598057 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -67,6 +67,12 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 	      (obj >= BTRFS_ROOT_TREE_OBJECTID &&			\
 	       obj <= BTRFS_QUOTA_TREE_OBJECTID)) ? __show_root_type(obj) : "-"
 
+#define FLUSH_ACTIONS								\
+	EM (BTRFS_RESERVE_NO_FLUSH,		"BTRFS_RESERVE_NO_FLUSH")	\
+	EM (BTRFS_RESERVE_FLUSH_LIMIT,		"BTRFS_RESERVE_FLUSH_LIMIT")	\
+	EM (BTRFS_RESERVE_FLUSH_ALL,		"BTRFS_RESERVE_FLUSH_ALL")	\
+	EMe (BTRFS_RESERVE_FLUSH_ALL_STEAL,	"BTRFS_RESERVE_FLUSH_ALL_STEAL")
+
 #define FI_TYPES							\
 	EM (BTRFS_FILE_EXTENT_INLINE,		"INLINE")	\
 	EM (BTRFS_FILE_EXTENT_REG,		"REG")		\
@@ -77,6 +83,7 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 #define EM(a, b) TRACE_DEFINE_ENUM(a);
 #define EMe(a, b) TRACE_DEFINE_ENUM(a);
 
+FLUSH_ACTIONS
 FI_TYPES
 
 #undef EM
@@ -1055,25 +1062,6 @@ TRACE_EVENT(btrfs_space_reservation,
 			__entry->bytes)
 );
 
-#define FLUSH_ACTIONS								\
-	EM (BTRFS_RESERVE_NO_FLUSH,		"BTRFS_RESERVE_NO_FLUSH")	\
-	EM (BTRFS_RESERVE_FLUSH_LIMIT,		"BTRFS_RESERVE_FLUSH_LIMIT")	\
-	EM (BTRFS_RESERVE_FLUSH_ALL,		"BTRFS_RESERVE_FLUSH_ALL")	\
-	EMe (BTRFS_RESERVE_FLUSH_ALL_STEAL,	"BTRFS_RESERVE_FLUSH_ALL_STEAL")
-
-#undef EM
-#undef EMe
-#define EM(a, b) TRACE_DEFINE_ENUM(a);
-#define EMe(a, b) TRACE_DEFINE_ENUM(a);
-
-FLUSH_ACTIONS
-
-#undef EM
-#undef EMe
-
-#define EM(a, b)        {a, b},
-#define EMe(a, b)       {a, b}
-
 TRACE_EVENT(btrfs_trigger_flush,
 
 	TP_PROTO(const struct btrfs_fs_info *fs_info, u64 flags, u64 bytes,
-- 
2.17.1

[PATCH 4/6] btrfs: tracepoints: Fix qgroup reservation type printing

[Nikolay Borisov]

Since qgroup's reservation types are define in a macro they must be
exported to user space in order for user space tools to convert raw
binary data to symbolic names. Currently trace-cmd report produces
the following output:

kworker/u8:2-459   [003]  1208.543587: qgroup_update_reserve:
2b742cae-e0e5-4def-9ef7-28a9b34a951e: qgid=5 type=0x2 cur_reserved=54870016 diff=-32768

With this fix the output is:

kworker/u8:2-459   [003]  1208.543587: qgroup_update_reserve:
2b742cae-e0e5-4def-9ef7-28a9b34a951e: qgid=5 type=BTRFS_QGROUP_RSV_META_PREALLOC cur_reserved=54870016 diff=-32768

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 include/trace/events/btrfs.h | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index e13c25598057..e6881ad5550a 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -78,6 +78,11 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 	EM (BTRFS_FILE_EXTENT_REG,		"REG")		\
 	EMe (BTRFS_FILE_EXTENT_PREALLOC,	"PREALLOC")
 
+#define QGROUP_RSV_TYPES					\
+	EM( BTRFS_QGROUP_RSV_DATA,	  "DATA"	)	\
+	EM( BTRFS_QGROUP_RSV_META_PERTRANS, "META_PERTRANS")	\
+	EMe( BTRFS_QGROUP_RSV_META_PREALLOC, "META_PREALLOC")
+
 #undef EM
 #undef EMe
 #define EM(a, b) TRACE_DEFINE_ENUM(a);
@@ -85,6 +90,7 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 
 FLUSH_ACTIONS
 FI_TYPES
+QGROUP_RSV_TYPES
 
 #undef EM
 #undef EMe
@@ -92,13 +98,6 @@ FI_TYPES
 #define EM(a, b)        {a, b},
 #define EMe(a, b)       {a, b}
 
-
-#define show_qgroup_rsv_type(type)					\
-	__print_symbolic(type,						\
-		{ BTRFS_QGROUP_RSV_DATA,	  "DATA"	},	\
-		{ BTRFS_QGROUP_RSV_META_PERTRANS, "META_PERTRANS" },	\
-		{ BTRFS_QGROUP_RSV_META_PREALLOC, "META_PREALLOC" })
-
 #define show_extent_io_tree_owner(owner)				       \
 	__print_symbolic(owner,						       \
 		{ IO_TREE_FS_PINNED_EXTENTS, 	  "PINNED_EXTENTS" },	       \
@@ -1704,7 +1703,7 @@ TRACE_EVENT(qgroup_update_reserve,
 	),
 
 	TP_printk_btrfs("qgid=%llu type=%s cur_reserved=%llu diff=%lld",
-		__entry->qgid, show_qgroup_rsv_type(__entry->type),
+		__entry->qgid, __print_symbolic(__entry->type, QGROUP_RSV_TYPES),
 		__entry->cur_reserved, __entry->diff)
 );
 
@@ -1728,7 +1727,7 @@ TRACE_EVENT(qgroup_meta_reserve,
 
 	TP_printk_btrfs("refroot=%llu(%s) type=%s diff=%lld",
 		show_root_type(__entry->refroot),
-		show_qgroup_rsv_type(__entry->type), __entry->diff)
+		__print_symbolic(__entry->type, QGROUP_RSV_TYPES), __entry->diff)
 );
 
 TRACE_EVENT(qgroup_meta_convert,
@@ -1749,8 +1748,8 @@ TRACE_EVENT(qgroup_meta_convert,
 
 	TP_printk_btrfs("refroot=%llu(%s) type=%s->%s diff=%lld",
 		show_root_type(__entry->refroot),
-		show_qgroup_rsv_type(BTRFS_QGROUP_RSV_META_PREALLOC),
-		show_qgroup_rsv_type(BTRFS_QGROUP_RSV_META_PERTRANS),
+		__print_symbolic(BTRFS_QGROUP_RSV_META_PREALLOC, QGROUP_RSV_TYPES),
+		__print_symbolic(BTRFS_QGROUP_RSV_META_PERTRANS, QGROUP_RSV_TYPES),
 		__entry->diff)
 );
 
@@ -1776,7 +1775,7 @@ TRACE_EVENT(qgroup_meta_free_all_pertrans,
 
 	TP_printk_btrfs("refroot=%llu(%s) type=%s diff=%lld",
 		show_root_type(__entry->refroot),
-		show_qgroup_rsv_type(__entry->type), __entry->diff)
+		__print_symbolic(__entry->type, QGROUP_RSV_TYPES), __entry->diff)
 );
 
 DECLARE_EVENT_CLASS(btrfs__prelim_ref,
-- 
2.17.1

[PATCH 5/6] btrfs: tracepoints: Switch extent_io_tree_owner to using EM macro

[Nikolay Borisov]

This fixes correct pint out of the extent io tree owner in
btrfs_set_extent_bit/btrfs_clear_extent_bit/btrfs_convert_extent_bit
tracepoints.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 include/trace/events/btrfs.h | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index e6881ad5550a..8a758892bdbe 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -83,6 +83,18 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 	EM( BTRFS_QGROUP_RSV_META_PERTRANS, "META_PERTRANS")	\
 	EMe( BTRFS_QGROUP_RSV_META_PREALLOC, "META_PREALLOC")
 
+#define IO_TREE_OWNER						    \
+	EM( IO_TREE_FS_PINNED_EXTENTS, 	  "PINNED_EXTENTS")	    \
+	EM( IO_TREE_FS_EXCLUDED_EXTENTS,  "EXCLUDED_EXTENTS")	    \
+	EM( IO_TREE_INODE_IO,		  "INODE_IO")		    \
+	EM( IO_TREE_INODE_IO_FAILURE,	  "INODE_IO_FAILURE")	    \
+	EM( IO_TREE_RELOC_BLOCKS,	  "RELOC_BLOCKS")	    \
+	EM( IO_TREE_TRANS_DIRTY_PAGES,	  "TRANS_DIRTY_PAGES")      \
+	EM( IO_TREE_ROOT_DIRTY_LOG_PAGES, "ROOT_DIRTY_LOG_PAGES")   \
+	EM( IO_TREE_INODE_FILE_EXTENT,	  "INODE_FILE_EXTENT")      \
+	EM( IO_TREE_LOG_CSUM_RANGE,	  "LOG_CSUM_RANGE")         \
+	EMe( IO_TREE_SELFTEST,		  "SELFTEST")
+
 #undef EM
 #undef EMe
 #define EM(a, b) TRACE_DEFINE_ENUM(a);
@@ -91,6 +103,7 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 FLUSH_ACTIONS
 FI_TYPES
 QGROUP_RSV_TYPES
+IO_TREE_OWNER
 
 #undef EM
 #undef EMe
@@ -98,18 +111,6 @@ QGROUP_RSV_TYPES
 #define EM(a, b)        {a, b},
 #define EMe(a, b)       {a, b}
 
-#define show_extent_io_tree_owner(owner)				       \
-	__print_symbolic(owner,						       \
-		{ IO_TREE_FS_PINNED_EXTENTS, 	  "PINNED_EXTENTS" },	       \
-		{ IO_TREE_FS_EXCLUDED_EXTENTS,	  "EXCLUDED_EXTENTS" },	       \
-		{ IO_TREE_INODE_IO,		  "INODE_IO" },		       \
-		{ IO_TREE_INODE_IO_FAILURE,	  "INODE_IO_FAILURE" },	       \
-		{ IO_TREE_RELOC_BLOCKS,		  "RELOC_BLOCKS" },	       \
-		{ IO_TREE_TRANS_DIRTY_PAGES,	  "TRANS_DIRTY_PAGES" },       \
-		{ IO_TREE_ROOT_DIRTY_LOG_PAGES,	  "ROOT_DIRTY_LOG_PAGES" },    \
-		{ IO_TREE_INODE_FILE_EXTENT,	  "INODE_FILE_EXTENT" },       \
-		{ IO_TREE_LOG_CSUM_RANGE,	  "LOG_CSUM_RANGE" },          \
-		{ IO_TREE_SELFTEST,		  "SELFTEST" })
 
 #define BTRFS_GROUP_FLAGS	\
 	{ BTRFS_BLOCK_GROUP_DATA,	"DATA"},	\
@@ -1933,7 +1934,7 @@ TRACE_EVENT(btrfs_set_extent_bit,
 
 	TP_printk_btrfs(
 		"io_tree=%s ino=%llu root=%llu start=%llu len=%llu set_bits=%s",
-		show_extent_io_tree_owner(__entry->owner), __entry->ino,
+		__print_symbolic(__entry->owner, IO_TREE_OWNER), __entry->ino,
 		__entry->rootid, __entry->start, __entry->len,
 		__print_flags(__entry->set_bits, "|", EXTENT_FLAGS))
 );
@@ -1972,7 +1973,7 @@ TRACE_EVENT(btrfs_clear_extent_bit,
 
 	TP_printk_btrfs(
 		"io_tree=%s ino=%llu root=%llu start=%llu len=%llu clear_bits=%s",
-		show_extent_io_tree_owner(__entry->owner), __entry->ino,
+		__print_symbolic(__entry->owner, IO_TREE_OWNER), __entry->ino,
 		__entry->rootid, __entry->start, __entry->len,
 		__print_flags(__entry->clear_bits, "|", EXTENT_FLAGS))
 );
@@ -2013,7 +2014,7 @@ TRACE_EVENT(btrfs_convert_extent_bit,
 
 	TP_printk_btrfs(
 "io_tree=%s ino=%llu root=%llu start=%llu len=%llu set_bits=%s clear_bits=%s",
-		  show_extent_io_tree_owner(__entry->owner), __entry->ino,
+		  __print_symbolic(__entry->owner, IO_TREE_OWNER), __entry->ino,
 		  __entry->rootid, __entry->start, __entry->len,
 		  __print_flags(__entry->set_bits , "|", EXTENT_FLAGS),
 		  __print_flags(__entry->clear_bits, "|", EXTENT_FLAGS))
-- 
2.17.1

[PATCH 6/6] btrfs: tracepoints: Convert flush states to using EM macros

[Nikolay Borisov]

Only 6 out of all flush states were being printed correctly since
only they were exported via the TRACE_DEFINE_ENUM macro. This patch
converts all flush states to use the newly introduced EM macro so that
they can all be printed correctly.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 include/trace/events/btrfs.h | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 8a758892bdbe..b0e98823c0a3 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -31,13 +31,6 @@ struct extent_io_tree;
 struct prelim_ref;
 struct btrfs_space_info;
 
-TRACE_DEFINE_ENUM(FLUSH_DELAYED_ITEMS_NR);
-TRACE_DEFINE_ENUM(FLUSH_DELAYED_ITEMS);
-TRACE_DEFINE_ENUM(FLUSH_DELALLOC);
-TRACE_DEFINE_ENUM(FLUSH_DELALLOC_WAIT);
-TRACE_DEFINE_ENUM(ALLOC_CHUNK);
-TRACE_DEFINE_ENUM(COMMIT_TRANS);
-
 #define show_ref_type(type)						\
 	__print_symbolic(type,						\
 		{ BTRFS_TREE_BLOCK_REF_KEY, 	"TREE_BLOCK_REF" },	\
@@ -95,6 +88,18 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 	EM( IO_TREE_LOG_CSUM_RANGE,	  "LOG_CSUM_RANGE")         \
 	EMe( IO_TREE_SELFTEST,		  "SELFTEST")
 
+#define FLUSH_STATES							\
+	EM( FLUSH_DELAYED_ITEMS_NR,	"FLUSH_DELAYED_ITEMS_NR")	\
+	EM( FLUSH_DELAYED_ITEMS,	"FLUSH_DELAYED_ITEMS")		\
+	EM( FLUSH_DELALLOC,		"FLUSH_DELALLOC")		\
+	EM( FLUSH_DELALLOC_WAIT,	"FLUSH_DELALLOC_WAIT")		\
+	EM( FLUSH_DELAYED_REFS_NR,	"FLUSH_DELAYED_REFS_NR")	\
+	EM( FLUSH_DELAYED_REFS,		"FLUSH_ELAYED_REFS")		\
+	EM( ALLOC_CHUNK,		"ALLOC_CHUNK")			\
+	EM( ALLOC_CHUNK_FORCE,		"ALLOC_CHUNK_FORCE")		\
+	EM( RUN_DELAYED_IPUTS,		"RUN_DELAYED_IPUTS")		\
+	EMe( COMMIT_TRANS,		"COMMIT_TRANS")
+
 #undef EM
 #undef EMe
 #define EM(a, b) TRACE_DEFINE_ENUM(a);
@@ -104,6 +109,7 @@ FLUSH_ACTIONS
 FI_TYPES
 QGROUP_RSV_TYPES
 IO_TREE_OWNER
+FLUSH_STATES
 
 #undef EM
 #undef EMe
@@ -1092,18 +1098,6 @@ TRACE_EVENT(btrfs_trigger_flush,
 		  __entry->bytes)
 );
 
-#define show_flush_state(state)							\
-	__print_symbolic(state,							\
-		{ FLUSH_DELAYED_ITEMS_NR,	"FLUSH_DELAYED_ITEMS_NR"},	\
-		{ FLUSH_DELAYED_ITEMS,		"FLUSH_DELAYED_ITEMS"},		\
-		{ FLUSH_DELALLOC,		"FLUSH_DELALLOC"},		\
-		{ FLUSH_DELALLOC_WAIT,		"FLUSH_DELALLOC_WAIT"},		\
-		{ FLUSH_DELAYED_REFS_NR,	"FLUSH_DELAYED_REFS_NR"},	\
-		{ FLUSH_DELAYED_REFS,		"FLUSH_ELAYED_REFS"},		\
-		{ ALLOC_CHUNK,			"ALLOC_CHUNK"},			\
-		{ ALLOC_CHUNK_FORCE,		"ALLOC_CHUNK_FORCE"},		\
-		{ RUN_DELAYED_IPUTS,		"RUN_DELAYED_IPUTS"},		\
-		{ COMMIT_TRANS,			"COMMIT_TRANS"})
 
 TRACE_EVENT(btrfs_flush_space,
 
@@ -1128,7 +1122,7 @@ TRACE_EVENT(btrfs_flush_space,
 
 	TP_printk_btrfs("state=%d(%s) flags=%llu(%s) num_bytes=%llu ret=%d",
 		  __entry->state,
-		  show_flush_state(__entry->state),
+		  __print_symbolic(__entry->state, FLUSH_STATES),
 		  __entry->flags,
 		  __print_flags((unsigned long)__entry->flags, "|",
 				BTRFS_GROUP_FLAGS),
-- 
2.17.1

Re: [PATCH 0/6] Fix enum values print in tracepoints

[David Sterba]

On Fri, Jun 19, 2020 at 03:24:45PM +0300, Nikolay Borisov wrote:
> While looking at tracepoint dumps with trace-cmd report I observed that
> tracepoints that should have printed text instead of raw values weren't
> doing so:
> 
> 13 kworker/u8:1-61    [000]    66.299527: btrfs_flush_space:    5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0
> 
> In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e
> the correct output should be:
> 
> 6 fio-370   [002]    56.762402: btrfs_trigger_flush:  d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360
> 
> Investigating this turned out to be caused  because enum values weren't exported
> to user space via TRACE_DEFINE_ENUM. This is required in order for user space
> tools to correctly map the raw binary values to their textual representation.
> More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space")
> 
> This series follows the approach taken by 190f0b76ca49 in defining the various
> enum mapping structures.

So I see where the unintuitive naming EM and EMe comes from, but ok
let's stick to that.

I've looked at the result and it could really use the comments from
190f0b76ca49 where the definitions switch the output and some whitespace
fixups in the new definitions.

Not all enums are converted, just search for use of __print_symbolic
inside the show_TYPE helpers (eg. show_ref_action, show_ref_type),
please add them too. Thanks.

Re: [PATCH 0/6] Fix enum values print in tracepoints

[Nikolay Borisov]

On 22.06.20 г. 17:21 ч., David Sterba wrote:
> On Fri, Jun 19, 2020 at 03:24:45PM +0300, Nikolay Borisov wrote:
>> While looking at tracepoint dumps with trace-cmd report I observed that
>> tracepoints that should have printed text instead of raw values weren't
>> doing so:
>>
>> 13 kworker/u8:1-61    [000]    66.299527: btrfs_flush_space:    5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0
>>
>> In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e
>> the correct output should be:
>>
>> 6 fio-370   [002]    56.762402: btrfs_trigger_flush:  d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360
>>
>> Investigating this turned out to be caused  because enum values weren't exported
>> to user space via TRACE_DEFINE_ENUM. This is required in order for user space
>> tools to correctly map the raw binary values to their textual representation.
>> More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space")
>>
>> This series follows the approach taken by 190f0b76ca49 in defining the various
>> enum mapping structures.
> 
> So I see where the unintuitive naming EM and EMe comes from, but ok
> let's stick to that.

Yeah, the difference between EM and EMe is the ending comma that's
needed when actually defining the array passed to __print_symbols.

> 
> I've looked at the result and it could really use the comments from
> 190f0b76ca49 where the definitions switch the output and some whitespace
> fixups in the new definitions.
> 
> Not all enums are converted, just search for use of __print_symbolic
> inside the show_TYPE helpers (eg. show_ref_action, show_ref_type),
> please add them too. Thanks.

I beg you to differ:

show_ref_action/show_ref_type/__show_root_type/__show_map_type/ - those
are defined and they are OK as is, because defines don't emit separate
symbols. However, when/if in the future those defines are switched to
enums then the respective tracepoint code should be converted to using
the EM macros as well.


> 

Re: [PATCH 0/6] Fix enum values print in tracepoints

[David Sterba]

On Mon, Jun 22, 2020 at 06:19:39PM +0300, Nikolay Borisov wrote:
> > I've looked at the result and it could really use the comments from
> > 190f0b76ca49 where the definitions switch the output and some whitespace
> > fixups in the new definitions.
> > 
> > Not all enums are converted, just search for use of __print_symbolic
> > inside the show_TYPE helpers (eg. show_ref_action, show_ref_type),
> > please add them too. Thanks.
> 
> I beg you to differ:
> 
> show_ref_action/show_ref_type/__show_root_type/__show_map_type/ - those
> are defined and they are OK as is, because defines don't emit separate
> symbols. However, when/if in the future those defines are switched to
> enums then the respective tracepoint code should be converted to using
> the EM macros as well.

I see, it's define vs enum.

Re: [PATCH 0/6] Fix enum values print in tracepoints

[David Sterba]

On Fri, Jun 19, 2020 at 03:24:45PM +0300, Nikolay Borisov wrote:
> While looking at tracepoint dumps with trace-cmd report I observed that
> tracepoints that should have printed text instead of raw values weren't
> doing so:
> 
> 13 kworker/u8:1-61    [000]    66.299527: btrfs_flush_space:    5302ee13-c65e-45bb-98ef-8fe3835bd943: state=3(0x3) flags=4(METADATA) num_bytes=2621440 ret=0
> 
> In the above line instead of (0x3) BTRFS_RESERVE_FLUSH_ALL should be printed. I.e
> the correct output should be:
> 
> 6 fio-370   [002]    56.762402: btrfs_trigger_flush:  d04cd7ac-38e2-452f-a7f5-8157529fd5f0: preempt: flush=3(BTRFS_RESERVE_FLUSH_ALL) flags=4(METADATA) bytes=655360
> 
> Investigating this turned out to be caused  because enum values weren't exported
> to user space via TRACE_DEFINE_ENUM. This is required in order for user space
> tools to correctly map the raw binary values to their textual representation.
> More information can be found in commit 190f0b76ca49 ("mm: tracing: Export enums in tracepoints to user space")
> 
> This series follows the approach taken by 190f0b76ca49 in defining the various
> enum mapping structures.
> 
> Nikolay Borisov (6):
>   btrfs: tracepoints: Fix btrfs_trigger_flush printout
>   btrfs: tracepoints: Fix extent type symbolic name print
>   btrfs: tracepoints: Move FLUSH_ACTIONS define
>   btrfs: tracepoints: Fix qgroup reservation type printing
>   btrfs: tracepoints: Switch extent_io_tree_owner to using EM macro
>   btrfs: tracepoints: Convert flush states to using EM macros

Whitespace fixed and series added to misc-next, thanks.