[PATCH 0/2] btrfs: trace: Add trace events for extent_io_tree

[PATCH 0/2] btrfs: trace: Add trace events for extent_io_tree

[Qu Wenruo]

This patchset can be fetched from github:
https://github.com/adam900710/linux/tree/ftrace_extent_io_tree
Which is based on v5.0-rc7 tag.

Btrfs uses (almost abuse) extent_io_tree for a lot of operations, e.g:
- Tree block locking
  BTRFS_I(btree_inode)->io_tree
- Page status tracing
  BTRFS_I(file_inode)->io_tree and BTRFS_I(file_inode)->io_failure_tree
  transaction->dirty_pages
- Pinned down extents tracing
  fs_info->freed_extents[2]
...

However we don't have any trace events for us to understand how btrfs
works.

This patchset will introduce trace events for
set/clear/convert_extent_bit().

The output examples and extra notes are: (copied from the 2nd patch)

  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22036480 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22040576 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22044672 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22048768 len=4096 set_bits=LOCKED
  btrfs_clear_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22036480 len=16384 clear_bits=LOCKED
  ^^^ Extent buffer 22036480 read from disc, the locking progress

  btrfs_set_extent_bit: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30425088 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30441472 len=16384 set_bits=DIRTY
  ^^^ 2 new tree blocks allocated in one transaction

  btrfs_set_extent_bit: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30523392 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30556160 len=16384 set_bits=DIRTY
  ^^^ 2 old tree blocks get pinned down

There are 2 points which need extra attention:
1) Those trace events are pretty heavy
   The following workload would generate over 400 trace events.
	mkfs.btrfs -f $dev
	start_trace
	mount $dev $mnt -o enospc_debug
	sync
	touch $mnt/file1
	touch $mnt/file2
	touch $mnt/file3
	xfs_io -f -c "pwrite 0 16k" $mnt/file4
	umount $mnt
	end_trace
   It's not recommended to use them in real world environment.

2) No way to distinguish between different btrfs filesystems
   Due to the fact that we can't get fs_info easily from extent_io_tree,
   there is no way to distinguish events from different fses.
   To avoid extra noise, please ensure there is only one btrfs in your
   test environment.

Qu Wenruo (2):
  btrfs: Introduce extent_io_tree::owner to distinguish different
    io_trees
  btrfs: trace: Add trace events for extent_io_tree

 fs/btrfs/disk-io.c               |  12 ++-
 fs/btrfs/extent_io.c             |   7 +-
 fs/btrfs/extent_io.h             |  20 +++-
 fs/btrfs/inode.c                 |   5 +-
 fs/btrfs/relocation.c            |   3 +-
 fs/btrfs/tests/btrfs-tests.c     |   6 +-
 fs/btrfs/tests/extent-io-tests.c |   2 +-
 fs/btrfs/transaction.c           |   2 +-
 include/trace/events/btrfs.h     | 151 +++++++++++++++++++++++++++++++
 9 files changed, 194 insertions(+), 14 deletions(-)

-- 
2.21.0

[PATCH 1/2] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees

[Qu Wenruo]

Btrfs has the following different extent_io_trees used:
- fs_info::free_extents[2]
- btrfs_inode::io_tree
  For both normal inodes and btree inode
- btrfs_inode::io_failure_tree
- btrfs_transaction::dirty_pages
- btrfs_root::dirty_log_pages

If we want to trace bits change in those io_trees, it will be pretty
hard to distinguish them.

Instead of using hard-to-read pointer address, this patch will introduc
a new member extent_io_tree::owner, which uses the unpopulated bits of
extent_io_tree::track_uptodate, to track who owns this extent_io_tree.

This modification needs all the callers of extent_io_tree_init() to
accept a new parameter @owner.

This patch provides the basis for later ftrace events.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c               | 12 ++++++++----
 fs/btrfs/extent_io.c             |  3 ++-
 fs/btrfs/extent_io.h             | 20 ++++++++++++++++++--
 fs/btrfs/inode.c                 |  5 +++--
 fs/btrfs/relocation.c            |  3 ++-
 fs/btrfs/tests/btrfs-tests.c     |  6 ++++--
 fs/btrfs/tests/extent-io-tests.c |  2 +-
 fs/btrfs/transaction.c           |  2 +-
 8 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6a2a2a951705..ab5b23be086c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1205,7 +1205,8 @@ static void __setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
 	root->log_transid_committed = -1;
 	root->last_log_commit = 0;
 	if (!dummy)
