[PATCH 00/11] btrfs: some optimizations and cleanups for io trees

[PATCH 00/11] btrfs: some optimizations and cleanups for io trees

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

An assorted set of optimizations and cleanups for io trees. The last patch
contains some benchmark results after applying the whole patchset.

Filipe Manana (11):
  btrfs: optimize clearing all bits from the last extent record in an io tree
  btrfs: turn extent_io_tree_panic() into a macro for better error reporting
  btrfs: tag as unlikely branches that call extent_io_tree_panic()
  btrfs: make add_extent_changeset() only return errors or success
  btrfs: use extent_io_tree_panic() instead of BUG_ON()
  btrfs: change last argument of add_extent_changeset() to boolean
  btrfs: remove wake parameter from clear_state_bit()
  btrfs: avoid unnecessary wake ups on io trees when there are no waiters
  btrfs: free cached state outside critical section in wait_extent_bit()
  btrfs: panic instead of warn when splitting extent state not in the tree
  btrfs: optimize clearing all bits from first extent record in an io tree

 fs/btrfs/extent-io-tree.c | 179 ++++++++++++++++++++++++++++----------
 1 file changed, 135 insertions(+), 44 deletions(-)

-- 
2.47.2

[PATCH 01/11] btrfs: optimize clearing all bits from the last extent record in an io tree

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When we are clearing all the bits from the last record that contains the
target range (i.e. the record starts before our target range and ends
beyond it), we are doing a lot of unnecessary work:

1) Allocating a prealloc state if we don't have one already;

2) Adjust that last record's start offset to the end of our range and
   make the prealloc state have a range going from the original start
   offset of that last record to the end offset of our target range and
   the same bits as the last record. Then we insert the prealloc extent
   in the rbtree - this is done in split_state();

3) Remove our prealloc state from the rbtree since all the bits were
   cleared - this is done in clear_state_bit().

