[PATCH 0/6] xfs-5.0: ftrace cleanups

[PATCH 0/6] xfs-5.0: ftrace cleanups

[Darrick J. Wong]

Hi all,

This series fixes symbolic decoding in ftrace tracepoints in xfs, and
introduces decoding of xfs_btnum_t and scrub types.

The first four patches fix some problems with the current ftrace usage
so that symbolic decoding of enum -> string mappings works again, and
fixes function pointer printing.

The two patches after that add symbolic decoding to btree cursor types
and scrub types.

If you're going to start using this mess, you probably ought to just
pull from my git trees.  The kernel patches[1] should apply against
4.20-rc4.

Comments and questions are, as always, welcome.

--D

[1] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=djwong-devel

[PATCH 1/6] xfs: fix symbolic enum printing in ftrace output

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

ftrace's __print_symbolic() has a (very poorly documented) requirement
that any enum values used in the symbol to string translation table be
wrapped in a TRACE_DEFINE_ENUM so that the enum value can be encoded in
the ftrace ring buffer.  Fix this unsatisfied requirement.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_trace.h |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)


diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index cabe5c8010b0..dcde4e9907f7 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -640,6 +640,16 @@ DEFINE_INODE_EVENT(xfs_inode_set_cowblocks_tag);
 DEFINE_INODE_EVENT(xfs_inode_clear_cowblocks_tag);
 DEFINE_INODE_EVENT(xfs_inode_free_cowblocks_invalid);
 