-		extent_io_tree_init(&root->dirty_log_pages, NULL);
+		extent_io_tree_init(&root->dirty_log_pages,
+				    IO_TREE_ROOT_DIRTY_LOG_PAGES, NULL);
 
 	memset(&root->root_key, 0, sizeof(root->root_key));
 	memset(&root->root_item, 0, sizeof(root->root_item));
@@ -2129,7 +2130,8 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info)
 	inode->i_mapping->a_ops = &btree_aops;
 
 	RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node);
-	extent_io_tree_init(&BTRFS_I(inode)->io_tree, inode);
+	extent_io_tree_init(&BTRFS_I(inode)->io_tree, IO_TREE_INODE_IO_TREE,
+			    inode);
 	BTRFS_I(inode)->io_tree.track_uptodate = 0;
 	extent_map_tree_init(&BTRFS_I(inode)->extent_tree);
 
@@ -2739,8 +2741,10 @@ int open_ctree(struct super_block *sb,
 	fs_info->block_group_cache_tree = RB_ROOT;
 	fs_info->first_logical_byte = (u64)-1;
 
-	extent_io_tree_init(&fs_info->freed_extents[0], NULL);
-	extent_io_tree_init(&fs_info->freed_extents[1], NULL);
+	extent_io_tree_init(&fs_info->freed_extents[0],
+			    IO_TREE_FS_INFO_FREED_EXTENTS0, NULL);
+	extent_io_tree_init(&fs_info->freed_extents[1],
+			    IO_TREE_FS_INFO_FREED_EXTENTS1, NULL);
 	fs_info->pinned_extents = &fs_info->freed_extents[0];
 	set_bit(BTRFS_FS_BARRIER, &fs_info->flags);
 
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 52abe4082680..8b1d76261e53 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -200,7 +200,7 @@ void __cold extent_io_exit(void)
 	bioset_exit(&btrfs_bioset);
 }
 
-void extent_io_tree_init(struct extent_io_tree *tree,
+void extent_io_tree_init(struct extent_io_tree *tree, unsigned int owner,
 			 void *private_data)
 {
 	tree->state = RB_ROOT;
@@ -208,6 +208,7 @@ void extent_io_tree_init(struct extent_io_tree *tree,
 	tree->dirty_bytes = 0;
 	spin_lock_init(&tree->lock);
 	tree->private_data = private_data;
+	tree->owner = owner;
 }
 
 static struct extent_state *alloc_extent_state(gfp_t mask)
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 9673be3f3d1f..84654157ca92 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -107,11 +107,26 @@ struct extent_io_ops {
 				    int mirror);
 };
 
+enum {
+	IO_TREE_FS_INFO_FREED_EXTENTS0,
+	IO_TREE_FS_INFO_FREED_EXTENTS1,
+	IO_TREE_INODE_IO_TREE,
+	IO_TREE_INODE_IO_FAILURE_TREE,
+	IO_TREE_RELOCATION_PROCESSED_BLOCKS,
+	IO_TREE_TRANSACTION_DIRTY_PAGES,
+	IO_TREE_ROOT_DIRTY_LOG_PAGES,
+	IO_TREE_TMP,
+};
+
 struct extent_io_tree {
 	struct rb_root state;
 	void *private_data;
 	u64 dirty_bytes;
-	int track_uptodate;
+	unsigned int track_uptodate:1;
+
+	/* who owns this io tree, should be above IO_TREE_* enum */
+	unsigned int owner:15;
+
 	spinlock_t lock;
 	const struct extent_io_ops *ops;
 };
@@ -240,7 +255,8 @@ typedef struct extent_map *(get_extent_t)(struct btrfs_inode *inode,
 					  u64 start, u64 len,
 					  int create);
 
-void extent_io_tree_init(struct extent_io_tree *tree, void *private_data);
+void extent_io_tree_init(struct extent_io_tree *tree, unsigned int owner,
+			 void *private_data);
 int try_release_extent_mapping(struct page *page, gfp_t mask);
 int try_release_extent_buffer(struct page *page);
 int lock_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5c349667c761..8eaa1c22346a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9128,8 +9128,9 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
 
 	inode = &ei->vfs_inode;
 	extent_map_tree_init(&ei->extent_tree);
-	extent_io_tree_init(&ei->io_tree, inode);
-	extent_io_tree_init(&ei->io_failure_tree, inode);
+	extent_io_tree_init(&ei->io_tree, IO_TREE_INODE_IO_TREE, inode);
+	extent_io_tree_init(&ei->io_failure_tree,
+			    IO_TREE_INODE_IO_FAILURE_TREE, inode);
 	ei->io_tree.track_uptodate = 1;
 	ei->io_failure_tree.track_uptodate = 1;
 	atomic_set(&ei->sync_writers, 0);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 272b287f8cf0..7e1d3b6fc050 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4175,7 +4175,8 @@ static struct reloc_control *alloc_reloc_control(void)
 	INIT_LIST_HEAD(&rc->reloc_roots);
 	backref_cache_init(&rc->backref_cache);
 	mapping_tree_init(&rc->reloc_root_tree);
-	extent_io_tree_init(&rc->processed_blocks, NULL);
+	extent_io_tree_init(&rc->processed_blocks,
+			    IO_TREE_RELOCATION_PROCESSED_BLOCKS, NULL);
 	return rc;
 }
 
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index 8a59597f1883..f34f2e3b271d 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -115,8 +115,10 @@ struct btrfs_fs_info *btrfs_alloc_dummy_fs_info(u32 nodesize, u32 sectorsize)
 	INIT_LIST_HEAD(&fs_info->tree_mod_seq_list);
 	INIT_RADIX_TREE(&fs_info->buffer_radix, GFP_ATOMIC);
 	INIT_RADIX_TREE(&fs_info->fs_roots_radix, GFP_ATOMIC);
