All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree
@ 2019-03-01  2:47 Qu Wenruo
  2019-03-01  2:47 ` [PATCH v2 1/3] btrfs: Introduce fs_info " Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Qu Wenruo @ 2019-03-01  2:47 UTC (permalink / raw)
  To: linux-btrfs

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().

Despite that, there are some other small modifications required:
- Introduce extent_io_tree::fs_info
  For trace events to output fsid to distinguish different fs.
  This increase the size of extent_io_tree by 8 bytes.

- Allow NULL fs_info for TP_fast_assign_fsid()
  There is extent bits operation in selftest which is too deep to pass
  fs_info. And since it's in selftest, it shouldn't trigger trace
  events.
  But to be safe, we still need to check fs_indo in
  TP_fast_assign_fsid(), for NULL fs_info, just keep fsid filled with
  zero.

- New extent_io_tree::owner
  To distinguish different extent io trees. It uses the unpopulated bits
  of original trace_uptodate, so it doesn't increase the size of
  extent_io_tree.

The output examples and extra notes are: (copied from the 3rd patch)
  btrfs_set_extent_bit: <FDID>: io_tree=IO_TREE ino=1 root=1 start=22036480 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22040576 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22044672 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22048768 len=4096 set_bits=LOCKED
  btrfs_clear_extent_bit: <FSID>: io_tree=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: <FSID>: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30425088 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: <FSID>: 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: <FSID>: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30523392 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: <FSID>: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30556160 len=16384 set_bits=DIRTY
  ^^^ 2 old tree blocks get pinned down

There is one point 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.

Changelog:
v2:
- Introduce fs_info to distinguish different btrfs filesystems
- Code style change to make trace code more elegant
- Minor IO_TREE_* naming change.
- Use btrfs_ino() to replace raw inode number.
- Change extent_io_tree::owner declaration to avoid affecting spinlock.

Qu Wenruo (3):
  btrfs: Introduce fs_info for extent_io_tree
  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             |   9 +-
 fs/btrfs/extent_io.h             |  22 ++++-
 fs/btrfs/inode.c                 |   6 +-
 fs/btrfs/relocation.c            |   9 +-
 fs/btrfs/tests/btrfs-tests.c     |   6 +-
 fs/btrfs/tests/extent-io-tests.c |   2 +-
 fs/btrfs/transaction.c           |   4 +-
 include/trace/events/btrfs.h     | 159 ++++++++++++++++++++++++++++++-
 9 files changed, 210 insertions(+), 19 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] btrfs: Introduce fs_info for extent_io_tree
  2019-03-01  2:47 [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
@ 2019-03-01  2:47 ` Qu Wenruo
  2019-03-01  2:47 ` [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees Qu Wenruo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2019-03-01  2:47 UTC (permalink / raw)
  To: linux-btrfs

This patch will introduce a new member, fs_info for extent_io_tree.

This provides the basis for later ftrace events to distinguish the
output between different btrfs filesystems.

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

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6a2a2a951705..9cf93645ac00 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1205,7 +1205,7 @@ 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(fs_info, &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 +2129,7 @@ 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(fs_info, &BTRFS_I(inode)->io_tree, inode);
 	BTRFS_I(inode)->io_tree.track_uptodate = 0;
 	extent_map_tree_init(&BTRFS_I(inode)->extent_tree);
 
@@ -2739,8 +2739,8 @@ 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, &fs_info->freed_extents[0], NULL);
+	extent_io_tree_init(fs_info, &fs_info->freed_extents[1], 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..dc4c526dcf7d 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -200,9 +200,10 @@ void __cold extent_io_exit(void)
 	bioset_exit(&btrfs_bioset);
 }
 
-void extent_io_tree_init(struct extent_io_tree *tree,
-			 void *private_data)
+void extent_io_tree_init(struct btrfs_fs_info *fs_info,
+			 struct extent_io_tree *tree, void *private_data)
 {
+	tree->fs_info = fs_info;
 	tree->state = RB_ROOT;
 	tree->ops = NULL;
 	tree->dirty_bytes = 0;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 9673be3f3d1f..6cb462b05f5a 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -109,6 +109,7 @@ struct extent_io_ops {
 
 struct extent_io_tree {
 	struct rb_root state;
+	struct btrfs_fs_info *fs_info;
 	void *private_data;
 	u64 dirty_bytes;
 	int track_uptodate;
@@ -240,7 +241,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 btrfs_fs_info *fs_info,
+			 struct extent_io_tree *tree, 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..2d316799eff5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9128,8 +9128,8 @@ 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(fs_info, &ei->io_tree, inode);
+	extent_io_tree_init(fs_info, &ei->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..152e7ac15b01 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4164,7 +4164,7 @@ struct inode *create_reloc_inode(struct btrfs_fs_info *fs_info,
 	return inode;
 }
 
-static struct reloc_control *alloc_reloc_control(void)
+static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 {
 	struct reloc_control *rc;
 
@@ -4175,7 +4175,7 @@ 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(fs_info, &rc->processed_blocks, NULL);
 	return rc;
 }
 
@@ -4217,7 +4217,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 		return -ETXTBSY;
 	}
 
-	rc = alloc_reloc_control();
+	rc = alloc_reloc_control(fs_info);
 	if (!rc) {
 		btrfs_put_block_group(bg);
 		return -ENOMEM;
@@ -4413,7 +4413,7 @@ int btrfs_recover_relocation(struct btrfs_root *root)
 	if (list_empty(&reloc_roots))
 		goto out;
 
-	rc = alloc_reloc_control();
+	rc = alloc_reloc_control(fs_info);
 	if (!rc) {
 		err = -ENOMEM;
 		goto out;
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index 8a59597f1883..cc1e5d017dc0 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -115,8 +115,8 @@ 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, &fs_info->freed_extents[0], NULL);
+	extent_io_tree_init(fs_info, &fs_info->freed_extents[1], 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..6ac9770a974c 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(NULL, &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..638b16c859ba 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -273,7 +273,7 @@ static noinline int join_transaction(struct btrfs_fs_info *fs_info,
 	INIT_LIST_HEAD(&cur_trans->deleted_bgs);
 	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,
+	extent_io_tree_init(fs_info, &cur_trans->dirty_pages,
 			     fs_info->btree_inode);
 	fs_info->generation++;
 	cur_trans->transid = fs_info->generation;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees
  2019-03-01  2:47 [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
  2019-03-01  2:47 ` [PATCH v2 1/3] btrfs: Introduce fs_info " Qu Wenruo