+/*
+ * ftrace's __print_symbolic requires that all enum values be wrapped in the
+ * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
+ * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
+ * code.
+ */
+TRACE_DEFINE_ENUM(PE_SIZE_PTE);
+TRACE_DEFINE_ENUM(PE_SIZE_PMD);
+TRACE_DEFINE_ENUM(PE_SIZE_PUD);
+
 TRACE_EVENT(xfs_filemap_fault,
 	TP_PROTO(struct xfs_inode *ip, enum page_entry_size pe_size,
 		 bool write_fault),
@@ -1208,6 +1218,12 @@ DEFINE_EVENT(xfs_readpage_class, name,	\
 DEFINE_READPAGE_EVENT(xfs_vm_readpage);
 DEFINE_READPAGE_EVENT(xfs_vm_readpages);
 
+TRACE_DEFINE_ENUM(XFS_IO_HOLE);
+TRACE_DEFINE_ENUM(XFS_IO_DELALLOC);
+TRACE_DEFINE_ENUM(XFS_IO_UNWRITTEN);
+TRACE_DEFINE_ENUM(XFS_IO_OVERWRITE);
+TRACE_DEFINE_ENUM(XFS_IO_COW);
+
 DECLARE_EVENT_CLASS(xfs_imap_class,
 	TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count,
 		 int type, struct xfs_bmbt_irec *irec),

[PATCH 2/6] xfs: fix function pointer type in ftrace format

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use %pS instead of %pF in ftrace strings so that we record the actual
function address instead of the function descriptor.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/scrub/trace.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index f1c144c6dcc5..d67e915cf91a 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -473,7 +473,7 @@ TRACE_EVENT(xchk_xref_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u xref error %d ret_ip %pF",
+	TP_printk("dev %d:%d type %u xref error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->type,
 		  __entry->error,

[PATCH 3/6] xfs: move XFS_AG_BTREE_CMP_FORMAT_STR mappings to libxfs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Move XFS_AG_BTREE_CMP_FORMAT_STR to libxfs so that we don't forget to
keep it updated, and TRACE_DEFINE_ENUM the values while we're at it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_types.h |    5 +++++
 fs/xfs/xfs_trace.h        |    7 +++----
 2 files changed, 8 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index 05e8fa558f3e..297cf2318f83 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -104,6 +104,11 @@ typedef enum {
 	XFS_LOOKUP_EQi, XFS_LOOKUP_LEi, XFS_LOOKUP_GEi
 } xfs_lookup_t;
 
+#define XFS_AG_BTREE_CMP_FORMAT_STR \
+	{ XFS_LOOKUP_EQi,	"eq" }, \
+	{ XFS_LOOKUP_LEi,	"le" }, \
+	{ XFS_LOOKUP_GEi,	"ge" }
+
 typedef enum {
 	XFS_BTNUM_BNOi, XFS_BTNUM_CNTi, XFS_BTNUM_RMAPi, XFS_BTNUM_BMAPi,
 	XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index dcde4e9907f7..3f57002ca660 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -2626,10 +2626,9 @@ DEFINE_AG_ERROR_EVENT(xfs_ag_resv_init_error);
 #define DEFINE_AG_EXTENT_EVENT(name) DEFINE_DISCARD_EVENT(name)
 
 /* ag btree lookup tracepoint class */
-#define XFS_AG_BTREE_CMP_FORMAT_STR \
-	{ XFS_LOOKUP_EQ,	"eq" }, \
-	{ XFS_LOOKUP_LE,	"le" }, \
-	{ XFS_LOOKUP_GE,	"ge" }
+TRACE_DEFINE_ENUM(XFS_LOOKUP_EQi);
+TRACE_DEFINE_ENUM(XFS_LOOKUP_LEi);
+TRACE_DEFINE_ENUM(XFS_LOOKUP_GEi);
 DECLARE_EVENT_CLASS(xfs_ag_btree_lookup_class,
 	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno,
 		 xfs_agblock_t agbno, xfs_lookup_t dir),

[PATCH 4/6] xfs: move XFS_INODE_FORMAT_STR mappings to libxfs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Move XFS_INODE_FORMAT_STR to libxfs so that we don't forget to keep it
updated, and add necessary TRACE_DEFINE_ENUM.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |    7 +++++++
 fs/xfs/xfs_trace.h         |   10 +++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index b15412e4c535..d8902114786d 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -925,6 +925,13 @@ typedef enum xfs_dinode_fmt {
 	XFS_DINODE_FMT_UUID		/* added long ago, but never used */
 } xfs_dinode_fmt_t;
 
+#define XFS_INODE_FORMAT_STR \
+	{ XFS_DINODE_FMT_DEV,		"dev" }, \
+	{ XFS_DINODE_FMT_LOCAL,		"local" }, \
+	{ XFS_DINODE_FMT_EXTENTS,	"extent" }, \
+	{ XFS_DINODE_FMT_BTREE,		"btree" }, \
+	{ XFS_DINODE_FMT_UUID,		"uuid" }
+
 /*
  * Inode minimum and maximum sizes.
  */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 3f57002ca660..94e289aca220 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1901,11 +1901,11 @@ TRACE_EVENT(xfs_dir2_leafn_moveents,
 	{ 0,	"target" }, \
 	{ 1,	"temp" }
 
-#define XFS_INODE_FORMAT_STR \
-	{ 0,	"invalid" }, \
-	{ 1,	"local" }, \
-	{ 2,	"extent" }, \
-	{ 3,	"btree" }
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_DEV);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_LOCAL);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_EXTENTS);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_BTREE);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_UUID);
 
 DECLARE_EVENT_CLASS(xfs_swap_extent_class,
 	TP_PROTO(struct xfs_inode *ip, int which),

[PATCH 5/6] xfs: stringify btree cursor types in ftrace output

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use __print_symbolic to print the btree type in ftrace output.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_types.h |    9 +++++++++
 fs/xfs/scrub/trace.h      |   38 ++++++++++++++++++++++++++------------
 fs/xfs/xfs_trace.h        |   12 ++++++++++--
 3 files changed, 45 insertions(+), 14 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index 297cf2318f83..eb04afe9cce0 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -114,6 +114,15 @@ typedef enum {
 	XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX
 } xfs_btnum_t;
 
+#define XFS_BTNUM_STRINGS \
+	{ XFS_BTNUM_BNOi,	"bnobt" }, \
+	{ XFS_BTNUM_CNTi,	"cntbt" }, \
+	{ XFS_BTNUM_RMAPi,	"rmapbt" }, \
+	{ XFS_BTNUM_BMAPi,	"bmbt" }, \
+	{ XFS_BTNUM_INOi,	"inobt" }, \
+	{ XFS_BTNUM_FINOi,	"finobt" }, \
+	{ XFS_BTNUM_REFCi,	"refcbt" }
+
 struct xfs_name {
 	const unsigned char	*name;
 	int			len;
diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index d67e915cf91a..0141eb1ac091 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -12,6 +12,20 @@
 #include <linux/tracepoint.h>
 #include "xfs_bit.h"
 
+/*
+ * ftrace's __print_symbolic requires that all enum values be wrapped in the
+ * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
+ * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
+ * code.
+ */
+TRACE_DEFINE_ENUM(XFS_BTNUM_BNOi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_CNTi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_BMAPi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_INOi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
+
 DECLARE_EVENT_CLASS(xchk_class,
 	TP_PROTO(struct xfs_inode *ip, struct xfs_scrub_metadata *sm,
 		 int error),
@@ -278,10 +292,10 @@ TRACE_EVENT(xchk_btree_op_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u btnum %d level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
+	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->type,
-		  __entry->btnum,
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
 		  __entry->agno,
@@ -321,12 +335,12 @@ TRACE_EVENT(xchk_ifork_btree_op_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btnum %d level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->whichfork,
 		  __entry->type,
-		  __entry->btnum,
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
 		  __entry->agno,
@@ -360,10 +374,10 @@ TRACE_EVENT(xchk_btree_error,
 		__entry->ptr = cur->bc_ptrs[level];
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u btnum %d level %d ptr %d agno %u agbno %u ret_ip %pS",
+	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->type,
-		  __entry->btnum,
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
 		  __entry->agno,
@@ -400,12 +414,12 @@ TRACE_EVENT(xchk_ifork_btree_error,
 		__entry->ptr = cur->bc_ptrs[level];
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btnum %d level %d ptr %d agno %u agbno %u ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->whichfork,
 		  __entry->type,
-		  __entry->btnum,
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
 		  __entry->agno,
@@ -439,10 +453,10 @@ DECLARE_EVENT_CLASS(xchk_sbtree_class,
 		__entry->nlevels = cur->bc_nlevels;
 		__entry->ptr = cur->bc_ptrs[level];
 	),
-	TP_printk("dev %d:%d type %u btnum %d agno %u agbno %u level %d nlevels %d ptr %d",
+	TP_printk("dev %d:%d type %u btree %s agno %u agbno %u level %d nlevels %d ptr %d",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->type,
-		  __entry->btnum,
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->agno,
 		  __entry->bno,
 		  __entry->level,
@@ -643,11 +657,11 @@ TRACE_EVENT(xrep_init_btblock,
 		__entry->agbno = agbno;
 		__entry->btnum = btnum;
 	),
-	TP_printk("dev %d:%d agno %u agbno %u btnum %d",
+	TP_printk("dev %d:%d agno %u agbno %u btree %s",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->agno,
 		  __entry->agbno,
-		  __entry->btnum)
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS))
 )
 TRACE_EVENT(xrep_findroot_block,
 	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, xfs_agblock_t agbno,
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 94e289aca220..6fcc893dfc91 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -2194,6 +2194,14 @@ DEFINE_DISCARD_EVENT(xfs_discard_exclude);
 DEFINE_DISCARD_EVENT(xfs_discard_busy);
 
 /* btree cursor events */
+TRACE_DEFINE_ENUM(XFS_BTNUM_BNOi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_CNTi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_BMAPi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_INOi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
+TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
+
 DECLARE_EVENT_CLASS(xfs_btree_cur_class,
 	TP_PROTO(struct xfs_btree_cur *cur, int level, struct xfs_buf *bp),
 	TP_ARGS(cur, level, bp),
@@ -2213,9 +2221,9 @@ DECLARE_EVENT_CLASS(xfs_btree_cur_class,
 		__entry->ptr = cur->bc_ptrs[level];
 		__entry->daddr = bp ? bp->b_bn : -1;
 	),
-	TP_printk("dev %d:%d btnum %d level %d/%d ptr %d daddr 0x%llx",
+	TP_printk("dev %d:%d btree %s level %d/%d ptr %d daddr 0x%llx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->btnum,
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->nlevels,
 		  __entry->ptr,

[PATCH 6/6] xfs: stringify scrub types in ftrace output

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use __print_symbolic to print the scrub type in ftrace output.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/scrub/trace.h |  103 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 77 insertions(+), 26 deletions(-)


diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index 0141eb1ac091..3c83e8b3b39c 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -26,6 +26,57 @@ TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
 TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
 TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
 
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PROBE);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SB);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGF);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGFL);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGI);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BNOBT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_CNTBT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INOBT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_FINOBT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RMAPBT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_REFCNTBT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INODE);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTD);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTA);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTC);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_DIR);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_XATTR);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SYMLINK);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PARENT);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTBITMAP);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTSUM);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_UQUOTA);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_GQUOTA);
+TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PQUOTA);
+
+#define XFS_SCRUB_TYPE_STRINGS \
+	{ XFS_SCRUB_TYPE_PROBE,		"probe" }, \
+	{ XFS_SCRUB_TYPE_SB,		"sb" }, \
+	{ XFS_SCRUB_TYPE_AGF,		"agf" }, \
+	{ XFS_SCRUB_TYPE_AGFL,		"agfl" }, \
+	{ XFS_SCRUB_TYPE_AGI,		"agi" }, \
+	{ XFS_SCRUB_TYPE_BNOBT,		"bnobt" }, \
+	{ XFS_SCRUB_TYPE_CNTBT,		"cntbt" }, \
+	{ XFS_SCRUB_TYPE_INOBT,		"inobt" }, \
+	{ XFS_SCRUB_TYPE_FINOBT,	"finobt" }, \
+	{ XFS_SCRUB_TYPE_RMAPBT,	"rmapbt" }, \
+	{ XFS_SCRUB_TYPE_REFCNTBT,	"refcountbt" }, \
+	{ XFS_SCRUB_TYPE_INODE,		"inode" }, \
+	{ XFS_SCRUB_TYPE_BMBTD,		"bmapbtd" }, \
+	{ XFS_SCRUB_TYPE_BMBTA,		"bmapbta" }, \
+	{ XFS_SCRUB_TYPE_BMBTC,		"bmapbtc" }, \
+	{ XFS_SCRUB_TYPE_DIR,		"directory" }, \
+	{ XFS_SCRUB_TYPE_XATTR,		"xattr" }, \
+	{ XFS_SCRUB_TYPE_SYMLINK,	"symlink" }, \
+	{ XFS_SCRUB_TYPE_PARENT,	"parent" }, \
+	{ XFS_SCRUB_TYPE_RTBITMAP,	"rtbitmap" }, \
+	{ XFS_SCRUB_TYPE_RTSUM,		"rtsummary" }, \
+	{ XFS_SCRUB_TYPE_UQUOTA,	"usrquota" }, \
+	{ XFS_SCRUB_TYPE_GQUOTA,	"grpquota" }, \
+	{ XFS_SCRUB_TYPE_PQUOTA,	"prjquota" }
+
 DECLARE_EVENT_CLASS(xchk_class,
 	TP_PROTO(struct xfs_inode *ip, struct xfs_scrub_metadata *sm,
 		 int error),
@@ -50,10 +101,10 @@ DECLARE_EVENT_CLASS(xchk_class,
 		__entry->flags = sm->sm_flags;
 		__entry->error = error;
 	),
-	TP_printk("dev %d:%d ino 0x%llx type %u agno %u inum %llu gen %u flags 0x%x error %d",
+	TP_printk("dev %d:%d ino 0x%llx type %s agno %u inum %llu gen %u flags 0x%x error %d",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->agno,
 		  __entry->inum,
 		  __entry->gen,
@@ -92,9 +143,9 @@ TRACE_EVENT(xchk_op_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u agno %u agbno %u error %d ret_ip %pS",
+	TP_printk("dev %d:%d type %s agno %u agbno %u error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->agno,
 		  __entry->bno,
 		  __entry->error,
@@ -123,11 +174,11 @@ TRACE_EVENT(xchk_file_op_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx fork %d type %u offset %llu error %d ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx fork %d type %s offset %llu error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->whichfork,
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->offset,
 		  __entry->error,
 		  __entry->ret_ip)
@@ -158,9 +209,9 @@ DECLARE_EVENT_CLASS(xchk_block_error_class,
 		__entry->bno = bno;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u agno %u agbno %u ret_ip %pS",
+	TP_printk("dev %d:%d type %s agno %u agbno %u ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->agno,
 		  __entry->bno,
 		  __entry->ret_ip)
@@ -190,10 +241,10 @@ DECLARE_EVENT_CLASS(xchk_ino_error_class,
 		__entry->type = sc->sm->sm_type;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx type %u ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx type %s ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->ret_ip)
 )
 
@@ -227,11 +278,11 @@ DECLARE_EVENT_CLASS(xchk_fblock_error_class,
 		__entry->offset = offset;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx fork %d type %u offset %llu ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx fork %d type %s offset %llu ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->whichfork,
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->offset,
 		  __entry->ret_ip)
 );
@@ -258,9 +309,9 @@ TRACE_EVENT(xchk_incomplete,
 		__entry->type = sc->sm->sm_type;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u ret_ip %pS",
+	TP_printk("dev %d:%d type %s ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->ret_ip)
 );
 
@@ -292,9 +343,9 @@ TRACE_EVENT(xchk_btree_op_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
+	TP_printk("dev %d:%d type %s btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
@@ -335,11 +386,11 @@ TRACE_EVENT(xchk_ifork_btree_op_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx fork %d type %s btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->whichfork,
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
@@ -374,9 +425,9 @@ TRACE_EVENT(xchk_btree_error,
 		__entry->ptr = cur->bc_ptrs[level];
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
+	TP_printk("dev %d:%d type %s btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
@@ -414,11 +465,11 @@ TRACE_EVENT(xchk_ifork_btree_error,
 		__entry->ptr = cur->bc_ptrs[level];
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
+	TP_printk("dev %d:%d ino 0x%llx fork %d type %s btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->whichfork,
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->level,
 		  __entry->ptr,
@@ -453,9 +504,9 @@ DECLARE_EVENT_CLASS(xchk_sbtree_class,
 		__entry->nlevels = cur->bc_nlevels;
 		__entry->ptr = cur->bc_ptrs[level];
 	),
-	TP_printk("dev %d:%d type %u btree %s agno %u agbno %u level %d nlevels %d ptr %d",
+	TP_printk("dev %d:%d type %s btree %s agno %u agbno %u level %d nlevels %d ptr %d",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
 		  __entry->agno,
 		  __entry->bno,
@@ -487,9 +538,9 @@ TRACE_EVENT(xchk_xref_error,
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
-	TP_printk("dev %d:%d type %u xref error %d ret_ip %pS",
+	TP_printk("dev %d:%d type %s xref error %d ret_ip %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->type,
+		  __print_symbolic(__entry->type, XFS_SCRUB_TYPE_STRINGS),
 		  __entry->error,
 		  __entry->ret_ip)
 );

Re: [PATCH 1/6] xfs: fix symbolic enum printing in ftrace output

[Eric Sandeen]

On 11/28/18 5:28 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> ftrace's __print_symbolic() has a (very poorly documented) requirement
> that any enum values used in the symbol to string translation table be
> wrapped in a TRACE_DEFINE_ENUM so that the enum value can be encoded in
> the ftrace ring buffer.  Fix this unsatisfied requirement.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/xfs_trace.h |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index cabe5c8010b0..dcde4e9907f7 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -640,6 +640,16 @@ DEFINE_INODE_EVENT(xfs_inode_set_cowblocks_tag);
>  DEFINE_INODE_EVENT(xfs_inode_clear_cowblocks_tag);
>  DEFINE_INODE_EVENT(xfs_inode_free_cowblocks_invalid);
>  
> +/*
> + * ftrace's __print_symbolic requires that all enum values be wrapped in the
> + * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
> + * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
> + * code.
> + */
> +TRACE_DEFINE_ENUM(PE_SIZE_PTE);
> +TRACE_DEFINE_ENUM(PE_SIZE_PMD);
> +TRACE_DEFINE_ENUM(PE_SIZE_PUD);
> +
>  TRACE_EVENT(xfs_filemap_fault,
>  	TP_PROTO(struct xfs_inode *ip, enum page_entry_size pe_size,
>  		 bool write_fault),
> @@ -1208,6 +1218,12 @@ DEFINE_EVENT(xfs_readpage_class, name,	\
>  DEFINE_READPAGE_EVENT(xfs_vm_readpage);
>  DEFINE_READPAGE_EVENT(xfs_vm_readpages);
>  
> +TRACE_DEFINE_ENUM(XFS_IO_HOLE);
> +TRACE_DEFINE_ENUM(XFS_IO_DELALLOC);
> +TRACE_DEFINE_ENUM(XFS_IO_UNWRITTEN);
> +TRACE_DEFINE_ENUM(XFS_IO_OVERWRITE);
> +TRACE_DEFINE_ENUM(XFS_IO_COW);
> +
>  DECLARE_EVENT_CLASS(xfs_imap_class,
>  	TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count,
>  		 int type, struct xfs_bmbt_irec *irec),
> 

Re: [PATCH 2/6] xfs: fix function pointer type in ftrace format

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use %pS instead of %pF in ftrace strings so that we record the actual
> function address instead of the function descriptor.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/scrub/trace.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index f1c144c6dcc5..d67e915cf91a 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -473,7 +473,7 @@ TRACE_EVENT(xchk_xref_error,
>  		__entry->error = error;
>  		__entry->ret_ip = ret_ip;
>  	),
> -	TP_printk("dev %d:%d type %u xref error %d ret_ip %pF",
> +	TP_printk("dev %d:%d type %u xref error %d ret_ip %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->type,
>  		  __entry->error,
> 

Re: [PATCH 5/6] xfs: stringify btree cursor types in ftrace output

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use __print_symbolic to print the btree type in ftrace output.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Ok, and because XFS_BTNUM_BNOi *shudder* et al are enums, you also
need the fancy wrappers.

> ---
>  fs/xfs/libxfs/xfs_types.h |    9 +++++++++
>  fs/xfs/scrub/trace.h      |   38 ++++++++++++++++++++++++++------------
>  fs/xfs/xfs_trace.h        |   12 ++++++++++--
>  3 files changed, 45 insertions(+), 14 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 297cf2318f83..eb04afe9cce0 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -114,6 +114,15 @@ typedef enum {
>  	XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX
>  } xfs_btnum_t;
>  
> +#define XFS_BTNUM_STRINGS \
> +	{ XFS_BTNUM_BNOi,	"bnobt" }, \
> +	{ XFS_BTNUM_CNTi,	"cntbt" }, \
> +	{ XFS_BTNUM_RMAPi,	"rmapbt" }, \
> +	{ XFS_BTNUM_BMAPi,	"bmbt" }, \
> +	{ XFS_BTNUM_INOi,	"inobt" }, \
> +	{ XFS_BTNUM_FINOi,	"finobt" }, \
> +	{ XFS_BTNUM_REFCi,	"refcbt" }
> +

how do we decide whether to put these string translations for trace in common
headers (as here) vs. in the the trace file itself (as with, say,
XFS_AG_BTREE_CMP_FORMAT_STR) ?

-Eric

Re: [PATCH 3/6] xfs: move XFS_AG_BTREE_CMP_FORMAT_STR mappings to libxfs

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move XFS_AG_BTREE_CMP_FORMAT_STR to libxfs so that we don't forget to
> keep it updated, and TRACE_DEFINE_ENUM the values while we're at it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

whoops I skipped right over this when I asked the question this answers
in response to patch 5/6 sorry.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_types.h |    5 +++++
>  fs/xfs/xfs_trace.h        |    7 +++----
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 05e8fa558f3e..297cf2318f83 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -104,6 +104,11 @@ typedef enum {
>  	XFS_LOOKUP_EQi, XFS_LOOKUP_LEi, XFS_LOOKUP_GEi
>  } xfs_lookup_t;
>  
> +#define XFS_AG_BTREE_CMP_FORMAT_STR \
> +	{ XFS_LOOKUP_EQi,	"eq" }, \
> +	{ XFS_LOOKUP_LEi,	"le" }, \
> +	{ XFS_LOOKUP_GEi,	"ge" }
> +
>  typedef enum {
>  	XFS_BTNUM_BNOi, XFS_BTNUM_CNTi, XFS_BTNUM_RMAPi, XFS_BTNUM_BMAPi,
>  	XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index dcde4e9907f7..3f57002ca660 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -2626,10 +2626,9 @@ DEFINE_AG_ERROR_EVENT(xfs_ag_resv_init_error);
>  #define DEFINE_AG_EXTENT_EVENT(name) DEFINE_DISCARD_EVENT(name)
>  
>  /* ag btree lookup tracepoint class */
> -#define XFS_AG_BTREE_CMP_FORMAT_STR \
> -	{ XFS_LOOKUP_EQ,	"eq" }, \
> -	{ XFS_LOOKUP_LE,	"le" }, \
> -	{ XFS_LOOKUP_GE,	"ge" }
> +TRACE_DEFINE_ENUM(XFS_LOOKUP_EQi);
> +TRACE_DEFINE_ENUM(XFS_LOOKUP_LEi);
> +TRACE_DEFINE_ENUM(XFS_LOOKUP_GEi);
>  DECLARE_EVENT_CLASS(xfs_ag_btree_lookup_class,
>  	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno,
>  		 xfs_agblock_t agbno, xfs_lookup_t dir),
> 

Re: [PATCH 4/6] xfs: move XFS_INODE_FORMAT_STR mappings to libxfs

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move XFS_INODE_FORMAT_STR to libxfs so that we don't forget to keep it
> updated, and add necessary TRACE_DEFINE_ENUM.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Now that I think of it, just adding a /* for ftrace */ above the #define
might be nice here and for the others, if you think it's appropriate, but:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_format.h |    7 +++++++
>  fs/xfs/xfs_trace.h         |   10 +++++-----
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index b15412e4c535..d8902114786d 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -925,6 +925,13 @@ typedef enum xfs_dinode_fmt {
>  	XFS_DINODE_FMT_UUID		/* added long ago, but never used */
>  } xfs_dinode_fmt_t;
>  
> +#define XFS_INODE_FORMAT_STR \
> +	{ XFS_DINODE_FMT_DEV,		"dev" }, \
> +	{ XFS_DINODE_FMT_LOCAL,		"local" }, \
> +	{ XFS_DINODE_FMT_EXTENTS,	"extent" }, \
> +	{ XFS_DINODE_FMT_BTREE,		"btree" }, \
> +	{ XFS_DINODE_FMT_UUID,		"uuid" }
> +
>  /*
>   * Inode minimum and maximum sizes.
>   */
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 3f57002ca660..94e289aca220 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -1901,11 +1901,11 @@ TRACE_EVENT(xfs_dir2_leafn_moveents,
>  	{ 0,	"target" }, \
>  	{ 1,	"temp" }
>  
> -#define XFS_INODE_FORMAT_STR \
> -	{ 0,	"invalid" }, \
> -	{ 1,	"local" }, \
> -	{ 2,	"extent" }, \
> -	{ 3,	"btree" }
> +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_DEV);
> +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_LOCAL);
> +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_EXTENTS);
> +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_BTREE);
> +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_UUID);
>  
>  DECLARE_EVENT_CLASS(xfs_swap_extent_class,
>  	TP_PROTO(struct xfs_inode *ip, int which),
> 

Re: [PATCH 4/6] xfs: move XFS_INODE_FORMAT_STR mappings to libxfs

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move XFS_INODE_FORMAT_STR to libxfs so that we don't forget to keep it
> updated, and add necessary TRACE_DEFINE_ENUM.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

...

> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 3f57002ca660..94e289aca220 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -1901,11 +1901,11 @@ TRACE_EVENT(xfs_dir2_leafn_moveents,
>  	{ 0,	"target" }, \
>  	{ 1,	"temp" }

^^^^ that context makes me wonder if we shouldn't move that #define
as well, and give 0 & 1 some meaningful macro names.  Patch 7/6? :)

-Eric

Re: [PATCH 5/6] xfs: stringify btree cursor types in ftrace output

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use __print_symbolic to print the btree type in ftrace output.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Ignore my previous question.

(if scrub's trace file starts duplicating too much of the common trace stuff,
though, we may want a common include file?)

But for now

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_types.h |    9 +++++++++
>  fs/xfs/scrub/trace.h      |   38 ++++++++++++++++++++++++++------------
>  fs/xfs/xfs_trace.h        |   12 ++++++++++--
>  3 files changed, 45 insertions(+), 14 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 297cf2318f83..eb04afe9cce0 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -114,6 +114,15 @@ typedef enum {
>  	XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX
>  } xfs_btnum_t;
>  
> +#define XFS_BTNUM_STRINGS \
> +	{ XFS_BTNUM_BNOi,	"bnobt" }, \
> +	{ XFS_BTNUM_CNTi,	"cntbt" }, \
> +	{ XFS_BTNUM_RMAPi,	"rmapbt" }, \
> +	{ XFS_BTNUM_BMAPi,	"bmbt" }, \
> +	{ XFS_BTNUM_INOi,	"inobt" }, \
> +	{ XFS_BTNUM_FINOi,	"finobt" }, \
> +	{ XFS_BTNUM_REFCi,	"refcbt" }
> +
>  struct xfs_name {
>  	const unsigned char	*name;
>  	int			len;
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index d67e915cf91a..0141eb1ac091 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -12,6 +12,20 @@
>  #include <linux/tracepoint.h>
>  #include "xfs_bit.h"
>  
> +/*
> + * ftrace's __print_symbolic requires that all enum values be wrapped in the
> + * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
> + * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
> + * code.
> + */
> +TRACE_DEFINE_ENUM(XFS_BTNUM_BNOi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_CNTi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_BMAPi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_INOi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
> +
>  DECLARE_EVENT_CLASS(xchk_class,
>  	TP_PROTO(struct xfs_inode *ip, struct xfs_scrub_metadata *sm,
>  		 int error),
> @@ -278,10 +292,10 @@ TRACE_EVENT(xchk_btree_op_error,
>  		__entry->error = error;
>  		__entry->ret_ip = ret_ip;
>  	),
> -	TP_printk("dev %d:%d type %u btnum %d level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
> +	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->type,
> -		  __entry->btnum,
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
>  		  __entry->level,
>  		  __entry->ptr,
>  		  __entry->agno,
> @@ -321,12 +335,12 @@ TRACE_EVENT(xchk_ifork_btree_op_error,
>  		__entry->error = error;
>  		__entry->ret_ip = ret_ip;
>  	),
> -	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btnum %d level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
> +	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->ino,
>  		  __entry->whichfork,
>  		  __entry->type,
> -		  __entry->btnum,
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
>  		  __entry->level,
>  		  __entry->ptr,
>  		  __entry->agno,
> @@ -360,10 +374,10 @@ TRACE_EVENT(xchk_btree_error,
>  		__entry->ptr = cur->bc_ptrs[level];
>  		__entry->ret_ip = ret_ip;
>  	),
> -	TP_printk("dev %d:%d type %u btnum %d level %d ptr %d agno %u agbno %u ret_ip %pS",
> +	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->type,
> -		  __entry->btnum,
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
>  		  __entry->level,
>  		  __entry->ptr,
>  		  __entry->agno,
> @@ -400,12 +414,12 @@ TRACE_EVENT(xchk_ifork_btree_error,
>  		__entry->ptr = cur->bc_ptrs[level];
>  		__entry->ret_ip = ret_ip;
>  	),
> -	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btnum %d level %d ptr %d agno %u agbno %u ret_ip %pS",
> +	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->ino,
>  		  __entry->whichfork,
>  		  __entry->type,
> -		  __entry->btnum,
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
>  		  __entry->level,
>  		  __entry->ptr,
>  		  __entry->agno,
> @@ -439,10 +453,10 @@ DECLARE_EVENT_CLASS(xchk_sbtree_class,
>  		__entry->nlevels = cur->bc_nlevels;
>  		__entry->ptr = cur->bc_ptrs[level];
>  	),
> -	TP_printk("dev %d:%d type %u btnum %d agno %u agbno %u level %d nlevels %d ptr %d",
> +	TP_printk("dev %d:%d type %u btree %s agno %u agbno %u level %d nlevels %d ptr %d",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->type,
> -		  __entry->btnum,
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
>  		  __entry->agno,
>  		  __entry->bno,
>  		  __entry->level,
> @@ -643,11 +657,11 @@ TRACE_EVENT(xrep_init_btblock,
>  		__entry->agbno = agbno;
>  		__entry->btnum = btnum;
>  	),
> -	TP_printk("dev %d:%d agno %u agbno %u btnum %d",
> +	TP_printk("dev %d:%d agno %u agbno %u btree %s",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->agno,
>  		  __entry->agbno,
> -		  __entry->btnum)
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS))
>  )
>  TRACE_EVENT(xrep_findroot_block,
>  	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, xfs_agblock_t agbno,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 94e289aca220..6fcc893dfc91 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -2194,6 +2194,14 @@ DEFINE_DISCARD_EVENT(xfs_discard_exclude);
>  DEFINE_DISCARD_EVENT(xfs_discard_busy);
>  
>  /* btree cursor events */
> +TRACE_DEFINE_ENUM(XFS_BTNUM_BNOi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_CNTi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_BMAPi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_INOi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
> +TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
> +
>  DECLARE_EVENT_CLASS(xfs_btree_cur_class,
>  	TP_PROTO(struct xfs_btree_cur *cur, int level, struct xfs_buf *bp),
>  	TP_ARGS(cur, level, bp),
> @@ -2213,9 +2221,9 @@ DECLARE_EVENT_CLASS(xfs_btree_cur_class,
>  		__entry->ptr = cur->bc_ptrs[level];
>  		__entry->daddr = bp ? bp->b_bn : -1;
>  	),
> -	TP_printk("dev %d:%d btnum %d level %d/%d ptr %d daddr 0x%llx",
> +	TP_printk("dev %d:%d btree %s level %d/%d ptr %d daddr 0x%llx",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> -		  __entry->btnum,
> +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
>  		  __entry->level,
>  		  __entry->nlevels,
>  		  __entry->ptr,
> 

Re: [PATCH 6/6] xfs: stringify scrub types in ftrace output

[Eric Sandeen]

On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use __print_symbolic to print the scrub type in ftrace output.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/scrub/trace.h |  103 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 77 insertions(+), 26 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 0141eb1ac091..3c83e8b3b39c 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -26,6 +26,57 @@ TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
>  TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
>  TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
>  
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PROBE);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SB);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGF);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGFL);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGI);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BNOBT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_CNTBT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INOBT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_FINOBT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RMAPBT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_REFCNTBT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INODE);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTD);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTA);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTC);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_DIR);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_XATTR);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SYMLINK);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PARENT);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTBITMAP);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTSUM);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_UQUOTA);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_GQUOTA);
> +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PQUOTA);
> +
> +#define XFS_SCRUB_TYPE_STRINGS \

Hm I thought you just moved these sorts of #defines /out/ of the trace files
and adjacent to their type definitions in core header files; why not this one?

> +	{ XFS_SCRUB_TYPE_PROBE,		"probe" }, \
> +	{ XFS_SCRUB_TYPE_SB,		"sb" }, \
> +	{ XFS_SCRUB_TYPE_AGF,		"agf" }, \
> +	{ XFS_SCRUB_TYPE_AGFL,		"agfl" }, \
> +	{ XFS_SCRUB_TYPE_AGI,		"agi" }, \
> +	{ XFS_SCRUB_TYPE_BNOBT,		"bnobt" }, \

-Eric

Re: [PATCH 4/6] xfs: move XFS_INODE_FORMAT_STR mappings to libxfs

[Darrick J. Wong]

On Tue, Dec 18, 2018 at 11:24:45AM -0600, Eric Sandeen wrote:
> On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move XFS_INODE_FORMAT_STR to libxfs so that we don't forget to keep it
> > updated, and add necessary TRACE_DEFINE_ENUM.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that I think of it, just adding a /* for ftrace */ above the #define
> might be nice here and for the others, if you think it's appropriate, but:

Will do, and hopefully nobody else will trip over this silly ftrace bomb.

--D

> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> > ---
> >  fs/xfs/libxfs/xfs_format.h |    7 +++++++
> >  fs/xfs/xfs_trace.h         |   10 +++++-----
> >  2 files changed, 12 insertions(+), 5 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index b15412e4c535..d8902114786d 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -925,6 +925,13 @@ typedef enum xfs_dinode_fmt {
> >  	XFS_DINODE_FMT_UUID		/* added long ago, but never used */
> >  } xfs_dinode_fmt_t;
> >  
> > +#define XFS_INODE_FORMAT_STR \
> > +	{ XFS_DINODE_FMT_DEV,		"dev" }, \
> > +	{ XFS_DINODE_FMT_LOCAL,		"local" }, \
> > +	{ XFS_DINODE_FMT_EXTENTS,	"extent" }, \
> > +	{ XFS_DINODE_FMT_BTREE,		"btree" }, \
> > +	{ XFS_DINODE_FMT_UUID,		"uuid" }
> > +
> >  /*
> >   * Inode minimum and maximum sizes.
> >   */
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 3f57002ca660..94e289aca220 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -1901,11 +1901,11 @@ TRACE_EVENT(xfs_dir2_leafn_moveents,
> >  	{ 0,	"target" }, \
> >  	{ 1,	"temp" }
> >  
> > -#define XFS_INODE_FORMAT_STR \
> > -	{ 0,	"invalid" }, \
> > -	{ 1,	"local" }, \
> > -	{ 2,	"extent" }, \
> > -	{ 3,	"btree" }
> > +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_DEV);
> > +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_LOCAL);
> > +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_EXTENTS);
> > +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_BTREE);
> > +TRACE_DEFINE_ENUM(XFS_DINODE_FMT_UUID);
> >  
> >  DECLARE_EVENT_CLASS(xfs_swap_extent_class,
> >  	TP_PROTO(struct xfs_inode *ip, int which),
> > 

Re: [PATCH 4/6] xfs: move XFS_INODE_FORMAT_STR mappings to libxfs

[Darrick J. Wong]

On Tue, Dec 18, 2018 at 11:25:41AM -0600, Eric Sandeen wrote:
> On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move XFS_INODE_FORMAT_STR to libxfs so that we don't forget to keep it
> > updated, and add necessary TRACE_DEFINE_ENUM.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> ...
> 
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 3f57002ca660..94e289aca220 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -1901,11 +1901,11 @@ TRACE_EVENT(xfs_dir2_leafn_moveents,
> >  	{ 0,	"target" }, \
> >  	{ 1,	"temp" }
> 
> ^^^^ that context makes me wonder if we shouldn't move that #define
> as well, and give 0 & 1 some meaningful macro names.  Patch 7/6? :)

Or just combine them into a single tracepoint that emits both inodes.

--D

> -Eric

Re: [PATCH 5/6] xfs: stringify btree cursor types in ftrace output

[Darrick J. Wong]

On Tue, Dec 18, 2018 at 11:27:42AM -0600, Eric Sandeen wrote:
> On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Use __print_symbolic to print the btree type in ftrace output.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Ignore my previous question.
> 
> (if scrub's trace file starts duplicating too much of the common trace stuff,
> though, we may want a common include file?)

I wasn't planning to expand any further than the scrub type.

--D

> But for now
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> > ---
> >  fs/xfs/libxfs/xfs_types.h |    9 +++++++++
> >  fs/xfs/scrub/trace.h      |   38 ++++++++++++++++++++++++++------------
> >  fs/xfs/xfs_trace.h        |   12 ++++++++++--
> >  3 files changed, 45 insertions(+), 14 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> > index 297cf2318f83..eb04afe9cce0 100644
> > --- a/fs/xfs/libxfs/xfs_types.h
> > +++ b/fs/xfs/libxfs/xfs_types.h
> > @@ -114,6 +114,15 @@ typedef enum {
> >  	XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX
> >  } xfs_btnum_t;
> >  
> > +#define XFS_BTNUM_STRINGS \
> > +	{ XFS_BTNUM_BNOi,	"bnobt" }, \
> > +	{ XFS_BTNUM_CNTi,	"cntbt" }, \
> > +	{ XFS_BTNUM_RMAPi,	"rmapbt" }, \
> > +	{ XFS_BTNUM_BMAPi,	"bmbt" }, \
> > +	{ XFS_BTNUM_INOi,	"inobt" }, \
> > +	{ XFS_BTNUM_FINOi,	"finobt" }, \
> > +	{ XFS_BTNUM_REFCi,	"refcbt" }
> > +
> >  struct xfs_name {
> >  	const unsigned char	*name;
> >  	int			len;
> > diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> > index d67e915cf91a..0141eb1ac091 100644
> > --- a/fs/xfs/scrub/trace.h
> > +++ b/fs/xfs/scrub/trace.h
> > @@ -12,6 +12,20 @@
> >  #include <linux/tracepoint.h>
> >  #include "xfs_bit.h"
> >  
> > +/*
> > + * ftrace's __print_symbolic requires that all enum values be wrapped in the
> > + * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
> > + * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
> > + * code.
> > + */
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_BNOi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_CNTi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_BMAPi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_INOi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
> > +
> >  DECLARE_EVENT_CLASS(xchk_class,
> >  	TP_PROTO(struct xfs_inode *ip, struct xfs_scrub_metadata *sm,
> >  		 int error),
> > @@ -278,10 +292,10 @@ TRACE_EVENT(xchk_btree_op_error,
> >  		__entry->error = error;
> >  		__entry->ret_ip = ret_ip;
> >  	),
> > -	TP_printk("dev %d:%d type %u btnum %d level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
> > +	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->type,
> > -		  __entry->btnum,
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
> >  		  __entry->level,
> >  		  __entry->ptr,
> >  		  __entry->agno,
> > @@ -321,12 +335,12 @@ TRACE_EVENT(xchk_ifork_btree_op_error,
> >  		__entry->error = error;
> >  		__entry->ret_ip = ret_ip;
> >  	),
> > -	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btnum %d level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
> > +	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u error %d ret_ip %pS",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->ino,
> >  		  __entry->whichfork,
> >  		  __entry->type,
> > -		  __entry->btnum,
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
> >  		  __entry->level,
> >  		  __entry->ptr,
> >  		  __entry->agno,
> > @@ -360,10 +374,10 @@ TRACE_EVENT(xchk_btree_error,
> >  		__entry->ptr = cur->bc_ptrs[level];
> >  		__entry->ret_ip = ret_ip;
> >  	),
> > -	TP_printk("dev %d:%d type %u btnum %d level %d ptr %d agno %u agbno %u ret_ip %pS",
> > +	TP_printk("dev %d:%d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->type,
> > -		  __entry->btnum,
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
> >  		  __entry->level,
> >  		  __entry->ptr,
> >  		  __entry->agno,
> > @@ -400,12 +414,12 @@ TRACE_EVENT(xchk_ifork_btree_error,
> >  		__entry->ptr = cur->bc_ptrs[level];
> >  		__entry->ret_ip = ret_ip;
> >  	),
> > -	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btnum %d level %d ptr %d agno %u agbno %u ret_ip %pS",
> > +	TP_printk("dev %d:%d ino 0x%llx fork %d type %u btree %s level %d ptr %d agno %u agbno %u ret_ip %pS",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->ino,
> >  		  __entry->whichfork,
> >  		  __entry->type,
> > -		  __entry->btnum,
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
> >  		  __entry->level,
> >  		  __entry->ptr,
> >  		  __entry->agno,
> > @@ -439,10 +453,10 @@ DECLARE_EVENT_CLASS(xchk_sbtree_class,
> >  		__entry->nlevels = cur->bc_nlevels;
> >  		__entry->ptr = cur->bc_ptrs[level];
> >  	),
> > -	TP_printk("dev %d:%d type %u btnum %d agno %u agbno %u level %d nlevels %d ptr %d",
> > +	TP_printk("dev %d:%d type %u btree %s agno %u agbno %u level %d nlevels %d ptr %d",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->type,
> > -		  __entry->btnum,
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
> >  		  __entry->agno,
> >  		  __entry->bno,
> >  		  __entry->level,
> > @@ -643,11 +657,11 @@ TRACE_EVENT(xrep_init_btblock,
> >  		__entry->agbno = agbno;
> >  		__entry->btnum = btnum;
> >  	),
> > -	TP_printk("dev %d:%d agno %u agbno %u btnum %d",
> > +	TP_printk("dev %d:%d agno %u agbno %u btree %s",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->agno,
> >  		  __entry->agbno,
> > -		  __entry->btnum)
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS))
> >  )
> >  TRACE_EVENT(xrep_findroot_block,
> >  	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, xfs_agblock_t agbno,
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 94e289aca220..6fcc893dfc91 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -2194,6 +2194,14 @@ DEFINE_DISCARD_EVENT(xfs_discard_exclude);
> >  DEFINE_DISCARD_EVENT(xfs_discard_busy);
> >  
> >  /* btree cursor events */
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_BNOi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_CNTi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_BMAPi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_INOi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
> > +TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
> > +
> >  DECLARE_EVENT_CLASS(xfs_btree_cur_class,
> >  	TP_PROTO(struct xfs_btree_cur *cur, int level, struct xfs_buf *bp),
> >  	TP_ARGS(cur, level, bp),
> > @@ -2213,9 +2221,9 @@ DECLARE_EVENT_CLASS(xfs_btree_cur_class,
> >  		__entry->ptr = cur->bc_ptrs[level];
> >  		__entry->daddr = bp ? bp->b_bn : -1;
> >  	),
> > -	TP_printk("dev %d:%d btnum %d level %d/%d ptr %d daddr 0x%llx",
> > +	TP_printk("dev %d:%d btree %s level %d/%d ptr %d daddr 0x%llx",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > -		  __entry->btnum,
> > +		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
> >  		  __entry->level,
> >  		  __entry->nlevels,
> >  		  __entry->ptr,
> > 

Re: [PATCH 6/6] xfs: stringify scrub types in ftrace output

[Darrick J. Wong]

On Tue, Dec 18, 2018 at 11:30:16AM -0600, Eric Sandeen wrote:
> On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Use __print_symbolic to print the scrub type in ftrace output.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/scrub/trace.h |  103 +++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 77 insertions(+), 26 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> > index 0141eb1ac091..3c83e8b3b39c 100644
> > --- a/fs/xfs/scrub/trace.h
> > +++ b/fs/xfs/scrub/trace.h
> > @@ -26,6 +26,57 @@ TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
> >  TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
> >  TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
> >  
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PROBE);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SB);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGF);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGFL);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGI);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BNOBT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_CNTBT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INOBT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_FINOBT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RMAPBT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_REFCNTBT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INODE);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTD);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTA);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTC);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_DIR);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_XATTR);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SYMLINK);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PARENT);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTBITMAP);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTSUM);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_UQUOTA);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_GQUOTA);
> > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PQUOTA);
> > +
> > +#define XFS_SCRUB_TYPE_STRINGS \
> 
> Hm I thought you just moved these sorts of #defines /out/ of the trace files
> and adjacent to their type definitions in core header files; why not this one?