-	extent_io_tree_init(&fs_info->freed_extents[0], NULL);
-	extent_io_tree_init(&fs_info->freed_extents[1], NULL);
+	extent_io_tree_init(&fs_info->freed_extents[0],
+			    IO_TREE_FS_INFO_FREED_EXTENTS0, NULL);
+	extent_io_tree_init(&fs_info->freed_extents[1],
+			    IO_TREE_FS_INFO_FREED_EXTENTS1, NULL);
 	fs_info->pinned_extents = &fs_info->freed_extents[0];
 	set_bit(BTRFS_FS_STATE_DUMMY_FS_INFO, &fs_info->fs_state);
 
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 3c46d7f23456..7eaeea348b31 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -77,7 +77,7 @@ static int test_find_delalloc(u32 sectorsize)
 		return -ENOMEM;
 	}
 
-	extent_io_tree_init(&tmp, NULL);
+	extent_io_tree_init(&tmp, IO_TREE_TMP, NULL);
 
 	/*
 	 * First go through and create and mark all of our pages dirty, we pin
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 4ec2b660d014..c609d1b396ee 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -274,7 +274,7 @@ static noinline int join_transaction(struct btrfs_fs_info *fs_info,
 	spin_lock_init(&cur_trans->dropped_roots_lock);
 	list_add_tail(&cur_trans->list, &fs_info->trans_list);
 	extent_io_tree_init(&cur_trans->dirty_pages,
-			     fs_info->btree_inode);
+			IO_TREE_TRANSACTION_DIRTY_PAGES, fs_info->btree_inode);
 	fs_info->generation++;
 	cur_trans->transid = fs_info->generation;
 	fs_info->running_transaction = cur_trans;
-- 
2.21.0

[PATCH 2/2] btrfs: trace: Add trace events for extent_io_tree

[Qu Wenruo]

Although btrfs heavily relies on extent_io_tree, we don't really have
any good trace events for them.

This patch will add the folowing trace events:
trace_btrfs_set_extent_bit()
trace_btrfs_clear_extent_bit()
trace_btrfs_convert_extent_bit()

The output would be:
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22036480 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22040576 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22044672 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22048768 len=4096 set_bits=LOCKED
  btrfs_clear_extent_bit: io_tree=INODE_IO_TREE ino=1 root=1 start=22036480 len=16384 clear_bits=LOCKED
  ^^^ Extent buffer 22036480 read from disc, the locking progress

  btrfs_set_extent_bit: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30425088 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30441472 len=16384 set_bits=DIRTY
  ^^^ 2 new tree blocks allocated in one transaction

  btrfs_set_extent_bit: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30523392 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30556160 len=16384 set_bits=DIRTY
  ^^^ 2 old tree blocks get pinned down

There are 2 points which need extra attention:
1) Those trace events are pretty heavy
   The following workload would generate over 400 trace events.
	mkfs.btrfs -f $dev
	start_trace
	mount $dev $mnt -o enospc_debug
	sync
	touch $mnt/file1
	touch $mnt/file2
	touch $mnt/file3
	xfs_io -f -c "pwrite 0 16k" $mnt/file4
	umount $mnt
	end_trace
   It's not recommended to use them in real world environment.

2) No way to distinguish between different btrfs filesystems
   Due to the fact that we can't get fs_info easily from extent_io_tree,
   there is no way to distinguish events from different fses.
   To avoid extra noise, please ensure there is only one btrfs in your
   test environment.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_io.c         |   4 +
 include/trace/events/btrfs.h | 151 +++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 8b1d76261e53..421417e4eb11 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -580,6 +580,7 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	int clear = 0;
 
 	btrfs_debug_check_extent_io_range(tree, start, end);
+	trace_btrfs_clear_extent_bit(tree, start, end - start + 1, bits);
 
 	if (bits & EXTENT_DELALLOC)
 		bits |= EXTENT_NORESERVE;
@@ -850,6 +851,7 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	u64 last_end;
 
 	btrfs_debug_check_extent_io_range(tree, start, end);
+	trace_btrfs_set_extent_bit(tree, start, end - start + 1, bits);
 
 	bits |= EXTENT_FIRST_DELALLOC;
 again:
@@ -1083,6 +1085,8 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	bool first_iteration = true;
 
 	btrfs_debug_check_extent_io_range(tree, start, end);
+	trace_btrfs_convert_extent_bit(tree, start, end - start + 1, bits,
+				       clear_bits);
 
 again:
 	if (!prealloc) {
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 2887503e4d12..4c8e4c60c422 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -27,6 +27,7 @@ struct btrfs_work;
 struct __btrfs_workqueue;
 struct btrfs_qgroup_extent_record;
 struct btrfs_qgroup;
+struct extent_io_tree;
 struct prelim_ref;
 
 TRACE_DEFINE_ENUM(FLUSH_DELAYED_ITEMS_NR);
@@ -77,6 +78,17 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 		{ 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_INFO_FREED_EXTENTS0, "FREED_EXTENTS0" },	\
+		{ IO_TREE_FS_INFO_FREED_EXTENTS1, "FREED_EXTENTS1" },	\
+		{ IO_TREE_INODE_IO_TREE,	 "INODE_IO_TREE" },	\
+		{ IO_TREE_INODE_IO_FAILURE_TREE, "INODE_IO_FAILURE" },	\
+		{ IO_TREE_RELOCATION_PROCESSED_BLOCKS, "RELOCATION" },	\
+		{ IO_TREE_TRANSACTION_DIRTY_PAGES, "TRANS_DIRTY_PAGES" },\
+		{ IO_TREE_ROOT_DIRTY_LOG_PAGES, "ROOT_DIRTY_LOG" },	\
+		{ IO_TREE_TMP })
+
 #define BTRFS_GROUP_FLAGS	\
 	{ BTRFS_BLOCK_GROUP_DATA,	"DATA"},	\
 	{ BTRFS_BLOCK_GROUP_SYSTEM,	"SYSTEM"},	\
@@ -88,6 +100,25 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
 	{ BTRFS_BLOCK_GROUP_RAID5,	"RAID5"},	\
 	{ BTRFS_BLOCK_GROUP_RAID6,	"RAID6"}
 
+#define EXTENT_FLAGS					\
+	{ EXTENT_DIRTY,		"DIRTY"},		\
+	{ EXTENT_WRITEBACK,	"WRITEBACK"},		\
+	{ EXTENT_UPTODATE,	"UPTODATE"},		\
+	{ EXTENT_LOCKED,	"LOCKED"},		\
+	{ EXTENT_NEW,		"NEW"},			\
+	{ EXTENT_DELALLOC,	"DELALLOC"},		\
+	{ EXTENT_DEFRAG,	"DEFRAG"},		\
+	{ EXTENT_BOUNDARY,	"BOUNDARY"},		\
+	{ EXTENT_NODATASUM,	"NODATASUM"},		\
+	{ EXTENT_CLEAR_META_RESV,"CLEAR_META_RSV"},	\
+	{ EXTENT_FIRST_DELALLOC,"FIRST_DELALLOC"},	\
+	{ EXTENT_NEED_WAIT,	"NEED_WAIT"},		\
+	{ EXTENT_DAMAGED,	"DAMAGED"},		\
+	{ EXTENT_NORESERVE,	"NORESERVE"},		\
+	{ EXTENT_QGROUP_RESERVED,"QGROUP_RESERVED"},	\
+	{ EXTENT_CLEAR_DATA_RESV,"CLEAR_DATA_RESV"},	\
+	{ EXTENT_DELALLOC_NEW,	"DELALLOC_NEW"}
+
 #define BTRFS_FSID_SIZE 16
 #define TP_STRUCT__entry_fsid __array(u8, fsid, BTRFS_FSID_SIZE)
 
@@ -1878,6 +1909,126 @@ DEFINE_EVENT(btrfs__block_group, btrfs_skip_unused_block_group,
 	TP_ARGS(bg_cache)
 );
 
+TRACE_EVENT(btrfs_set_extent_bit,
+	TP_PROTO(const struct extent_io_tree *tree,
+		 u64 start, u64 len, unsigned set_bits),
+
+	TP_ARGS(tree, start, len, set_bits),
+
+	TP_STRUCT__entry(
+		__field(	unsigned,	owner	)
+		__field(	u64,		ino	)
+		__field(	u64,		rootid	)
+		__field(	u64,		start	)
+		__field(	u64,		len	)
+		__field(	unsigned,	set_bits)
+	),
+
+	TP_fast_assign(
+		__entry->owner	= tree->owner;
+		if (tree->private_data) {
+			struct inode *inode = tree->private_data;
+
+			__entry->ino	= inode->i_ino;
+			__entry->rootid	=
+				BTRFS_I(inode)->root->root_key.objectid;
+		} else {
+			__entry->ino	= 0;
+			__entry->rootid	= 0;
+		}
+		__entry->start		= start;
+		__entry->len		= len;
+		__entry->set_bits	= set_bits;
+	),
+
+	TP_printk(
+"io_tree=%s ino=%llu root=%llu start=%llu len=%llu set_bits=%s",
+		  show_extent_io_tree_owner(__entry->owner), __entry->ino,
+		  __entry->rootid, __entry->start, __entry->len,
+		  __print_flags(__entry->set_bits, "|", EXTENT_FLAGS))
+);
+
+TRACE_EVENT(btrfs_clear_extent_bit,
+	TP_PROTO(const struct extent_io_tree *tree,
+		 u64 start, u64 len, unsigned clear_bits),
+
+	TP_ARGS(tree, start, len, clear_bits),
+
+	TP_STRUCT__entry(
+		__field(	unsigned,	owner	)
+		__field(	u64,		ino	)
+		__field(	u64,		rootid	)
+		__field(	u64,		start	)
+		__field(	u64,		len	)
+		__field(	unsigned,	clear_bits)
+	),
+
+	TP_fast_assign(
+		__entry->owner	= tree->owner;
+		if (tree->private_data) {
+			struct inode *inode = tree->private_data;
+
+			__entry->ino	= inode->i_ino;
+			__entry->rootid	=
+				BTRFS_I(inode)->root->root_key.objectid;
+		} else {
+			__entry->ino	= 0;
+			__entry->rootid	= 0;
+		}
+		__entry->start		= start;
+		__entry->len		= len;
+		__entry->clear_bits	= clear_bits;
+	),
+
+	TP_printk(
+"io_tree=%s ino=%llu root=%llu start=%llu len=%llu clear_bits=%s",
+		  show_extent_io_tree_owner(__entry->owner), __entry->ino,
+		  __entry->rootid, __entry->start, __entry->len,
+		  __print_flags(__entry->clear_bits, "|", EXTENT_FLAGS))
+);
+
+TRACE_EVENT(btrfs_convert_extent_bit,
+	TP_PROTO(const struct extent_io_tree *tree,
+		 u64 start, u64 len, unsigned set_bits, unsigned clear_bits),
+
+	TP_ARGS(tree, start, len, set_bits, clear_bits),
+
+	TP_STRUCT__entry(
+		__field(	unsigned,	owner	)
+		__field(	u64,		ino	)
+		__field(	u64,		rootid	)
+		__field(	u64,		start	)
+		__field(	u64,		len	)
+		__field(	unsigned,	set_bits)
+		__field(	unsigned,	clear_bits)
+	),
+
+	TP_fast_assign(
+		__entry->owner	= tree->owner;
+		if (tree->private_data) {
+			struct inode *inode = tree->private_data;
+
+			__entry->ino	= inode->i_ino;
+			__entry->rootid	=
+				BTRFS_I(inode)->root->root_key.objectid;
+		} else {
+			__entry->ino	= 0;
+			__entry->rootid	= 0;
+		}
+		__entry->start		= start;
+		__entry->len		= len;
+		__entry->set_bits	= set_bits;
+		__entry->clear_bits	= clear_bits;
+	),
+
+	TP_printk(
+"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,
+		  __entry->rootid, __entry->start, __entry->len,
+		  __print_flags(__entry->set_bits , "|", EXTENT_FLAGS),
+		  __print_flags(__entry->clear_bits, "|", EXTENT_FLAGS))
+);
+
 #endif /* _TRACE_BTRFS_H */
 
 /* This part must be outside protection */