@ 2019-03-01  2:47 ` Qu Wenruo
  2019-03-07 17:10   ` David Sterba
  2019-03-11 15:20   ` David Sterba
  2019-03-01  2:48 ` [PATCH v2 3/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
  2019-03-07 16:32 ` [PATCH v2 0/3] " David Sterba
  3 siblings, 2 replies; 10+ messages in thread
From: Qu Wenruo @ 2019-03-01  2:47 UTC (permalink / raw)
  To: linux-btrfs

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             |  4 +++-
 fs/btrfs/extent_io.h             | 22 +++++++++++++++++++---
 fs/btrfs/inode.c                 |  6 ++++--
 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, 42 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9cf93645ac00..e3d7e4a4f035 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(fs_info, &root->dirty_log_pages, NULL);
+		extent_io_tree_init(fs_info, &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(fs_info, &BTRFS_I(inode)->io_tree, inode);
+	extent_io_tree_init(fs_info, &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, &fs_info->freed_extents[0], NULL);
-	extent_io_tree_init(fs_info, &fs_info->freed_extents[1], NULL);
+	extent_io_tree_init(fs_info, &fs_info->freed_extents[0],
+			    IO_TREE_FS_INFO_FREED_EXTENTS0, NULL);
+	extent_io_tree_init(fs_info, &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 dc4c526dcf7d..dd2cfd054eef 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -201,7 +201,8 @@ void __cold extent_io_exit(void)
 }
 
 void extent_io_tree_init(struct btrfs_fs_info *fs_info,
-			 struct extent_io_tree *tree, void *private_data)
+			 struct extent_io_tree *tree, unsigned int owner,
+			 void *private_data)
 {
 	tree->fs_info = fs_info;
 	tree->state = RB_ROOT;
@@ -209,6 +210,7 @@ void extent_io_tree_init(struct btrfs_fs_info *fs_info,
 	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 6cb462b05f5a..54736a22fa2f 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -107,12 +107,27 @@ 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_RELOC_BLOCKS,
+	IO_TREE_TRANSACTION_DIRTY_PAGES,
+	IO_TREE_ROOT_DIRTY_LOG_PAGES,
+	IO_TREE_SELFTEST_TMP,
+};
+
 struct extent_io_tree {
 	struct rb_root state;
 	struct btrfs_fs_info *fs_info;
 	void *private_data;
 	u64 dirty_bytes;
-	int track_uptodate;
+	bool track_uptodate;
+
+	/* who owns this io tree, should be above IO_TREE_* enum */
+	u8 owner;
+
 	spinlock_t lock;
 	const struct extent_io_ops *ops;
 };
@@ -241,8 +256,9 @@ typedef struct extent_map *(get_extent_t)(struct btrfs_inode *inode,
 					  u64 start, u64 len,
 					  int create);
 
-void extent_io_tree_init(struct btrfs_fs_info *fs_info,
-			 struct extent_io_tree *tree, void *private_data);
+void extent_io_tree_init(struct btrfs_fs_info *fs_info, 
+			 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 2d316799eff5..accbe06caf84 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9128,8 +9128,10 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
 
 	inode = &ei->vfs_inode;
 	extent_map_tree_init(&ei->extent_tree);
-	extent_io_tree_init(fs_info, &ei->io_tree, inode);
-	extent_io_tree_init(fs_info, &ei->io_failure_tree, inode);
+	extent_io_tree_init(fs_info, &ei->io_tree, IO_TREE_INODE_IO_TREE,
+			    inode);
+	extent_io_tree_init(fs_info, &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 152e7ac15b01..0f5ed4a7c022 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4175,7 +4175,8 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 	INIT_LIST_HEAD(&rc->reloc_roots);
 	backref_cache_init(&rc->backref_cache);
 	mapping_tree_init(&rc->reloc_root_tree);
-	extent_io_tree_init(fs_info, &rc->processed_blocks, NULL);
+	extent_io_tree_init(fs_info, &rc->processed_blocks,
+			    IO_TREE_RELOC_BLOCKS, NULL);
 	return rc;
 }
 
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index cc1e5d017dc0..1351ac2afdd2 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, &fs_info->freed_extents[0], NULL);
-	extent_io_tree_init(fs_info, &fs_info->freed_extents[1], NULL);
+	extent_io_tree_init(fs_info, &fs_info->freed_extents[0],
+			    IO_TREE_FS_INFO_FREED_EXTENTS0, NULL);
+	extent_io_tree_init(fs_info, &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 6ac9770a974c..60812f4d3fe3 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(NULL, &tmp, NULL);
+	extent_io_tree_init(NULL, &tmp, IO_TREE_SELFTEST_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 638b16c859ba..4799a3b6e2c9 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(fs_info, &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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] btrfs: trace: Add trace events for extent_io_tree
  2019-03-01  2:47 [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
  2019-03-01  2:47 ` [PATCH v2 1/3] btrfs: Introduce fs_info " Qu Wenruo
  2019-03-01  2:47 ` [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees Qu Wenruo
@ 2019-03-01  2:48 ` Qu Wenruo
  2019-03-07 17:14   ` David Sterba
  2019-03-07 16:32 ` [PATCH v2 0/3] " David Sterba
  3 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2019-03-01  2:48 UTC (permalink / raw)
  To: linux-btrfs

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()

Since selftest could create temporary extent_io_tree without fs_info,
modify TP_fast_assign_fsid() to accept NULL as fs_info.
NULL fs_info will lead to all zero fsid.

The output would be:
  btrfs_set_extent_bit: <FDID>: io_tree=IO_TREE ino=1 root=1 start=22036480 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22040576 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22044672 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22048768 len=4096 set_bits=LOCKED
  btrfs_clear_extent_bit: <FSID>: io_tree=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: <FSID>: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30425088 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: <FSID>: 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: <FSID>: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30523392 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: <FSID>: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30556160 len=16384 set_bits=DIRTY
  ^^^ 2 old tree blocks get pinned down

There is one point 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.

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

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index dd2cfd054eef..d9ef31def740 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -582,6 +582,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;
@@ -852,6 +853,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:
@@ -1085,6 +1087,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..2b4779b1855a 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,		"IO_TREE" },          \
+		{ IO_TREE_INODE_IO_FAILURE_TREE,	"IO_FAILURE_TREE" },  \
+		{ IO_TREE_RELOC_BLOCKS,			"RELOCATION" },	      \
+		{ IO_TREE_TRANSACTION_DIRTY_PAGES,	"TRANS_DIRTY_PAGES" },\
+		{ IO_TREE_ROOT_DIRTY_LOG_PAGES,		"ROOT_DIRTY_LOG" },   \
+		{ IO_TREE_SELFTEST_TMP,			"SELFTEST_TMP" })
+
 #define BTRFS_GROUP_FLAGS	\
 	{ BTRFS_BLOCK_GROUP_DATA,	"DATA"},	\
 	{ BTRFS_BLOCK_GROUP_SYSTEM,	"SYSTEM"},	\
