[PATCH 0/5] btrfs: tag as unlikely several unexpected critical errors

[PATCH 0/5] btrfs: tag as unlikely several unexpected critical errors

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Several critical error checks are never expected to be hit, so tag them
as unlikely. Details in the changelogs.

Filipe Manana (5):
  btrfs: store and use node size in local variable in check_eb_alignment()
  btrfs: mark extent buffer alignment checks as unlikely
  btrfs: mark as unlikely not uptodate check in read_extent_buffer_pages()
  btrfs: mark as unlikely not uptodate extent buffer checks when navigating btrees
  btrfs: mark leaf space and overflow checks as unlikely on insert and extension

 fs/btrfs/ctree.c     | 22 +++++++++++-----------
 fs/btrfs/extent_io.c | 21 +++++++++++----------
 2 files changed, 22 insertions(+), 21 deletions(-)

-- 
2.47.2

[PATCH 1/5] btrfs: store and use node size in local variable in check_eb_alignment()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Instead of dereferencing fs_info every time we need to access the node
size, store in a local variable to make the code less verbose and avoid
a line split too.

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

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index ca7174fa0240..681f4f2e4419 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3226,29 +3226,30 @@ static struct extent_buffer *grab_extent_buffer(struct btrfs_fs_info *fs_info,
  */
 static bool check_eb_alignment(struct btrfs_fs_info *fs_info, u64 start)
 {
+	const u32 nodesize = fs_info->nodesize;
+
 	if (!IS_ALIGNED(start, fs_info->sectorsize)) {
 		btrfs_err(fs_info, "bad tree block start %llu", start);
 		return true;
 	}
 
-	if (fs_info->nodesize < PAGE_SIZE && !IS_ALIGNED(start, fs_info->nodesize)) {
+	if (nodesize < PAGE_SIZE && !IS_ALIGNED(start, nodesize)) {
 		btrfs_err(fs_info,
 		"tree block is not nodesize aligned, start %llu nodesize %u",
-			  start, fs_info->nodesize);
+			  start, nodesize);
 		return true;
 	}
-	if (fs_info->nodesize >= PAGE_SIZE &&
-	    !PAGE_ALIGNED(start)) {
+	if (nodesize >= PAGE_SIZE && !PAGE_ALIGNED(start)) {
 		btrfs_err(fs_info,
 		"tree block is not page aligned, start %llu nodesize %u",
-			  start, fs_info->nodesize);
+			  start, nodesize);
 		return true;
 	}
-	if (!IS_ALIGNED(start, fs_info->nodesize) &&
+	if (!IS_ALIGNED(start, nodesize) &&
 	    !test_and_set_bit(BTRFS_FS_UNALIGNED_TREE_BLOCK, &fs_info->flags)) {
 		btrfs_warn(fs_info,
 "tree block not nodesize aligned, start %llu nodesize %u, can be resolved by a full metadata balance",
-			      start, fs_info->nodesize);
+			      start, nodesize);
 	}
 	return false;
 }
-- 
2.47.2

[PATCH 2/5] btrfs: mark extent buffer alignment checks as unlikely

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We are not expecting to ever fail the extent buffer alignment checks, so
mark them as unlikely to allow the compiler to potentially generate more
optimized code.

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

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 681f4f2e4419..5f0cce1bb7c6 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3228,25 +3228,25 @@ static bool check_eb_alignment(struct btrfs_fs_info *fs_info, u64 start)
 {
 	const u32 nodesize = fs_info->nodesize;
 
-	if (!IS_ALIGNED(start, fs_info->sectorsize)) {
+	if (unlikely(!IS_ALIGNED(start, fs_info->sectorsize))) {
 		btrfs_err(fs_info, "bad tree block start %llu", start);
 		return true;
 	}
 
-	if (nodesize < PAGE_SIZE && !IS_ALIGNED(start, nodesize)) {
+	if (unlikely(nodesize < PAGE_SIZE && !IS_ALIGNED(start, nodesize))) {
 		btrfs_err(fs_info,
 		"tree block is not nodesize aligned, start %llu nodesize %u",
 			  start, nodesize);
 		return true;
 	}
-	if (nodesize >= PAGE_SIZE && !PAGE_ALIGNED(start)) {
+	if (unlikely(nodesize >= PAGE_SIZE && !PAGE_ALIGNED(start))) {
 		btrfs_err(fs_info,
 		"tree block is not page aligned, start %llu nodesize %u",
 			  start, nodesize);
 		return true;
 	}
-	if (!IS_ALIGNED(start, nodesize) &&
-	    !test_and_set_bit(BTRFS_FS_UNALIGNED_TREE_BLOCK, &fs_info->flags)) {
+	if (unlikely(!IS_ALIGNED(start, nodesize) &&
+		     !test_and_set_bit(BTRFS_FS_UNALIGNED_TREE_BLOCK, &fs_info->flags))) {
 		btrfs_warn(fs_info,
 "tree block not nodesize aligned, start %llu nodesize %u, can be resolved by a full metadata balance",
 			      start, nodesize);
-- 
2.47.2

[PATCH 3/5] btrfs: mark as unlikely not uptodate check in read_extent_buffer_pages()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

After we waited for the bios that read an extent buffer's extent from
disk, we expect to have the extent buffer up to date (no errors happened),
so mark the check for not being up to date as unlikely and allow the
compiler to potentially generate better code.

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

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 5f0cce1bb7c6..704fd922c18b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3868,7 +3868,7 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int mirror_num,
 		return ret;
 
 	wait_on_bit_io(&eb->bflags, EXTENT_BUFFER_READING, TASK_UNINTERRUPTIBLE);
-	if (!test_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags))
+	if (unlikely(!test_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags)))
 		return -EIO;
 	return 0;
 }
-- 
2.47.2

[PATCH 4/5] btrfs: mark as unlikely not uptodate extent buffer checks when navigating btrees

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We expect that after attempting to read an extent buffer we had no errors
therefore the extent buffer is up to date, so mark the checks for a not up
to date extent buffer as unlikely and allow the compiler to pontentially
generate better code.

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

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6f9465d4ce54..f6d2a4a4b9eb 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -844,7 +844,7 @@ struct extent_buffer *btrfs_read_node_slot(struct extent_buffer *parent,
 			     &check);
 	if (IS_ERR(eb))
 		return eb;
-	if (!extent_buffer_uptodate(eb)) {
+	if (unlikely(!extent_buffer_uptodate(eb))) {
 		free_extent_buffer(eb);
 		return ERR_PTR(-EIO);
 	}
@@ -1571,7 +1571,7 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,
 	 * and give up so that our caller doesn't loop forever
 	 * on our EAGAINs.
 	 */
-	if (!extent_buffer_uptodate(tmp)) {
+	if (unlikely(!extent_buffer_uptodate(tmp))) {
 		ret = -EIO;
 		goto out;
 	}
@@ -1752,7 +1752,7 @@ static struct extent_buffer *btrfs_search_slot_get_root(struct btrfs_root *root,
 	 * The root may have failed to write out at some point, and thus is no
 	 * longer valid, return an error in this case.
 	 */
-	if (!extent_buffer_uptodate(b)) {
+	if (unlikely(!extent_buffer_uptodate(b))) {
 		if (root_lock)
 			btrfs_tree_unlock_rw(b, root_lock);
 		free_extent_buffer(b);
@@ -2260,7 +2260,7 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
 
 again:
 	b = btrfs_get_old_root(root, time_seq);
-	if (!b) {
+	if (unlikely(!b)) {
 		ret = -EIO;
 		goto done;
 	}
-- 
2.47.2

[PATCH 5/5] btrfs: mark leaf space and overflow checks as unlikely on insert and extension

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We have several sanity checks when inserting or extending items in a btree
that verify we didn't overflow the leaf or access a slot beyond the last
one. These are cases that are never expected to be hit so mark them as
unlikely, allowing the compiler to potentially generate better code.

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

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index f6d2a4a4b9eb..dc185322362b 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3086,7 +3086,7 @@ int btrfs_leaf_free_space(const struct extent_buffer *leaf)
 	int ret;
 
 	ret = BTRFS_LEAF_DATA_SIZE(fs_info) - leaf_space_used(leaf, 0, nritems);
-	if (ret < 0) {
+	if (unlikely(ret < 0)) {
 		btrfs_crit(fs_info,
 			   "leaf free space ret %d, leaf data size %lu, used %d nritems %d",
 			   ret,
@@ -4075,7 +4075,7 @@ void btrfs_truncate_item(struct btrfs_trans_handle *trans,
 	btrfs_set_item_size(leaf, slot, new_size);
 	btrfs_mark_buffer_dirty(trans, leaf);
 
-	if (btrfs_leaf_free_space(leaf) < 0) {
+	if (unlikely(btrfs_leaf_free_space(leaf) < 0)) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
@@ -4108,7 +4108,7 @@ void btrfs_extend_item(struct btrfs_trans_handle *trans,
 	old_data = btrfs_item_data_end(leaf, slot);
 
 	BUG_ON(slot < 0);
-	if (slot >= nritems) {
+	if (unlikely(slot >= nritems)) {
 		btrfs_print_leaf(leaf);
 		btrfs_crit(leaf->fs_info, "slot %d too large, nritems %d",
 			   slot, nritems);
@@ -4135,7 +4135,7 @@ void btrfs_extend_item(struct btrfs_trans_handle *trans,
 	btrfs_set_item_size(leaf, slot, old_size + data_size);
 	btrfs_mark_buffer_dirty(trans, leaf);
 
-	if (btrfs_leaf_free_space(leaf) < 0) {
+	if (unlikely(btrfs_leaf_free_space(leaf) < 0)) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
@@ -4183,7 +4183,7 @@ static void setup_items_for_insert(struct btrfs_trans_handle *trans,
 	data_end = leaf_data_end(leaf);
 	total_size = batch->total_data_size + (batch->nr * sizeof(struct btrfs_item));
 
-	if (btrfs_leaf_free_space(leaf) < total_size) {
+	if (unlikely(btrfs_leaf_free_space(leaf) < total_size)) {
 		btrfs_print_leaf(leaf);
 		btrfs_crit(fs_info, "not enough freespace need %u have %d",
 			   total_size, btrfs_leaf_free_space(leaf));
@@ -4193,7 +4193,7 @@ static void setup_items_for_insert(struct btrfs_trans_handle *trans,
 	if (slot != nritems) {
 		unsigned int old_data = btrfs_item_data_end(leaf, slot);
 
-		if (old_data < data_end) {
+		if (unlikely(old_data < data_end)) {
 			btrfs_print_leaf(leaf);
 			btrfs_crit(fs_info,
 		"item at slot %d with data offset %u beyond data end of leaf %u",
@@ -4232,7 +4232,7 @@ static void setup_items_for_insert(struct btrfs_trans_handle *trans,
 	btrfs_set_header_nritems(leaf, nritems + batch->nr);
 	btrfs_mark_buffer_dirty(trans, leaf);
 
-	if (btrfs_leaf_free_space(leaf) < 0) {
+	if (unlikely(btrfs_leaf_free_space(leaf) < 0)) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
-- 
2.47.2

Re: [PATCH 0/5] btrfs: tag as unlikely several unexpected critical errors

[Qu Wenruo]

在 2025/9/17 17:22, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Several critical error checks are never expected to be hit, so tag them
> as unlikely. Details in the changelogs.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> 
> Filipe Manana (5):
>    btrfs: store and use node size in local variable in check_eb_alignment()
>    btrfs: mark extent buffer alignment checks as unlikely
>    btrfs: mark as unlikely not uptodate check in read_extent_buffer_pages()
>    btrfs: mark as unlikely not uptodate extent buffer checks when navigating btrees
>    btrfs: mark leaf space and overflow checks as unlikely on insert and extension
> 
>   fs/btrfs/ctree.c     | 22 +++++++++++-----------
>   fs/btrfs/extent_io.c | 21 +++++++++++----------
>   2 files changed, 22 insertions(+), 21 deletions(-)
> 

Re: [PATCH 0/5] btrfs: tag as unlikely several unexpected critical errors

[David Sterba]

On Wed, Sep 17, 2025 at 08:52:37AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Several critical error checks are never expected to be hit, so tag them
> as unlikely. Details in the changelogs.
> 
> Filipe Manana (5):
>   btrfs: store and use node size in local variable in check_eb_alignment()
>   btrfs: mark extent buffer alignment checks as unlikely
>   btrfs: mark as unlikely not uptodate check in read_extent_buffer_pages()
>   btrfs: mark as unlikely not uptodate extent buffer checks when navigating btrees
>   btrfs: mark leaf space and overflow checks as unlikely on insert and extension

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

Alternatively, as this is an obvious pattern the annotations can be done
in bigger batches.  On a test build with all EIO, EUCLEAN and
transaction abort branches annotated as unlikely the difference in
object size is <200 bytes. The asm code for error handling is reorded as
expected.

The reason for doing it in smaller patches is to avoid endless conflicts
with new patches or backports but as this would be an one time change
maybe we can do it now before the 6.18 freeze. While your changelogs
explaining each case are great I think we can spare ourselves of that,
it's really obvious and too repetitive.

I'll send the EIO/EUCLEAN/abort annotations for RFC.