-- 
2.21.0

Re: [PATCH 0/2] btrfs: trace: Add trace events for extent_io_tree

[David Sterba]

On Thu, Feb 28, 2019 at 06:02:53PM +0800, Qu Wenruo wrote:
> 2) No way to distinguish between different btrfs filesystems
>    Due to the fact that we can't get fs_info easily from extent_io_tree,
>    there is no way to distinguish events from different fses.
>    To avoid extra noise, please ensure there is only one btrfs in your
>    test environment.

The id of the filesystem is IMO important for tracepoints, I think we
can live with the 8 bytes of fs_info pointer in extent_io_tree.

Re: [PATCH 0/2] btrfs: trace: Add trace events for extent_io_tree

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 714 bytes --]



On 2019/2/28 下午9:51, David Sterba wrote:
> On Thu, Feb 28, 2019 at 06:02:53PM +0800, Qu Wenruo wrote:
>> 2) No way to distinguish between different btrfs filesystems
>>    Due to the fact that we can't get fs_info easily from extent_io_tree,
>>    there is no way to distinguish events from different fses.
>>    To avoid extra noise, please ensure there is only one btrfs in your
>>    test environment.
> 
> The id of the filesystem is IMO important for tracepoints, I think we
> can live with the 8 bytes of fs_info pointer in extent_io_tree.