[paraphrasing a conversation on irc]

So I'm becoming more convinced that these stringify things should all
just move to xfs_trace.h and scrub/trace.h.  The /only/ users are the
tracepoints, but (according to Dave, I think?) the reason for putting
them next to the enum definition is so that we don't forget to update
the string list when we update the enums.

However, now that we know we have to maintain this TRACE_DEFINE_ENUM
hugpile (and it has to be in the trace header file) we might as well
move the stringify crap to the trace headers and leave a comment in
xfs_format.h.

--D

> > +	{ XFS_SCRUB_TYPE_PROBE,		"probe" }, \
> > +	{ XFS_SCRUB_TYPE_SB,		"sb" }, \
> > +	{ XFS_SCRUB_TYPE_AGF,		"agf" }, \
> > +	{ XFS_SCRUB_TYPE_AGFL,		"agfl" }, \
> > +	{ XFS_SCRUB_TYPE_AGI,		"agi" }, \
> > +	{ XFS_SCRUB_TYPE_BNOBT,		"bnobt" }, \
> 
> -Eric

Re: [PATCH 6/6] xfs: stringify scrub types in ftrace output

[Darrick J. Wong]

On Tue, Dec 18, 2018 at 10:41:41AM -0800, Darrick J. Wong wrote:
> On Tue, Dec 18, 2018 at 11:30:16AM -0600, Eric Sandeen wrote:
> > On 11/28/18 5:29 PM, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Use __print_symbolic to print the scrub type in ftrace output.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  fs/xfs/scrub/trace.h |  103 +++++++++++++++++++++++++++++++++++++-------------
> > >  1 file changed, 77 insertions(+), 26 deletions(-)
> > > 
> > > 
> > > diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> > > index 0141eb1ac091..3c83e8b3b39c 100644
> > > --- a/fs/xfs/scrub/trace.h
> > > +++ b/fs/xfs/scrub/trace.h
> > > @@ -26,6 +26,57 @@ TRACE_DEFINE_ENUM(XFS_BTNUM_FINOi);
> > >  TRACE_DEFINE_ENUM(XFS_BTNUM_RMAPi);
> > >  TRACE_DEFINE_ENUM(XFS_BTNUM_REFCi);
> > >  
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PROBE);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SB);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGF);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGFL);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_AGI);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BNOBT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_CNTBT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INOBT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_FINOBT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RMAPBT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_REFCNTBT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_INODE);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTD);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTA);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_BMBTC);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_DIR);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_XATTR);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_SYMLINK);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PARENT);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTBITMAP);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_RTSUM);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_UQUOTA);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_GQUOTA);
> > > +TRACE_DEFINE_ENUM(XFS_SCRUB_TYPE_PQUOTA);
> > > +
> > > +#define XFS_SCRUB_TYPE_STRINGS \
> > 
> > Hm I thought you just moved these sorts of #defines /out/ of the trace files
> > and adjacent to their type definitions in core header files; why not this one?