@@ -88,11 +100,36 @@ 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)
 
 #define TP_fast_assign_fsid(fs_info)					\
-	memcpy(__entry->fsid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE)
+({									\
+ 	if (fs_info)							\
+		memcpy(__entry->fsid, fs_info->fs_devices->fsid,	\
+		       BTRFS_FSID_SIZE);				\
+	else								\
+		memset(__entry->fsid, 0, BTRFS_FSID_SIZE);		\
+})
 
 #define TP_STRUCT__entry_btrfs(args...)					\
 	TP_STRUCT__entry(						\
@@ -1878,6 +1915,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_btrfs(
+		__field(	unsigned,	owner	)
+		__field(	u64,		ino	)
+		__field(	u64,		rootid	)
+		__field(	u64,		start	)
+		__field(	u64,		len	)
+		__field(	unsigned,	set_bits)
+	),
+
+	TP_fast_assign_btrfs(tree->fs_info,
+		__entry->owner	= tree->owner;
+		if (tree->private_data) {
+			struct inode *inode = tree->private_data;
+
+			__entry->ino	= btrfs_ino(BTRFS_I(inode));
+			__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_btrfs(
+"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_btrfs(
+		__field(	unsigned,	owner	)
+		__field(	u64,		ino	)
+		__field(	u64,		rootid	)
+		__field(	u64,		start	)
+		__field(	u64,		len	)
+		__field(	unsigned,	clear_bits)
+	),
+
+	TP_fast_assign_btrfs(tree->fs_info,
+		__entry->owner	= tree->owner;
+		if (tree->private_data) {
+			struct inode *inode = tree->private_data;
+
+			__entry->ino	= btrfs_ino(BTRFS_I(inode));
+			__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_btrfs(
+"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_btrfs(
+		__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_btrfs(tree->fs_info,
+		__entry->owner	= tree->owner;
+		if (tree->private_data) {
+			struct inode *inode = tree->private_data;
+
+			__entry->ino	= btrfs_ino(BTRFS_I(inode));
+			__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_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,
+		  __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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree
  2019-03-01  2:47 [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
                   ` (2 preceding siblings ...)
  2019-03-01  2:48 ` [PATCH v2 3/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
@ 2019-03-07 16:32 ` David Sterba
  2019-03-08  0:41   ` Qu Wenruo
  3 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2019-03-07 16:32 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Mar 01, 2019 at 10:47:57AM +0800, Qu Wenruo wrote:
> - Allow NULL fs_info for TP_fast_assign_fsid()
>   There is extent bits operation in selftest which is too deep to pass
>   fs_info. And since it's in selftest, it shouldn't trigger trace
>   events.
>   But to be safe, we still need to check fs_indo in
>   TP_fast_assign_fsid(), for NULL fs_info, just keep fsid filled with
>   zero.

Ok, better be safe here. I'd still like to remove the conditional, as
the tests typically access only a single filesystem we could export the
fs_info globally, avoiding the need to pass it around.

> There is one point which need extra attention:
> 1) Those trace events are pretty heavy
>    The following workload would generate over 400 trace events.

I'm not sure if 400 is considered a lot, I'd say 400.000 would be a lot
for the steps below.

> 	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.
> 
> Changelog:
> v2:
> - Introduce fs_info to distinguish different btrfs filesystems
> - Code style change to make trace code more elegant
> - Minor IO_TREE_* naming change.
> - Use btrfs_ino() to replace raw inode number.
> - Change extent_io_tree::owner declaration to avoid affecting spinlock.

v2 looks good to me, thanks. I'll add it to the 5.2 queue.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees
  2019-03-01  2:47 ` [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees Qu Wenruo
@ 2019-03-07 17:10   ` David Sterba
  2019-03-11 15:20   ` David Sterba
  1 sibling, 0 replies; 10+ messages in thread
From: David Sterba @ 2019-03-07 17:10 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Mar 01, 2019 at 10:47:59AM +0800, Qu Wenruo wrote:
> +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_RELOC_BLOCKS,
> +	IO_TREE_TRANSACTION_DIRTY_PAGES,
> +	IO_TREE_ROOT_DIRTY_LOG_PAGES,
> +	IO_TREE_SELFTEST_TMP,

Well, I guess that's always something that sounds like a pointless
nitpicking but future code readers will complain, so allow me to
complain now.

IO_TREE_INODE_IO_TREE repeats the 'IO_TREE', so my idea is do only
IO_TREE_INODE_IO, similarly IO_TREE_INODE_IO_FAILURE.

And IO_TREE_SELFTEST, the 'TMP' part seems redundant.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] btrfs: trace: Add trace events for extent_io_tree
  2019-03-01  2:48 ` [PATCH v2 3/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
@ 2019-03-07 17:14   ` David Sterba
  0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2019-03-07 17:14 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Mar 01, 2019 at 10:48:00AM +0800, Qu Wenruo wrote:
> +#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,		"IO_TREE" },          \
> +		{ IO_TREE_INODE_IO_FAILURE_TREE,	"IO_FAILURE_TREE" },  \
> +		{ IO_TREE_RELOC_BLOCKS,			"RELOCATION" },	      \
> +		{ IO_TREE_TRANSACTION_DIRTY_PAGES,	"TRANS_DIRTY_PAGES" },\
> +		{ IO_TREE_ROOT_DIRTY_LOG_PAGES,		"ROOT_DIRTY_LOG" },   \
> +		{ IO_TREE_SELFTEST_TMP,			"SELFTEST_TMP" })

Additional to the previous patch, I think the point of the strings is to
match the constants or at least be an exact substring. When I see a
trace event I can simply copy&paste and search inside the sources if I'm
interested. So in this case it would be without the IO_TREE_ prefix.

I'm going to update the patche as it's only a simple rename, no need to
resend.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree
  2019-03-07 16:32 ` [PATCH v2 0/3] " David Sterba
@ 2019-03-08  0:41   ` Qu Wenruo
  0 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2019-03-08  0:41 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, linux-btrfs


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



On 2019/3/8 上午12:32, David Sterba wrote:
> On Fri, Mar 01, 2019 at 10:47:57AM +0800, Qu Wenruo wrote:
>> - Allow NULL fs_info for TP_fast_assign_fsid()
>>   There is extent bits operation in selftest which is too deep to pass
>>   fs_info. And since it's in selftest, it shouldn't trigger trace
>>   events.
>>   But to be safe, we still need to check fs_indo in
>>   TP_fast_assign_fsid(), for NULL fs_info, just keep fsid filled with
>>   zero.
> 
> Ok, better be safe here. I'd still like to remove the conditional, as
> the tests typically access only a single filesystem we could export the
> fs_info globally, avoiding the need to pass it around.

In fact, it's purely to be safe here.

Since the selftest is only executed at module load time, it's pretty
hard to enable trace event at that small time windows.
And if btrfs is complied into kernel, it's definitely impossible.

> 
>> There is one point which need extra attention:
>> 1) Those trace events are pretty heavy
>>    The following workload would generate over 400 trace events.
> 
> I'm not sure if 400 is considered a lot, I'd say 400.000 would be a lot
> for the steps below.

Considering other btrfs events are hardly to hit 40 for that small
workload, among btrfs specific events, it looks a little heavy.

> 
>> 	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.
>>
>> Changelog:
>> v2:
>> - Introduce fs_info to distinguish different btrfs filesystems
>> - Code style change to make trace code more elegant
>> - Minor IO_TREE_* naming change.
>> - Use btrfs_ino() to replace raw inode number.
>> - Change extent_io_tree::owner declaration to avoid affecting spinlock.
> 
> v2 looks good to me, thanks. I'll add it to the 5.2 queue.
> 
Thanks for merging and update it for the renaming.

Thanks,
Qu


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees
  2019-03-01  2:47 ` [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees Qu Wenruo
  2019-03-07 17:10   ` David Sterba
@ 2019-03-11 15:20   ` David Sterba
  1 sibling, 0 replies; 10+ messages in thread
From: David Sterba @ 2019-03-11 15:20 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Mar 01, 2019 at 10:47:59AM +0800, Qu Wenruo wrote:
>  struct extent_io_tree {
>  	struct rb_root state;
>  	struct btrfs_fs_info *fs_info;
>  	void *private_data;
>  	u64 dirty_bytes;
> -	int track_uptodate;
> +	bool track_uptodate;

Next time please do such change in a separate patch, I did that myself
for now.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree
@ 2019-03-01  2:45 Qu Wenruo
  0 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2019-03-01  2:45 UTC (permalink / raw)
  To: linux-btrfs

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().

Despite that, there are some other small modifications required:
- Introduce extent_io_tree::fs_info
  For trace events to output fsid to distinguish different fs.
  This increase the size of extent_io_tree by 8 bytes.

- Allow NULL fs_info for TP_fast_assign_fsid()
  There is extent bits operation in selftest which is too deep to pass
  fs_info. And since it's in selftest, it shouldn't trigger trace
  events.
  But to be safe, we still need to check fs_indo in
  TP_fast_assign_fsid(), for NULL fs_info, just keep fsid filled with
  zero.

- New extent_io_tree::owner
  To distinguish different extent io trees. It uses the unpopulated bits
  of original trace_uptodate, so it doesn't increase the size of
  extent_io_tree.

The output examples and extra notes are: (copied from the 3rd patch)
  btrfs_set_extent_bit: <FDID>: io_tree=IO_TREE ino=1 root=1 start=22036480 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22040576 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22044672 len=4096 set_bits=LOCKED
  btrfs_set_extent_bit: <FSID>: io_tree=IO_TREE ino=1 root=1 start=22048768 len=4096 set_bits=LOCKED
  btrfs_clear_extent_bit: <FSID>: io_tree=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: <FSID>: io_tree=TRANS_DIRTY_PAGES ino=1 root=1 start=30425088 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: <FSID>: 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: <FSID>: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30523392 len=16384 set_bits=DIRTY
  btrfs_set_extent_bit: <FSID>: io_tree=FREED_EXTENTS0 ino=0 root=0 start=30556160 len=16384 set_bits=DIRTY
  ^^^ 2 old tree blocks get pinned down

There is one point 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.


Qu Wenruo (3):
  btrfs: Introduce fs_info for extent_io_tree
  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             |   9 +-
 fs/btrfs/extent_io.h             |  22 ++++-
 fs/btrfs/inode.c                 |   6 +-
 fs/btrfs/relocation.c            |   9 +-
 fs/btrfs/tests/btrfs-tests.c     |   6 +-
 fs/btrfs/tests/extent-io-tests.c |   2 +-
 fs/btrfs/transaction.c           |   4 +-
 include/trace/events/btrfs.h     | 159 ++++++++++++++++++++++++++++++-
 9 files changed, 210 insertions(+), 19 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-03-11 15:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01  2:47 [PATCH v2 0/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
2019-03-01  2:47 ` [PATCH v2 1/3] btrfs: Introduce fs_info " Qu Wenruo
2019-03-01  2:47 ` [PATCH v2 2/3] btrfs: Introduce extent_io_tree::owner to distinguish different io_trees Qu Wenruo
2019-03-07 17:10   ` David Sterba
2019-03-11 15:20   ` David Sterba
2019-03-01  2:48 ` [PATCH v2 3/3] btrfs: trace: Add trace events for extent_io_tree Qu Wenruo
2019-03-07 17:14   ` David Sterba
2019-03-07 16:32 ` [PATCH v2 0/3] " David Sterba
2019-03-08  0:41   ` Qu Wenruo
  -- strict thread matches above, loose matches on Subject: below --
2019-03-01  2:45 Qu Wenruo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.