That's my original concern about the extra 8 bytes.
But since you're OK with that, I'm completely fine adding fs_info into them.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] btrfs: trace: Add trace events for extent_io_tree

[David Sterba]

On Thu, Feb 28, 2019 at 06:02:55PM +0800, Qu Wenruo wrote:
>  TRACE_DEFINE_ENUM(FLUSH_DELAYED_ITEMS_NR);
> +#define show_extent_io_tree_owner(owner)				\
> +	__print_symbolic(owner,						\
> +		{ IO_TREE_FS_INFO_FREED_EXTENTS0, "FREED_EXTENTS0" },	\
> +		{ IO_TREE_FS_INFO_FREED_EXTENTS1, "FREED_EXTENTS1" },	\
> +		{ IO_TREE_INODE_IO_TREE,	 "INODE_IO_TREE" },	\
> +		{ IO_TREE_INODE_IO_FAILURE_TREE, "INODE_IO_FAILURE" },	\
> +		{ IO_TREE_RELOCATION_PROCESSED_BLOCKS, "RELOCATION" },	\
> +		{ IO_TREE_TRANSACTION_DIRTY_PAGES, "TRANS_DIRTY_PAGES" },\
> +		{ IO_TREE_ROOT_DIRTY_LOG_PAGES, "ROOT_DIRTY_LOG" },	\
> +		{ IO_TREE_TMP })