Forgot to answer the original question: Because I really don't want
ftrace stringification junk ending up in the xfs_fs.h that we ship in
xfslibs-dev.

> [paraphrasing a conversation on irc]
> 
> So I'm becoming more convinced that these stringify things should all
> just move to xfs_trace.h and scrub/trace.h.  The /only/ users are the
> tracepoints, but (according to Dave, I think?) the reason for putting
> them next to the enum definition is so that we don't forget to update
> the string list when we update the enums.
> 
> However, now that we know we have to maintain this TRACE_DEFINE_ENUM
> hugpile (and it has to be in the trace header file) we might as well
> move the stringify crap to the trace headers and leave a comment in
> xfs_format.h.

Unfortunately... this also means we can't share the stringifier between
xfs and xfs_scrub's tracepoints.  Soooo ... I think I'm going to leave
things as they are here and get on with other things.

(Not thrilled to have spent three hours analyzing where to put stringify
macros, tbh...)

--D

> --D
> 
> > > +	{ XFS_SCRUB_TYPE_PROBE,		"probe" }, \
> > > +	{ XFS_SCRUB_TYPE_SB,		"sb" }, \
> > > +	{ XFS_SCRUB_TYPE_AGF,		"agf" }, \
> > > +	{ XFS_SCRUB_TYPE_AGFL,		"agfl" }, \
> > > +	{ XFS_SCRUB_TYPE_AGI,		"agi" }, \
> > > +	{ XFS_SCRUB_TYPE_BNOBT,		"bnobt" }, \
> > 
> > -Eric