This is only wasting time when we can simply trim the last record so
that it's start offset is adjust to the end of the target range. So
optimize for that case and avoid the prealloc state allocation, insertion
and deletion from the rbtree.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index d0dd50f7d279..4ba916cb27ac 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -724,6 +724,45 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 	 * We need to split the extent, and clear the bit on the first half.
 	 */
 	if (state->start <= end && state->end > end) {
+		const u32 bits_to_clear = bits & ~EXTENT_CTLBITS;
+
+		/*
+		 * If all bits are cleared, there's no point in allocating or
+		 * using the prealloc extent, split the state record, insert the
+		 * prealloc record and then remove it. We can just adjust the
+		 * start offset of the current state and avoid all that.
+		 */
+		if ((state->state & ~bits_to_clear) == 0) {
+			const u64 orig_end = state->end;
+
+			if (tree->owner == IO_TREE_INODE_IO)
+				btrfs_split_delalloc_extent(tree->inode, state, end + 1);
+
+			/*
+			 * Temporarily adjust the end offset to match the
+			 * removed subrange to update the changeset.
+			 */
+			state->end = end;
+
+			ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
+			if (unlikely(ret < 0)) {
+				extent_io_tree_panic(tree, state,
+						     "add_extent_changeset", ret);
+				goto out;
+			}
+			ret = 0;
+
+			if (tree->owner == IO_TREE_INODE_IO)
+				btrfs_clear_delalloc_extent(tree->inode, state, bits);
+
+			state->start = end + 1;
+			state->end = orig_end;
+
+			if (wake)
+				wake_up(&state->wq);
+			goto out;
+		}
+
 		prealloc = alloc_extent_state_atomic(prealloc);
 		if (!prealloc)
 			goto search_again;
-- 
2.47.2

[PATCH 02/11] btrfs: turn extent_io_tree_panic() into a macro for better error reporting

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When extent_io_tree_panic() is called we get a stace trace that is not
very useful since the error message reports the location inside the
extent_io_tree_panic() function and not in the caller of the function.

Example:

  [ 7830.424291] BTRFS critical (device sdb): panic in extent_io_tree_panic:334: extent io tree error on add_extent_changeset state start 4083712 end 4112383 (errno=1 unknown)
  [ 7830.426816] ------------[ cut here ]------------
  [ 7830.427581] kernel BUG at fs/btrfs/extent-io-tree.c:334!
  [ 7830.428495] Oops: invalid opcode: 0000 [#1] SMP PTI
  [ 7830.429318] CPU: 5 UID: 0 PID: 1451600 Comm: fsstress Not tainted 7.0.0-rc2-btrfs-next-227+ #1 PREEMPT(full)
  [ 7830.430899] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
  [ 7830.432771] RIP: 0010:extent_io_tree_panic+0x41/0x43 [btrfs]
  [ 7830.433815] Code: 75 0a 48 8b (...)
  [ 7830.436849] RSP: 0018:ffffd2334f4a3b68 EFLAGS: 00010246
  [ 7830.437668] RAX: 0000000000000000 RBX: 00000000003ebfff RCX: 0000000000000000
  [ 7830.438801] RDX: ffffffffc08d4368 RSI: ffffffffbb6ce475 RDI: ffff896501d6b780
  [ 7830.439671] RBP: 0000000000001000 R08: 0000000000000000 R09: 00000000ffefffff
  [ 7830.440575] R10: 0000000000000000 R11: 0000000000000003 R12: 0000000000000000
  [ 7830.441458] R13: ffff896547374c08 R14: 00000000003effff R15: ffff896547374c08
  [ 7830.442333] FS:  00007f3e252af0c0(0000) GS:ffff896c6185d000(0000) knlGS:0000000000000000
  [ 7830.443326] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [ 7830.444047] CR2: 00007f3e252ad000 CR3: 0000000113b0a004 CR4: 0000000000370ef0
  [ 7830.444905] Call Trace:
  [ 7830.445229]  <TASK>
  [ 7830.445557]  btrfs_clear_extent_bit_changeset.cold+0x43/0x80 [btrfs]
  [ 7830.446543]  btrfs_clear_record_extent_bits+0x19/0x20 [btrfs]
  [ 7830.447308]  qgroup_free_reserved_data+0xf9/0x170 [btrfs]
  [ 7830.448040]  btrfs_buffered_write+0x368/0x8e0 [btrfs]
  [ 7830.448707]  btrfs_direct_write+0x1a5/0x480 [btrfs]
  [ 7830.449396]  btrfs_do_write_iter+0x18c/0x210 [btrfs]
  [ 7830.450167]  vfs_write+0x21f/0x450
  [ 7830.450662]  ksys_write+0x5f/0xd0
  [ 7830.451092]  do_syscall_64+0xe9/0xf20
  [ 7830.451610]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

Change extent_io_tree_panic() to a macro so that we get a report that
gives the exact place where the error happens.

Example after this change:

  [63677.406061] BTRFS critical (device sdc): panic in btrfs_clear_extent_bit_changeset:744: extent io tree error on add_extent_changeset state start 1818624 end 1830911 (errno=1 unknown)
  [63677.410055] ------------[ cut here ]------------
  [63677.410910] kernel BUG at fs/btrfs/extent-io-tree.c:744!
  [63677.411918] Oops: invalid opcode: 0000 [#1] SMP PTI
  [63677.413032] CPU: 0 UID: 0 PID: 13028 Comm: fsstress Not tainted 7.0.0-rc2-btrfs-next-227+ #1 PREEMPT(full)
  [63677.415139] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
  [63677.417283] RIP: 0010:btrfs_clear_extent_bit_changeset.cold+0xcd/0x10c [btrfs]
  [63677.418676] Code: 8b 37 48 8b (...)
  [63677.421917] RSP: 0018:ffffd2290a417b30 EFLAGS: 00010246
  [63677.422824] RAX: 0000000000000000 RBX: 00000000001befff RCX: 0000000000000000
  [63677.424320] RDX: ffffffffc0970348 RSI: ffffffffa92ce475 RDI: ffff8897ded9dc80
  [63677.429772] RBP: 0000000000001000 R08: 0000000000000000 R09: 00000000ffefffff
  [63677.430787] R10: 0000000000000000 R11: 0000000000000003 R12: 0000000000000000
  [63677.431818] R13: ffff8897966655d8 R14: 00000000001bffff R15: ffff8897966655d8
  [63677.432764] FS:  00007f5c074c50c0(0000) GS:ffff889ef3b1d000(0000) knlGS:0000000000000000
  [63677.433940] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [63677.434787] CR2: 00007f5c074c3000 CR3: 000000014b9de002 CR4: 0000000000370ef0
  [63677.435960] Call Trace:
  [63677.436432]  <TASK>
  [63677.436838]  btrfs_clear_record_extent_bits+0x19/0x20 [btrfs]
  [63677.437980]  qgroup_free_reserved_data+0xf9/0x170 [btrfs]
  [63677.439070]  btrfs_buffered_write+0x368/0x8e0 [btrfs]
  [63677.439889]  btrfs_do_write_iter+0x1a8/0x210 [btrfs]
  [63677.441460]  do_iter_readv_writev+0x145/0x240
  [63677.446309]  vfs_writev+0x120/0x3b0
  [63677.446878]  ? __do_sys_newfstat+0x33/0x60
  [63677.447759]  ? do_pwritev+0x8a/0xd0
  [63677.449119]  do_pwritev+0x8a/0xd0
  [63677.452342]  do_syscall_64+0xe9/0xf20
  [63677.452961]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 4ba916cb27ac..68e48b493950 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -326,15 +326,10 @@ static inline struct extent_state *tree_search(struct extent_io_tree *tree, u64
 	return tree_search_for_insert(tree, offset, NULL, NULL);
 }
 
-static void __cold extent_io_tree_panic(const struct extent_io_tree *tree,
-					const struct extent_state *state,
-					const char *opname,
-					int err)
-{
-	btrfs_panic(btrfs_extent_io_tree_to_fs_info(tree), err,
-		    "extent io tree error on %s state start %llu end %llu",
-		    opname, state->start, state->end);
-}
+#define extent_io_tree_panic(tree, state, opname, err)                      \
+	btrfs_panic(btrfs_extent_io_tree_to_fs_info((tree)), (err),         \
+		    "extent io tree error on %s state start %llu end %llu", \
+		    (opname), (state)->start, (state)->end)
 
 static void merge_prev_state(struct extent_io_tree *tree, struct extent_state *state)
 {
-- 
2.47.2

[PATCH 03/11] btrfs: tag as unlikely branches that call extent_io_tree_panic()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

It's unexpected to ever call extent_io_tree_panic() so surround with
'unlikely' every if statement condition that leads to it, making it
explicit to a reader and to hint the compiler to potentially generate
better code.

On x86_64, using gcc 14.2.0-19 from Debian, this resulted in a slightly
decrease of the btrfs module's text size.

Before:

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  1999832	 174320	  15592	2189744	 2169b0	fs/btrfs/btrfs.ko

After:

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  1999768	 174320	  15592	2189680	 216970	fs/btrfs/btrfs.ko

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 68e48b493950..93dca9199249 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -696,7 +696,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 			goto search_again;
 		ret = split_state(tree, state, prealloc, start);
 		prealloc = NULL;
-		if (ret) {
+		if (unlikely(ret)) {
 			extent_io_tree_panic(tree, state, "split", ret);
 			goto out;
 		}
@@ -762,7 +762,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 		if (!prealloc)
 			goto search_again;
 		ret = split_state(tree, state, prealloc, end + 1);
-		if (ret) {
+		if (unlikely(ret)) {
 			extent_io_tree_panic(tree, state, "split", ret);
 			prealloc = NULL;
 			goto out;
@@ -1203,7 +1203,7 @@ static int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		if (!prealloc)
 			goto search_again;
 		ret = split_state(tree, state, prealloc, start);
-		if (ret)
+		if (unlikely(ret))
 			extent_io_tree_panic(tree, state, "split", ret);
 
 		prealloc = NULL;
@@ -1293,7 +1293,7 @@ static int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		if (!prealloc)
 			goto search_again;
 		ret = split_state(tree, state, prealloc, end + 1);
-		if (ret) {
+		if (unlikely(ret)) {
 			extent_io_tree_panic(tree, state, "split", ret);
 			prealloc = NULL;
 			goto out;
@@ -1448,7 +1448,7 @@ int btrfs_convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		}
 		ret = split_state(tree, state, prealloc, start);
 		prealloc = NULL;
-		if (ret) {
+		if (unlikely(ret)) {
 			extent_io_tree_panic(tree, state, "split", ret);
 			goto out;
 		}
@@ -1532,7 +1532,7 @@ int btrfs_convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		}
 
 		ret = split_state(tree, state, prealloc, end + 1);
-		if (ret) {
+		if (unlikely(ret)) {
 			extent_io_tree_panic(tree, state, "split", ret);
 			prealloc = NULL;
 			goto out;
-- 
2.47.2

[PATCH 04/11] btrfs: make add_extent_changeset() only return errors or success

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Currently add_extent_changeset() always returns the return value from its
call to ulist_add(), which can return an error, 0 or 1. There are no
callers that care about the difference between 0 and 1 and all except one
of them, check for negative values and ignore other values, but there is
another caller (btrfs_clear_extent_bit_changeset()) that must set its
'ret' variable to 0 after calling add_extent_changeset(), so that it
does not return an unexpected value of 1 to its caller.

So change add_extent_changeset() to only return errors or 0, avoiding
that caller (and any future callers) from having to deal with a return
value of 1.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 93dca9199249..5972fe28716e 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -187,6 +187,8 @@ static int add_extent_changeset(struct extent_state *state, u32 bits,
 				 struct extent_changeset *changeset,
 				 int set)
 {
+	int ret;
+
 	if (!changeset)
 		return 0;
 	if (set && (state->state & bits) == bits)
@@ -195,7 +197,10 @@ static int add_extent_changeset(struct extent_state *state, u32 bits,
 		return 0;
 	changeset->bytes_changed += state->end - state->start + 1;
 
-	return ulist_add(&changeset->range_changed, state->start, state->end, GFP_ATOMIC);
+	ret = ulist_add(&changeset->range_changed, state->start, state->end, GFP_ATOMIC);
+	if (ret < 0)
+		return ret;
+	return 0;
 }
 
 static inline struct extent_state *next_state(struct extent_state *state)
@@ -745,7 +750,6 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 						     "add_extent_changeset", ret);
 				goto out;
 			}
-			ret = 0;
 
 			if (tree->owner == IO_TREE_INODE_IO)
 				btrfs_clear_delalloc_extent(tree->inode, state, bits);
-- 
2.47.2

[PATCH 05/11] btrfs: use extent_io_tree_panic() instead of BUG_ON()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There's no need to call BUG_ON(), instead call extent_io_tree_panic(),
which also calls BUG(), but it prints an additional error message with
some useful information before hitting BUG().

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 5972fe28716e..c2ede01f1762 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -395,7 +395,8 @@ static void set_state_bits(struct extent_io_tree *tree,
 		btrfs_set_delalloc_extent(tree->inode, state, bits);
 
 	ret = add_extent_changeset(state, bits_to_set, changeset, 1);
-	BUG_ON(ret < 0);
+	if (unlikely(ret))
+		extent_io_tree_panic(tree, state, "add_extent_changeset", ret);
 	state->state |= bits_to_set;
 }
 
@@ -567,7 +568,8 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 		btrfs_clear_delalloc_extent(tree->inode, state, bits);
 
 	ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
-	BUG_ON(ret < 0);
+	if (unlikely(ret))
+		extent_io_tree_panic(tree, state, "add_extent_changeset", ret);
 	state->state &= ~bits_to_clear;
 	if (wake)
 		wake_up(&state->wq);
-- 
2.47.2

[PATCH 06/11] btrfs: change last argument of add_extent_changeset() to boolean

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

The argument is used as a boolean but it's defined as an integer.
Switch it to a boolean for better readability.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index c2ede01f1762..a99e67585d64 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -185,7 +185,7 @@ void btrfs_free_extent_state(struct extent_state *state)
 
 static int add_extent_changeset(struct extent_state *state, u32 bits,
 				 struct extent_changeset *changeset,
-				 int set)
+				 bool set)
 {
 	int ret;
 
@@ -394,7 +394,7 @@ static void set_state_bits(struct extent_io_tree *tree,
 	if (tree->owner == IO_TREE_INODE_IO)
 		btrfs_set_delalloc_extent(tree->inode, state, bits);
 
-	ret = add_extent_changeset(state, bits_to_set, changeset, 1);
+	ret = add_extent_changeset(state, bits_to_set, changeset, true);
 	if (unlikely(ret))
 		extent_io_tree_panic(tree, state, "add_extent_changeset", ret);
 	state->state |= bits_to_set;
@@ -567,7 +567,7 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	if (tree->owner == IO_TREE_INODE_IO)
 		btrfs_clear_delalloc_extent(tree->inode, state, bits);
 
-	ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
+	ret = add_extent_changeset(state, bits_to_clear, changeset, false);
 	if (unlikely(ret))
 		extent_io_tree_panic(tree, state, "add_extent_changeset", ret);
 	state->state &= ~bits_to_clear;
@@ -746,7 +746,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 			 */
 			state->end = end;
 
-			ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
+			ret = add_extent_changeset(state, bits_to_clear, changeset, false);
 			if (unlikely(ret < 0)) {
 				extent_io_tree_panic(tree, state,
 						     "add_extent_changeset", ret);
-- 
2.47.2

[PATCH 07/11] btrfs: remove wake parameter from clear_state_bit()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There's no need to pass the 'wake' parameter, we can determine if we have
to wake up waiters by checking if EXTENT_LOCK_BITS is set in the bits to
clear. So simplify things and remove the parameter.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index a99e67585d64..5ffb99c258e9 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -550,14 +550,14 @@ static inline struct extent_state *next_search_state(struct extent_state *state,
 
 /*
  * Utility function to clear some bits in an extent state struct.  It will
- * optionally wake up anyone waiting on this state (wake == 1).
+ * optionally wake up anyone waiting on this state.
  *
  * If no bits are set on the state struct after clearing things, the
  * struct is freed and removed from the tree
  */
 static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 					    struct extent_state *state,
-					    u32 bits, int wake, u64 end,
+					    u32 bits, u64 end,
 					    struct extent_changeset *changeset)
 {
 	struct extent_state *next;
@@ -571,7 +571,7 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	if (unlikely(ret))
 		extent_io_tree_panic(tree, state, "add_extent_changeset", ret);
 	state->state &= ~bits_to_clear;
-	if (wake)
+	if (bits & EXTENT_LOCK_BITS)
 		wake_up(&state->wq);
 	if (state->state == 0) {
 		next = next_search_state(state, end);
@@ -708,8 +708,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 			goto out;
 		}
 		if (state->end <= end) {
-			state = clear_state_bit(tree, state, bits, wake, end,
-						changeset);
+			state = clear_state_bit(tree, state, bits, end, changeset);
 			goto next;
 		}
 		if (need_resched())
@@ -777,13 +776,13 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 		if (wake)
 			wake_up(&state->wq);
 
-		clear_state_bit(tree, prealloc, bits, wake, end, changeset);
+		clear_state_bit(tree, prealloc, bits, end, changeset);
 
 		prealloc = NULL;
 		goto out;
 	}
 
-	state = clear_state_bit(tree, state, bits, wake, end, changeset);
+	state = clear_state_bit(tree, state, bits, end, changeset);
 next:
 	if (last_end >= end)
 		goto out;
@@ -1422,7 +1421,7 @@ int btrfs_convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	if (state->start == start && state->end <= end) {
 		set_state_bits(tree, state, bits, NULL);
 		cache_state(state, cached_state);
-		state = clear_state_bit(tree, state, clear_bits, 0, end, NULL);
+		state = clear_state_bit(tree, state, clear_bits, end, NULL);
 		if (last_end >= end)
 			goto out;
 		start = last_end + 1;
@@ -1461,7 +1460,7 @@ int btrfs_convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		if (state->end <= end) {
 			set_state_bits(tree, state, bits, NULL);
 			cache_state(state, cached_state);
-			state = clear_state_bit(tree, state, clear_bits, 0, end, NULL);
+			state = clear_state_bit(tree, state, clear_bits, end, NULL);
 			if (last_end >= end)
 				goto out;
 			start = last_end + 1;
@@ -1546,7 +1545,7 @@ int btrfs_convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 
 		set_state_bits(tree, prealloc, bits, NULL);
 		cache_state(prealloc, cached_state);
-		clear_state_bit(tree, prealloc, clear_bits, 0, end, NULL);
+		clear_state_bit(tree, prealloc, clear_bits, end, NULL);
 		prealloc = NULL;
 		goto out;
 	}
-- 
2.47.2

[PATCH 08/11] btrfs: avoid unnecessary wake ups on io trees when there are no waiters

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Whenever clearing the extent lock bits of an extent state record, we
unconditionally call wake_up() on the state's waitqueue. Most of the
time there are no waiters on the queue so we are just wasting time calling
wake_up(), since that requires locking and unlocking the queue's spinlock,
disable and re-enable interrupts, function calls, and other minor overhead
while we are holding a critical section delimited by the extent io tree's
spinlock.

So call wake_up() only if there are waiters on an extent state's wait
queue.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 5ffb99c258e9..0d9437e72bd4 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -536,6 +536,24 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
 	return 0;
 }
 
+static inline void state_wake_up(struct extent_io_tree *tree,
+				 struct extent_state *state, u32 bits)
+{
+	lockdep_assert_held(&tree->lock);
+
+	if (!(bits & EXTENT_LOCK_BITS))
+		return;
+
+	/*
+	 * No memory barriers because the tree's lock is held while:
+	 *
+	 * 1) Adding waiters to the queue.
+	 * 2) Waking up waiters.
+	 * 3) Removing waiters from queue.
+	 */
+	cond_wake_up_nomb(&state->wq);
+}
+
 /*
  * Use this during tree iteration to avoid doing next node searches when it's
  * not needed (the current record ends at or after the target range's end).
@@ -571,8 +589,7 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	if (unlikely(ret))
 		extent_io_tree_panic(tree, state, "add_extent_changeset", ret);
 	state->state &= ~bits_to_clear;
-	if (bits & EXTENT_LOCK_BITS)
-		wake_up(&state->wq);
+	state_wake_up(tree, state, bits);
 	if (state->state == 0) {
 		next = next_search_state(state, end);
 		if (extent_state_in_tree(state)) {
@@ -618,7 +635,6 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 	u64 last_end;
 	int ret = 0;
 	bool clear;
-	bool wake;
 	const bool delete = (bits & EXTENT_CLEAR_ALL_BITS);
 	gfp_t mask;
 
@@ -632,7 +648,6 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 	if (bits & EXTENT_DELALLOC)
 		bits |= EXTENT_NORESERVE;
 
-	wake = (bits & EXTENT_LOCK_BITS);
 	clear = (bits & (EXTENT_LOCK_BITS | EXTENT_BOUNDARY));
 again:
 	if (!prealloc) {
@@ -758,8 +773,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 			state->start = end + 1;
 			state->end = orig_end;
 
-			if (wake)
-				wake_up(&state->wq);
+			state_wake_up(tree, state, bits);
 			goto out;
 		}
 
@@ -773,8 +787,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 			goto out;
 		}
 
-		if (wake)
-			wake_up(&state->wq);
+		state_wake_up(tree, state, bits);
 
 		clear_state_bit(tree, prealloc, bits, end, changeset);
 
-- 
2.47.2

[PATCH 09/11] btrfs: free cached state outside critical section in wait_extent_bit()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There's no need to free the cached extent state record while holding the
io tree's spinlock, it's just making the critical section longer than it
needs to be. So just do it after unlocking the io tree.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 0d9437e72bd4..29fcd08b9ca9 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -877,13 +877,13 @@ static void wait_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		}
 	}
 out:
+	spin_unlock(&tree->lock);
 	/* This state is no longer useful, clear it and free it up. */
 	if (cached_state && *cached_state) {
 		state = *cached_state;
 		*cached_state = NULL;
 		btrfs_free_extent_state(state);
 	}
-	spin_unlock(&tree->lock);
 }
 
 static void cache_state_if_flags(struct extent_state *state,
-- 
2.47.2

[PATCH 10/11] btrfs: panic instead of warn when splitting extent state not in the tree

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We are not expected ever to split an extent state record that is not in
the rbtree, as every record we pass to split_state() was found by
iterating the rbtree, so if that ever happens it means we are not holding
the extent io tree's spinlock or we have some memory corruption.

Instead of simply warning in case the extent state record passed to
split_state() is not in the rbtree, panic as this is a serious problem.
Also tag as unlikely the case where the record is not in the rbtree.

This also makes a tiny reduction the btrfs module's text size.

Before:

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  2000080	 174328	  15592	2190000	 216ab0	fs/btrfs/btrfs.ko

After:

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  2000064	 174328	  15592	2189984	 216aa0	fs/btrfs/btrfs.ko

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 29fcd08b9ca9..72ddd8d2e7a3 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -591,14 +591,13 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	state->state &= ~bits_to_clear;
 	state_wake_up(tree, state, bits);
 	if (state->state == 0) {
+		if (unlikely(!extent_state_in_tree(state)))
+			extent_io_tree_panic(tree, state, "extent_state_in_tree", -EUCLEAN);
+
 		next = next_search_state(state, end);
-		if (extent_state_in_tree(state)) {
-			rb_erase(&state->rb_node, &tree->state);
-			RB_CLEAR_NODE(&state->rb_node);
-			btrfs_free_extent_state(state);
-		} else {
-			WARN_ON(1);
-		}
+		rb_erase(&state->rb_node, &tree->state);
+		RB_CLEAR_NODE(&state->rb_node);
+		btrfs_free_extent_state(state);
 	} else {
 		merge_state(tree, state);
 		next = next_search_state(state, end);
-- 
2.47.2

[PATCH 11/11] btrfs: optimize clearing all bits from first extent record in an io tree

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When we are clearing all the bits from the first record that contains the
target range and that record ends at or before our target range but starts
before our target range, we are doing a lot of unnecessary work:

1) Allocating a prealloc state if we don't have one already;

2) Adjust that record's start offset to the start of our range and
   make the prealloc state have a range going from the original start
   offset of that first record to the start offset of our target range,
   and with the same bits as that first record. Then we insert the
   prealloc extent in the rbtree - this is done in split_state();

3) Remove our adjusted first state from the rbtree since all the bits
   were cleared - this is done in clear_state_bit().

This is only wasting time when we can simply trim that first record, so
that it represents the range from its start offset to the start offset of
our target range. So optimize for that case and avoid the prealloc state
allocation, insertion and deletion from the rbtree.

This patch is the last patch of a patchset comprised of the following
patches (in descending order):

  btrfs: optimize clearing all bits from first extent record in an io tree
  btrfs: panic instead of warn when splitting extent state not in the tree
  btrfs: free cached state outside critical section in wait_extent_bit()
  btrfs: avoid unnecessary wake ups on io trees when there are no waiters
  btrfs: remove wake parameter from clear_state_bit()
  btrfs: change last argument of add_extent_changeset() to boolean
  btrfs: use extent_io_tree_panic() instead of BUG_ON()
  btrfs: make add_extent_changeset() only return errors or success
  btrfs: tag as unlikely branches that call extent_io_tree_panic()
  btrfs: turn extent_io_tree_panic() into a macro for better error reporting
  btrfs: optimize clearing all bits from the last extent record in an io tree

The following fio script was used to measure performance before and after
applying all the patches:

  $ cat ./fio-io-uring-2.sh
  #!/bin/bash

  DEV=/dev/nullb0
  MNT=/mnt/nullb0
  MOUNT_OPTIONS="-o ssd"
  MKFS_OPTIONS=""

  if [ $# -ne 3 ]; then
      echo "Use $0 NUM_JOBS FILE_SIZE RUN_TIME"
      exit 1
  fi

  NUM_JOBS=$1
  FILE_SIZE=$2
  RUN_TIME=$3

  cat <<EOF > /tmp/fio-job.ini
  [io_uring_rw]
  rw=randwrite
  fsync=0
  fallocate=none
  group_reporting=1
  direct=1
  ioengine=io_uring
  fixedbufs=1
  iodepth=64
  bs=4K
  filesize=$FILE_SIZE
  runtime=$RUN_TIME
  time_based
  filename=foobar
  directory=$MNT
  numjobs=$NUM_JOBS
  thread
  EOF

  echo performance | \
      tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor

  echo
  echo "Using config:"
  echo
  cat /tmp/fio-job.ini
  echo

  umount $MNT &> /dev/null
  mkfs.btrfs -f $MKFS_OPTIONS $DEV &> /dev/null
  mount $MOUNT_OPTIONS $DEV $MNT

  fio /tmp/fio-job.ini

  umount $MNT

When running this script on a 12 cores machine using a 16G null block
device the results were the following:

Before patchset:

  $ ./fio-io-uring-2.sh 12 8G 60
  (...)
  WRITE: bw=74.8MiB/s (78.5MB/s), 74.8MiB/s-74.8MiB/s (78.5MB/s-78.5MB/s), io=4504MiB (4723MB), run=60197-60197msec

After patchset:

  $ ./fio-io-uring-2.sh 12 8G 60
  (...)
  WRITE: bw=82.2MiB/s (86.2MB/s), 82.2MiB/s-82.2MiB/s (86.2MB/s-86.2MB/s), io=4937MiB (5176MB), run=60027-60027msec

Also, using bpftrace to collect the duration (in nanoseconds) of all the
btrfs_clear_extent_bit_changeset() calls done during that fio test and
then making an histogram from that data, held the following results:

Before patchset:

  Count: 6304804
  Range:  0.000 - 7587172.000; Mean: 2011.308; Median: 1219.000; Stddev: 17117.533
  Percentiles:  90th: 1888.000; 95th: 2189.000; 99th: 16104.000
        0.000 -    8.098:        7 |
        8.098 -   40.385:       20 |
       40.385 -  187.254:      146 |
      187.254 -  855.347:   742048 #######
      855.347 - 3894.426:  5462542 #####################################################
     3894.426 - 17718.848:   41489 |
    17718.848 - 80604.558:   46085 |
    80604.558 - 366664.449:  11285 |
   366664.449 - 1667918.122:   961 |
  1667918.122 - 7587172.000:   113 |

After patchset:

  Count: 6282879
  Range:  0.000 - 6029290.000; Mean: 1896.482; Median: 1126.000; Stddev: 15276.691
  Percentiles:  90th: 1741.000; 95th: 2026.000; 99th: 15713.000
        0.000 - 60.014:         12 |
       60.014 - 217.984:        63 |
      217.984 - 784.949:    517515 #####
      784.949 - 2819.823:  5632335 #####################################################
     2819.823 - 10123.127:   55716 #
    10123.127 - 36335.184:   46034 |
    36335.184 - 130412.049:  25708 |
   130412.049 - 468060.350:   4824 |
   468060.350 - 1679903.189:   549 |
  1679903.189 - 6029290.000:    84 |

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 44 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 72ddd8d2e7a3..6ae7709cba23 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -635,6 +635,7 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 	int ret = 0;
 	bool clear;
 	const bool delete = (bits & EXTENT_CLEAR_ALL_BITS);
+	const u32 bits_to_clear = (bits & ~EXTENT_CTLBITS);
 	gfp_t mask;
 
 	set_gfp_mask_from_bits(&bits, &mask);
@@ -712,6 +713,47 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 	 */
 
 	if (state->start < start) {
+		/*
+		 * If all bits are cleared, there's no point in allocating or
+		 * using the prealloc extent, split the state record, insert the
+		 * prealloc record and then remove this record. We can just
+		 * adjust this record and move on to the next without adding or
+		 * removing anything to the tree.
+		 */
+		if (state->end <= end && (state->state & ~bits_to_clear) == 0) {
+			const u64 orig_start = state->start;
+
+			if (tree->owner == IO_TREE_INODE_IO)
+				btrfs_split_delalloc_extent(tree->inode, state, start);
+
+			/*
+			 * Temporarilly ajdust this state's range to match the
+			 * range for which we are clearing bits.
+			 */
+			state->start = start;
+
+			ret = add_extent_changeset(state, bits_to_clear, changeset, false);
+			if (unlikely(ret < 0)) {
+				extent_io_tree_panic(tree, state,
+						     "add_extent_changeset", ret);
+				goto out;
+			}
+
+			if (tree->owner == IO_TREE_INODE_IO)
+				btrfs_clear_delalloc_extent(tree->inode, state, bits);
+
+			/*
+			 * Now adjust the range to the section for which no bits
+			 * are cleared.
+			 */
+			state->start = orig_start;
+			state->end = start - 1;
+
+			state_wake_up(tree, state, bits);
+			state = next_search_state(state, end);
+			goto next;
+		}
+
 		prealloc = alloc_extent_state_atomic(prealloc);
 		if (!prealloc)
 			goto search_again;
@@ -739,8 +781,6 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 	 * We need to split the extent, and clear the bit on the first half.
 	 */
 	if (state->start <= end && state->end > end) {
-		const u32 bits_to_clear = bits & ~EXTENT_CTLBITS;
-
 		/*
 		 * If all bits are cleared, there's no point in allocating or
 		 * using the prealloc extent, split the state record, insert the
-- 
2.47.2

Re: [PATCH 00/11] btrfs: some optimizations and cleanups for io trees

[David Sterba]

On Mon, Mar 16, 2026 at 04:14:03PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> An assorted set of optimizations and cleanups for io trees. The last patch
> contains some benchmark results after applying the whole patchset.
> 
> Filipe Manana (11):
>   btrfs: optimize clearing all bits from the last extent record in an io tree
>   btrfs: turn extent_io_tree_panic() into a macro for better error reporting
>   btrfs: tag as unlikely branches that call extent_io_tree_panic()
>   btrfs: make add_extent_changeset() only return errors or success
>   btrfs: use extent_io_tree_panic() instead of BUG_ON()
>   btrfs: change last argument of add_extent_changeset() to boolean
>   btrfs: remove wake parameter from clear_state_bit()
>   btrfs: avoid unnecessary wake ups on io trees when there are no waiters
>   btrfs: free cached state outside critical section in wait_extent_bit()
>   btrfs: panic instead of warn when splitting extent state not in the tree
>   btrfs: optimize clearing all bits from first extent record in an io tree

Reviewed-by: David Sterba <dsterba@suse.com>