Please align the strings

> +
>  #define BTRFS_GROUP_FLAGS	\
>  	{ BTRFS_BLOCK_GROUP_DATA,	"DATA"},	\
>  	{ BTRFS_BLOCK_GROUP_SYSTEM,	"SYSTEM"},	\
> @@ -88,6 +100,25 @@ TRACE_DEFINE_ENUM(COMMIT_TRANS);
>  	{ BTRFS_BLOCK_GROUP_RAID5,	"RAID5"},	\
>  	{ BTRFS_BLOCK_GROUP_RAID6,	"RAID6"}
>  
> +#define EXTENT_FLAGS					\
> +	{ EXTENT_DIRTY,		"DIRTY"},		\
> +	{ EXTENT_WRITEBACK,	"WRITEBACK"},		\
> +	{ EXTENT_UPTODATE,	"UPTODATE"},		\
> +	{ EXTENT_LOCKED,	"LOCKED"},		\
> +	{ EXTENT_NEW,		"NEW"},			\
> +	{ EXTENT_DELALLOC,	"DELALLOC"},		\
> +	{ EXTENT_DEFRAG,	"DEFRAG"},		\
> +	{ EXTENT_BOUNDARY,	"BOUNDARY"},		\
> +	{ EXTENT_NODATASUM,	"NODATASUM"},		\
> +	{ EXTENT_CLEAR_META_RESV,"CLEAR_META_RSV"},	\
> +	{ EXTENT_FIRST_DELALLOC,"FIRST_DELALLOC"},	\
> +	{ EXTENT_NEED_WAIT,	"NEED_WAIT"},		\
> +	{ EXTENT_DAMAGED,	"DAMAGED"},		\
> +	{ EXTENT_NORESERVE,	"NORESERVE"},		\
> +	{ EXTENT_QGROUP_RESERVED,"QGROUP_RESERVED"},	\
> +	{ EXTENT_CLEAR_DATA_RESV,"CLEAR_DATA_RESV"},	\
> +	{ EXTENT_DELALLOC_NEW,	"DELALLOC_NEW"}