[PATCH 4/6] xfs: move XFS_INODE_FORMAT_STR mappings to libxfs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Move XFS_INODE_FORMAT_STR to libxfs so that we don't forget to keep it
updated, and add necessary TRACE_DEFINE_ENUM.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_format.h |   10 ++++++++++
 fs/xfs/xfs_trace.h         |   10 +++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index b15412e4c535..9bb3c48843ec 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -916,6 +916,9 @@ static inline uint xfs_dinode_size(int version)
 
 /*
  * Values for di_format
+ *
+ * This enum is used in string mapping in xfs_trace.h; please keep the
+ * TRACE_DEFINE_ENUMs for it up to date.
  */
 typedef enum xfs_dinode_fmt {
 	XFS_DINODE_FMT_DEV,		/* xfs_dev_t */
@@ -925,6 +928,13 @@ typedef enum xfs_dinode_fmt {
 	XFS_DINODE_FMT_UUID		/* added long ago, but never used */
 } xfs_dinode_fmt_t;
 
+#define XFS_INODE_FORMAT_STR \
+	{ XFS_DINODE_FMT_DEV,		"dev" }, \
+	{ XFS_DINODE_FMT_LOCAL,		"local" }, \
+	{ XFS_DINODE_FMT_EXTENTS,	"extent" }, \
+	{ XFS_DINODE_FMT_BTREE,		"btree" }, \
+	{ XFS_DINODE_FMT_UUID,		"uuid" }
+
 /*
  * Inode minimum and maximum sizes.
  */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 3f57002ca660..94e289aca220 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1901,11 +1901,11 @@ TRACE_EVENT(xfs_dir2_leafn_moveents,
 	{ 0,	"target" }, \
 	{ 1,	"temp" }
 
-#define XFS_INODE_FORMAT_STR \
-	{ 0,	"invalid" }, \
-	{ 1,	"local" }, \
-	{ 2,	"extent" }, \
-	{ 3,	"btree" }
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_DEV);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_LOCAL);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_EXTENTS);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_BTREE);
+TRACE_DEFINE_ENUM(XFS_DINODE_FMT_UUID);
 
 DECLARE_EVENT_CLASS(xfs_swap_extent_class,
 	TP_PROTO(struct xfs_inode *ip, int which),