and there, the maximum line width is 80 and there's enough space.

> +
>  #define BTRFS_FSID_SIZE 16
>  #define TP_STRUCT__entry_fsid __array(u8, fsid, BTRFS_FSID_SIZE)
>  
> @@ -1878,6 +1909,126 @@ DEFINE_EVENT(btrfs__block_group, btrfs_skip_unused_block_group,
>  	TP_ARGS(bg_cache)
>  );
>  
> +TRACE_EVENT(btrfs_set_extent_bit,
> +	TP_PROTO(const struct extent_io_tree *tree,
> +		 u64 start, u64 len, unsigned set_bits),
> +
> +	TP_ARGS(tree, start, len, set_bits),
> +
> +	TP_STRUCT__entry(
> +		__field(	unsigned,	owner	)
> +		__field(	u64,		ino	)
> +		__field(	u64,		rootid	)
> +		__field(	u64,		start	)
> +		__field(	u64,		len	)
> +		__field(	unsigned,	set_bits)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->owner	= tree->owner;
> +		if (tree->private_data) {
> +			struct inode *inode = tree->private_data;
> +
> +			__entry->ino	= inode->i_ino;

Don't use raw i_ino but btrfs_ino

> +			__entry->rootid	=
> +				BTRFS_I(inode)->root->root_key.objectid;
> +		} else {
> +			__entry->ino	= 0;
> +			__entry->rootid	= 0;
> +		}
> +		__entry->start		= start;
> +		__entry->len		= len;
> +		__entry->set_bits	= set_bits;
> +	),

Re: [PATCH 1/2] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees

[David Sterba]

On Thu, Feb 28, 2019 at 06:02:54PM +0800, Qu Wenruo wrote:
> Btrfs has the following different extent_io_trees used:
> - fs_info::free_extents[2]
> - btrfs_inode::io_tree
>   For both normal inodes and btree inode
> - btrfs_inode::io_failure_tree
> - btrfs_transaction::dirty_pages
> - btrfs_root::dirty_log_pages
> 
> If we want to trace bits change in those io_trees, it will be pretty
> hard to distinguish them.
> 
> Instead of using hard-to-read pointer address, this patch will introduc
> a new member extent_io_tree::owner, which uses the unpopulated bits of
> extent_io_tree::track_uptodate, to track who owns this extent_io_tree.
> 
> This modification needs all the callers of extent_io_tree_init() to
> accept a new parameter @owner.
> 
> This patch provides the basis for later ftrace events.

Nice.

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/disk-io.c               | 12 ++++++++----
>  fs/btrfs/extent_io.c             |  3 ++-
>  fs/btrfs/extent_io.h             | 20 ++++++++++++++++++--
>  fs/btrfs/inode.c                 |  5 +++--
>  fs/btrfs/relocation.c            |  3 ++-
>  fs/btrfs/tests/btrfs-tests.c     |  6 ++++--
>  fs/btrfs/tests/extent-io-tests.c |  2 +-
>  fs/btrfs/transaction.c           |  2 +-
>  8 files changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 6a2a2a951705..ab5b23be086c 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1205,7 +1205,8 @@ static void __setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
>  	root->log_transid_committed = -1;
>  	root->last_log_commit = 0;
>  	if (!dummy)
> -		extent_io_tree_init(&root->dirty_log_pages, NULL);
> +		extent_io_tree_init(&root->dirty_log_pages,
> +				    IO_TREE_ROOT_DIRTY_LOG_PAGES, NULL);
>  
>  	memset(&root->root_key, 0, sizeof(root->root_key));
>  	memset(&root->root_item, 0, sizeof(root->root_item));
> @@ -2129,7 +2130,8 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info)
>  	inode->i_mapping->a_ops = &btree_aops;
>  
>  	RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node);
> -	extent_io_tree_init(&BTRFS_I(inode)->io_tree, inode);
> +	extent_io_tree_init(&BTRFS_I(inode)->io_tree, IO_TREE_INODE_IO_TREE,
> +			    inode);
>  	BTRFS_I(inode)->io_tree.track_uptodate = 0;
>  	extent_map_tree_init(&BTRFS_I(inode)->extent_tree);
>  
> @@ -2739,8 +2741,10 @@ int open_ctree(struct super_block *sb,
>  	fs_info->block_group_cache_tree = RB_ROOT;
>  	fs_info->first_logical_byte = (u64)-1;
>  
> -	extent_io_tree_init(&fs_info->freed_extents[0], NULL);
> -	extent_io_tree_init(&fs_info->freed_extents[1], NULL);
> +	extent_io_tree_init(&fs_info->freed_extents[0],
> +			    IO_TREE_FS_INFO_FREED_EXTENTS0, NULL);
> +	extent_io_tree_init(&fs_info->freed_extents[1],
> +			    IO_TREE_FS_INFO_FREED_EXTENTS1, NULL);
>  	fs_info->pinned_extents = &fs_info->freed_extents[0];
>  	set_bit(BTRFS_FS_BARRIER, &fs_info->flags);
>  
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 52abe4082680..8b1d76261e53 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -200,7 +200,7 @@ void __cold extent_io_exit(void)
>  	bioset_exit(&btrfs_bioset);
>  }
>  
> -void extent_io_tree_init(struct extent_io_tree *tree,
> +void extent_io_tree_init(struct extent_io_tree *tree, unsigned int owner,
>  			 void *private_data)
>  {
>  	tree->state = RB_ROOT;
> @@ -208,6 +208,7 @@ void extent_io_tree_init(struct extent_io_tree *tree,
>  	tree->dirty_bytes = 0;
>  	spin_lock_init(&tree->lock);
>  	tree->private_data = private_data;
> +	tree->owner = owner;
>  }
>  
>  static struct extent_state *alloc_extent_state(gfp_t mask)
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 9673be3f3d1f..84654157ca92 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -107,11 +107,26 @@ struct extent_io_ops {
>  				    int mirror);
>  };
>  
> +enum {
> +	IO_TREE_FS_INFO_FREED_EXTENTS0,
> +	IO_TREE_FS_INFO_FREED_EXTENTS1,
> +	IO_TREE_INODE_IO_TREE,
> +	IO_TREE_INODE_IO_FAILURE_TREE,
> +	IO_TREE_RELOCATION_PROCESSED_BLOCKS,

That's 35 chars long, looks like you got some inspiration in the device
replace identifiers but it's not the example to follow.
IO_TREE_RELOC_BLOCKS. Others look ok.

> +	IO_TREE_TRANSACTION_DIRTY_PAGES,
> +	IO_TREE_ROOT_DIRTY_LOG_PAGES,
> +	IO_TREE_TMP,

That one is for tests so the name should reflect it.

> +};
> +
>  struct extent_io_tree {
>  	struct rb_root state;
>  	void *private_data;
>  	u64 dirty_bytes;
> -	int track_uptodate;
> +	unsigned int track_uptodate:1;
> +
> +	/* who owns this io tree, should be above IO_TREE_* enum */
> +	unsigned int owner:15;
> +
>  	spinlock_t lock;

Note that spinlocks next to bitfields is an unsafe construct that can
lead to corrupted locks if the bitfield update is not atomic. This
happens on some arches.

You can use bool for track_uptodate and char or u8 for the owner.

>  	const struct extent_io_ops *ops;
>  };