[PATCH 0/4] btrfs: replace BUG() and BUG_ON() with error handling

[PATCH 0/4] btrfs: replace BUG() and BUG_ON() with error handling

[Adarsh Das]

Replace BUG() and BUG_ON() calls in compression.c and extent-tree.c
with proper error handling so the kernel does not crash on unexpected
conditions. Also fix checkpatch warnings and errors found in both files.

Adarsh Das (4):
  btrfs: replace BUG() with error handling in compression.c
  btrfs: clean coding style errors and warnings in compression.c
  btrfs: replace BUG() and BUG_ON() with error handling in extent-tree.c
  btrfs: clean coding style errors in extent-tree.c

 fs/btrfs/compression.c | 66 ++++++++++++-------------------
 fs/btrfs/extent-tree.c | 89 +++++++++++++++++++++++++++++++-----------
 2 files changed, 91 insertions(+), 64 deletions(-)

-- 
2.53.0

[PATCH 1/4] btrfs: replace BUG() with error handling in compression.c

[Adarsh Das]

Replace BUG() calls with proper error handling. Where fs_info is
available, use btrfs_err() and return -EUCLEAN. Where fs_info is
not available, use WARN_ONCE() with the invalid type value so the
stack trace carries enough context for debugging.

Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
 fs/btrfs/compression.c | 42 ++++++++++++------------------------------
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 790518a8c803..29281aba925e 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -95,11 +95,9 @@ static int compression_decompress_bio(struct list_head *ws,
 	case BTRFS_COMPRESS_ZSTD: return zstd_decompress_bio(ws, cb);
 	case BTRFS_COMPRESS_NONE:
 	default:
-		/*
-		 * This can't happen, the type is validated several times
-		 * before we get here.
-		 */
-		BUG();
+		btrfs_err(cb_to_fs_info(cb), "invalid compression type %d",
+			  cb->compress_type);
+		return -EUCLEAN;
 	}
 }
 
@@ -116,11 +114,7 @@ static int compression_decompress(int type, struct list_head *ws,
 						dest_pgoff, srclen, destlen);
 	case BTRFS_COMPRESS_NONE:
 	default:
-		/*
-		 * This can't happen, the type is validated several times
-		 * before we get here.
-		 */
-		BUG();
+		return -EUCLEAN;
 	}
 }
 
@@ -703,11 +697,8 @@ static struct list_head *alloc_workspace(struct btrfs_fs_info *fs_info, int type
 	case BTRFS_COMPRESS_LZO:  return lzo_alloc_workspace(fs_info);
 	case BTRFS_COMPRESS_ZSTD: return zstd_alloc_workspace(fs_info, level);
 	default:
-		/*
-		 * This can't happen, the type is validated several times
-		 * before we get here.
-		 */
-		BUG();
+		btrfs_err(fs_info, "invalid compression type %d", type);
+		return ERR_PTR(-EUCLEAN);
 	}
 }
 
@@ -719,11 +710,8 @@ static void free_workspace(int type, struct list_head *ws)
 	case BTRFS_COMPRESS_LZO:  return lzo_free_workspace(ws);
 	case BTRFS_COMPRESS_ZSTD: return zstd_free_workspace(ws);
 	default:
-		/*
-		 * This can't happen, the type is validated several times
-		 * before we get here.
-		 */
-		BUG();
+		WARN_ONCE(1, "invalid compression type %d", type);
+		return;
 	}
 }
 
@@ -874,11 +862,8 @@ static struct list_head *get_workspace(struct btrfs_fs_info *fs_info, int type,
 	case BTRFS_COMPRESS_LZO:  return btrfs_get_workspace(fs_info, type, level);
 	case BTRFS_COMPRESS_ZSTD: return zstd_get_workspace(fs_info, level);
 	default:
-		/*
-		 * This can't happen, the type is validated several times
-		 * before we get here.
-		 */
-		BUG();
+		btrfs_err(fs_info, "invalid compression type %d", type);
+		return ERR_PTR(-EUCLEAN);
 	}
 }
 
@@ -925,11 +910,8 @@ static void put_workspace(struct btrfs_fs_info *fs_info, int type, struct list_h
 	case BTRFS_COMPRESS_LZO:  return btrfs_put_workspace(fs_info, type, ws);
 	case BTRFS_COMPRESS_ZSTD: return zstd_put_workspace(fs_info, ws);
 	default:
-		/*
-		 * This can't happen, the type is validated several times
-		 * before we get here.
-		 */
-		BUG();
+		btrfs_err(fs_info, "invalid compression type %d", type);
+		return;
 	}
 }
 
-- 
2.53.0

Re: [PATCH 1/4] btrfs: replace BUG() with error handling in compression.c

[Qu Wenruo]

在 2026/2/28 05:01, Adarsh Das 写道:
> Replace BUG() calls with proper error handling. Where fs_info is
> available, use btrfs_err() and return -EUCLEAN. Where fs_info is
> not available,

btrfs_err() can accept a NULL pointer as @fs_info.

[...]
>   
> @@ -874,11 +862,8 @@ static struct list_head *get_workspace(struct btrfs_fs_info *fs_info, int type,
>   	case BTRFS_COMPRESS_LZO:  return btrfs_get_workspace(fs_info, type, level);
>   	case BTRFS_COMPRESS_ZSTD: return zstd_get_workspace(fs_info, level);
>   	default:
> -		/*
> -		 * This can't happen, the type is validated several times
> -		 * before we get here.
> -		 */
> -		BUG();
> +		btrfs_err(fs_info, "invalid compression type %d", type);
> +		return ERR_PTR(-EUCLEAN);

Although for such unknown compression level case, the workspace user and 
put_workspace() are all erroring out correctly, it's not following the 
common pattern of checking the error first.

I'd prefer to just remove those default: branch, and add an ASSERT() 
checking the type is inside the BTRFS_NR_COMPRESS_TYPE before calling 
switch().

Just as the comment said, the @type has been verify too many times, we 
don't need to add a new error pattern where no one is checking.

Thanks,
Qu

>   	}
>   }
>   
> @@ -925,11 +910,8 @@ static void put_workspace(struct btrfs_fs_info *fs_info, int type, struct list_h
>   	case BTRFS_COMPRESS_LZO:  return btrfs_put_workspace(fs_info, type, ws);
>   	case BTRFS_COMPRESS_ZSTD: return zstd_put_workspace(fs_info, ws);
>   	default:
> -		/*
> -		 * This can't happen, the type is validated several times
> -		 * before we get here.
> -		 */
> -		BUG();
> +		btrfs_err(fs_info, "invalid compression type %d", type);
> +		return;
>   	}
>   }
>   

[PATCH 2/4] btrfs: clean coding style errors and warnings in compression.c

[Adarsh Das]

As the previous patch is making changes to compression.c, this patch
takes the oppurtunity to fix errors and warning in compression.c

Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
 fs/btrfs/compression.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 29281aba925e..6c3be3550442 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -36,9 +36,9 @@
 
 static struct bio_set btrfs_compressed_bioset;
 
-static const char* const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
+static const char * const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
 
-const char* btrfs_compress_type2str(enum btrfs_compression_type type)
+const char *btrfs_compress_type2str(enum btrfs_compression_type type)
 {
 	switch (type) {
 	case BTRFS_COMPRESS_ZLIB:
@@ -478,6 +478,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
 
 			if (zero_offset) {
 				int zeros;
+
 				zeros = folio_size(folio) - zero_offset;
 				folio_zero_range(folio, zero_offset, zeros);
 			}
@@ -780,7 +781,7 @@ struct list_head *btrfs_get_workspace(struct btrfs_fs_info *fs_info, int type, i
 	struct workspace_manager *wsm = fs_info->compr_wsm[type];
 	struct list_head *workspace;
 	int cpus = num_online_cpus();
-	unsigned nofs_flag;
+	unsigned int nofs_flag;
 	struct list_head *idle_ws;
 	spinlock_t *ws_lock;
 	atomic_t *total_ws;
@@ -1163,17 +1164,17 @@ static u64 file_offset_from_bvec(const struct bio_vec *bvec)
  * @buf:		The decompressed data buffer
  * @buf_len:		The decompressed data length
  * @decompressed:	Number of bytes that are already decompressed inside the
- * 			compressed extent
+ *			compressed extent
  * @cb:			The compressed extent descriptor
  * @orig_bio:		The original bio that the caller wants to read for
  *
  * An easier to understand graph is like below:
  *
- * 		|<- orig_bio ->|     |<- orig_bio->|
- * 	|<-------      full decompressed extent      ----->|
- * 	|<-----------    @cb range   ---->|
- * 	|			|<-- @buf_len -->|
- * 	|<--- @decompressed --->|
+ *		|<- orig_bio ->|     |<- orig_bio->|
+ *	|<-------      full decompressed extent      ----->|
+ *	|<-----------    @cb range   ---->|
+ *	|			|<-- @buf_len -->|
+ *	|<--- @decompressed --->|
  *
  * Note that, @cb can be a subpage of the full decompressed extent, but
  * @cb->start always has the same as the orig_file_offset value of the full
@@ -1295,7 +1296,8 @@ static u32 shannon_entropy(struct heuristic_ws *ws)
 #define RADIX_BASE		4U
 #define COUNTERS_SIZE		(1U << RADIX_BASE)
 
-static u8 get4bits(u64 num, int shift) {
+static u8 get4bits(u64 num, int shift)
+{
 	u8 low4bits;
 
 	num >>= shift;
@@ -1370,7 +1372,7 @@ static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
 		 */
 		memset(counters, 0, sizeof(counters));
 
-		for (i = 0; i < num; i ++) {
+		for (i = 0; i < num; i++) {
 			buf_num = array_buf[i].count;
 			addr = get4bits(buf_num, shift);
 			counters[addr]++;
-- 
2.53.0

Re: [PATCH 2/4] btrfs: clean coding style errors and warnings in compression.c

[Qu Wenruo]

在 2026/2/28 05:01, Adarsh Das 写道:
> As the previous patch is making changes to compression.c, this patch
> takes the oppurtunity to fix errors and warning in compression.c
> 
> Signed-off-by: Adarsh Das <adarshdas950@gmail.com>

This minor cleanup should be folded into the previous patch.

Thanks,
Qu

> ---
>   fs/btrfs/compression.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 29281aba925e..6c3be3550442 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -36,9 +36,9 @@
>   
>   static struct bio_set btrfs_compressed_bioset;
>   
> -static const char* const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
> +static const char * const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
>   
> -const char* btrfs_compress_type2str(enum btrfs_compression_type type)
> +const char *btrfs_compress_type2str(enum btrfs_compression_type type)
>   {
>   	switch (type) {
>   	case BTRFS_COMPRESS_ZLIB:
> @@ -478,6 +478,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>   
>   			if (zero_offset) {
>   				int zeros;
> +
>   				zeros = folio_size(folio) - zero_offset;
>   				folio_zero_range(folio, zero_offset, zeros);
>   			}
> @@ -780,7 +781,7 @@ struct list_head *btrfs_get_workspace(struct btrfs_fs_info *fs_info, int type, i
>   	struct workspace_manager *wsm = fs_info->compr_wsm[type];
>   	struct list_head *workspace;
>   	int cpus = num_online_cpus();
> -	unsigned nofs_flag;
> +	unsigned int nofs_flag;
>   	struct list_head *idle_ws;
>   	spinlock_t *ws_lock;
>   	atomic_t *total_ws;
> @@ -1163,17 +1164,17 @@ static u64 file_offset_from_bvec(const struct bio_vec *bvec)
>    * @buf:		The decompressed data buffer
>    * @buf_len:		The decompressed data length
>    * @decompressed:	Number of bytes that are already decompressed inside the
> - * 			compressed extent
> + *			compressed extent
>    * @cb:			The compressed extent descriptor
>    * @orig_bio:		The original bio that the caller wants to read for
>    *
>    * An easier to understand graph is like below:
>    *
> - * 		|<- orig_bio ->|     |<- orig_bio->|
> - * 	|<-------      full decompressed extent      ----->|
> - * 	|<-----------    @cb range   ---->|
> - * 	|			|<-- @buf_len -->|
> - * 	|<--- @decompressed --->|
> + *		|<- orig_bio ->|     |<- orig_bio->|
> + *	|<-------      full decompressed extent      ----->|
> + *	|<-----------    @cb range   ---->|
> + *	|			|<-- @buf_len -->|
> + *	|<--- @decompressed --->|
>    *
>    * Note that, @cb can be a subpage of the full decompressed extent, but
>    * @cb->start always has the same as the orig_file_offset value of the full
> @@ -1295,7 +1296,8 @@ static u32 shannon_entropy(struct heuristic_ws *ws)
>   #define RADIX_BASE		4U
>   #define COUNTERS_SIZE		(1U << RADIX_BASE)
>   
> -static u8 get4bits(u64 num, int shift) {
> +static u8 get4bits(u64 num, int shift)
> +{
>   	u8 low4bits;
>   
>   	num >>= shift;
> @@ -1370,7 +1372,7 @@ static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
>   		 */
>   		memset(counters, 0, sizeof(counters));
>   
> -		for (i = 0; i < num; i ++) {
> +		for (i = 0; i < num; i++) {
>   			buf_num = array_buf[i].count;
>   			addr = get4bits(buf_num, shift);
>   			counters[addr]++;

[PATCH 3/4] btrfs: replace BUG() and BUG_ON() with error handling in extent-tree.c

[Adarsh Das]

Replace BUG() and BUG_ON() calls with proper error handling instead
of crashing the kernel. Use btrfs_err() and return -EUCLEAN where
fs_info is available, and WARN_ON() with an error return where it
is not or where the condition is a programming bug rather than
on-disk corruption.

Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
 fs/btrfs/extent-tree.c | 82 ++++++++++++++++++++++++++++++++----------
 1 file changed, 63 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 03cf9f242c70..9748a4c5bc2d 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -604,7 +604,13 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
 		return -EUCLEAN;
 	}
 
-	BUG_ON(num_refs < refs_to_drop);
+	if (unlikely(num_refs < refs_to_drop)) {
+		btrfs_err(trans->fs_info,
+			  "dropping more refs than available, have %u want %u",
+			  num_refs, refs_to_drop);
+		btrfs_abort_transaction(trans, -EUCLEAN);
+		return -EUCLEAN;
+	}
 	num_refs -= refs_to_drop;
 
 	if (num_refs == 0) {
@@ -863,7 +869,13 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
 
 	if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK && !skinny_metadata) {
 		ptr += sizeof(struct btrfs_tree_block_info);
-		BUG_ON(ptr > end);
+		if (unlikely(ptr > end)) {
+			btrfs_err(
+				fs_info,
+				"extent item overflow at slot %d, ptr %lu end %lu",
+				path->slots[0], ptr, end);
+			return -EUCLEAN;
+		}
 	}
 
 	if (owner >= BTRFS_FIRST_FREE_OBJECTID)
@@ -1237,7 +1249,12 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
 {
 	int ret = 0;
 
-	BUG_ON(!is_data && refs_to_drop != 1);
+	if (unlikely(!is_data && refs_to_drop != 1)) {
+		btrfs_err(trans->fs_info,
+			  "invalid refs_to_drop %d for tree block, must be 1",
+			  refs_to_drop);
+		return -EUCLEAN;
+	}
 	if (iref)
 		ret = update_inline_extent_backref(trans, path, iref,
 						   -refs_to_drop, NULL);
@@ -1453,8 +1470,9 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
 
 	ASSERT(generic_ref->type != BTRFS_REF_NOT_SET &&
 	       generic_ref->action);
-	BUG_ON(generic_ref->type == BTRFS_REF_METADATA &&
-	       generic_ref->ref_root == BTRFS_TREE_LOG_OBJECTID);
+	if (WARN_ON(generic_ref->type == BTRFS_REF_METADATA &&
+		    generic_ref->ref_root == BTRFS_TREE_LOG_OBJECTID))
+		return -EINVAL;
 
 	if (generic_ref->type == BTRFS_REF_METADATA)
 		ret = btrfs_add_delayed_tree_ref(trans, generic_ref, NULL);
@@ -1622,7 +1640,9 @@ static int run_delayed_data_ref(struct btrfs_trans_handle *trans,
 	} else if (node->action == BTRFS_DROP_DELAYED_REF) {
 		ret = __btrfs_free_extent(trans, href, node, extent_op);
 	} else {
-		BUG();
+		btrfs_err(trans->fs_info, "unexpected delayed ref action %d",
+			  node->action);
+		return -EUCLEAN;
 	}
 	return ret;
 }
@@ -1639,7 +1659,8 @@ static void __run_delayed_extent_op(struct btrfs_delayed_extent_op *extent_op,
 
 	if (extent_op->update_key) {
 		struct btrfs_tree_block_info *bi;
-		BUG_ON(!(flags & BTRFS_EXTENT_FLAG_TREE_BLOCK));
+		if (WARN_ON(!(flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)))
+			return;
 		bi = (struct btrfs_tree_block_info *)(ei + 1);
 		btrfs_set_tree_block_key(leaf, bi, &extent_op->key);
 	}
@@ -1775,7 +1796,9 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans,
 		else
 			ret = __btrfs_free_extent(trans, href, node, extent_op);
 	} else {
-		BUG();
+		btrfs_err(trans->fs_info, "unexpected delayed ref action %d",
+			  node->action);
+		return -EUCLEAN;
 	}
 	return ret;
 }
@@ -2645,7 +2668,11 @@ int btrfs_pin_extent(struct btrfs_trans_handle *trans, u64 bytenr, u64 num_bytes
 	struct btrfs_block_group *cache;
 
 	cache = btrfs_lookup_block_group(trans->fs_info, bytenr);
-	BUG_ON(!cache); /* Logic error */
+	if (unlikely(!cache)) {
+		btrfs_err(trans->fs_info,
+			  "unable to find block group for bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
 
 	pin_down_extent(trans, cache, bytenr, num_bytes, true);
 
@@ -4125,7 +4152,9 @@ static int do_allocation(struct btrfs_block_group *block_group,
 	case BTRFS_EXTENT_ALLOC_ZONED:
 		return do_allocation_zoned(block_group, ffe_ctl, bg_ret);
 	default:
-		BUG();
+		btrfs_err(block_group->fs_info, "invalid allocation policy %d",
+			  ffe_ctl->policy);
+		return -EUCLEAN;
 	}
 }
 
@@ -4141,11 +4170,20 @@ static void release_block_group(struct btrfs_block_group *block_group,
 		/* Nothing to do */
 		break;
 	default:
-		BUG();
+		btrfs_err(block_group->fs_info, "invalid allocation policy %d",
+			  ffe_ctl->policy);
+		goto release;
+	}
+
+	if (unlikely(btrfs_bg_flags_to_raid_index(block_group->flags) !=
+		     ffe_ctl->index)) {
+		btrfs_err(block_group->fs_info,
+			  "mismatched raid index, block group flags %llu index %d",
+			  block_group->flags, ffe_ctl->index);
+		goto release;
 	}
 
-	BUG_ON(btrfs_bg_flags_to_raid_index(block_group->flags) !=
-	       ffe_ctl->index);
+release:
 	btrfs_release_block_group(block_group, delalloc);
 }
 
@@ -4172,7 +4210,8 @@ static void found_extent(struct find_free_extent_ctl *ffe_ctl,
 		/* Nothing to do */
 		break;
 	default:
-		BUG();
+		WARN_ONCE(1, "invalid allocation policy %d", ffe_ctl->policy);
+		return;
 	}
 }
 
@@ -4238,7 +4277,9 @@ static int can_allocate_chunk(struct btrfs_fs_info *fs_info,
 	case BTRFS_EXTENT_ALLOC_ZONED:
 		return can_allocate_chunk_zoned(fs_info, ffe_ctl);
 	default:
-		BUG();
+		btrfs_err(fs_info, "invalid allocation policy %d",
+			  ffe_ctl->policy);
+		return -EUCLEAN;
 	}
 }
 
@@ -4448,7 +4489,9 @@ static int prepare_allocation(struct btrfs_fs_info *fs_info,
 	case BTRFS_EXTENT_ALLOC_ZONED:
 		return prepare_allocation_zoned(fs_info, ffe_ctl, space_info);
 	default:
-		BUG();
+		btrfs_err(fs_info, "invalid allocation policy %d",
+			  ffe_ctl->policy);
+		return -EUCLEAN;
 	}
 }
 
@@ -5292,8 +5335,8 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
 			parent = ins.objectid;
 		flags |= BTRFS_BLOCK_FLAG_FULL_BACKREF;
 		owning_root = reloc_src_root;
-	} else
-		BUG_ON(parent > 0);
+	} else if (WARN_ON(parent > 0))
+		return ERR_PTR(-EINVAL);
 
 	if (root_objectid != BTRFS_TREE_LOG_OBJECTID) {
 		struct btrfs_delayed_extent_op *extent_op;
@@ -6437,7 +6480,8 @@ int btrfs_drop_subtree(struct btrfs_trans_handle *trans,
 	int parent_level;
 	int ret = 0;
 
-	BUG_ON(btrfs_root_id(root) != BTRFS_TREE_RELOC_OBJECTID);
+	if (WARN_ON(btrfs_root_id(root) != BTRFS_TREE_RELOC_OBJECTID))
+		return -EINVAL;
 
 	path = btrfs_alloc_path();
 	if (!path)
-- 
2.53.0

Re: [PATCH 3/4] btrfs: replace BUG() and BUG_ON() with error handling in extent-tree.c

[Qu Wenruo]

在 2026/2/28 05:01, Adarsh Das 写道:
> Replace BUG() and BUG_ON() calls with proper error handling instead
> of crashing the kernel. Use btrfs_err() and return -EUCLEAN where
> fs_info is available, and WARN_ON() with an error return where it
> is not or where the condition is a programming bug rather than
> on-disk corruption.
> 
> Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
> ---
>   fs/btrfs/extent-tree.c | 82 ++++++++++++++++++++++++++++++++----------
>   1 file changed, 63 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 03cf9f242c70..9748a4c5bc2d 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -604,7 +604,13 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
>   		return -EUCLEAN;
>   	}
>   
> -	BUG_ON(num_refs < refs_to_drop);
> +	if (unlikely(num_refs < refs_to_drop)) {
> +		btrfs_err(trans->fs_info,
> +			  "dropping more refs than available, have %u want %u",
> +			  num_refs, refs_to_drop);
> +		btrfs_abort_transaction(trans, -EUCLEAN);
> +		return -EUCLEAN;
> +	}
>   	num_refs -= refs_to_drop;
>   
>   	if (num_refs == 0) {
> @@ -863,7 +869,13 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
>   
>   	if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK && !skinny_metadata) {
>   		ptr += sizeof(struct btrfs_tree_block_info);
> -		BUG_ON(ptr > end);
> +		if (unlikely(ptr > end)) {
> +			btrfs_err(
> +				fs_info,

Please put @fs_info in the same line of "btrfs_err(", there is 
definitely enough space for it.

> +				"extent item overflow at slot %d, ptr %lu end %lu",
> +				path->slots[0], ptr, end);
> +			return -EUCLEAN;
> +		}

And I don't think we even need to have this handling.
If the size is not correct, it should be caught by tree-checker first, 
and that will provide better debug output normally.

Normally it can be replaced by an ASSERT().

>   	}
>   
>   	if (owner >= BTRFS_FIRST_FREE_OBJECTID)
> @@ -1237,7 +1249,12 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
>   {
>   	int ret = 0;
>   
> -	BUG_ON(!is_data && refs_to_drop != 1);
> +	if (unlikely(!is_data && refs_to_drop != 1)) {
> +		btrfs_err(trans->fs_info,
> +			  "invalid refs_to_drop %d for tree block, must be 1",
> +			  refs_to_drop);
> +		return -EUCLEAN;
> +	}

This is more like a runtime sanity checks, and we do not want it to be 
just ignored (although it will also flips the fs RO and noisy enough for 
most cases).

When this happens it's definitely some logical not correct.

I'd prefer it to be changed to ASSERT().

>   	if (iref)
>   		ret = update_inline_extent_backref(trans, path, iref,
>   						   -refs_to_drop, NULL);
> @@ -1453,8 +1470,9 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>   
>   	ASSERT(generic_ref->type != BTRFS_REF_NOT_SET &&
>   	       generic_ref->action);
> -	BUG_ON(generic_ref->type == BTRFS_REF_METADATA &&
> -	       generic_ref->ref_root == BTRFS_TREE_LOG_OBJECTID);
> +	if (WARN_ON(generic_ref->type == BTRFS_REF_METADATA &&
> +		    generic_ref->ref_root == BTRFS_TREE_LOG_OBJECTID))
> +		return -EINVAL;

Again it's a wrong logic, ASSERT() would be enough, especially the 
previous check is also an ASSERT().

>   
>   	if (generic_ref->type == BTRFS_REF_METADATA)
>   		ret = btrfs_add_delayed_tree_ref(trans, generic_ref, NULL);
> @@ -1622,7 +1640,9 @@ static int run_delayed_data_ref(struct btrfs_trans_handle *trans,
>   	} else if (node->action == BTRFS_DROP_DELAYED_REF) {
>   		ret = __btrfs_free_extent(trans, href, node, extent_op);
>   	} else {
> -		BUG();
> +		btrfs_err(trans->fs_info, "unexpected delayed ref action %d",
> +			  node->action);
> +		return -EUCLEAN;

There is already an ASSERT() in add_delayed_data_ref(), you can enhance 
that check and replace the BUG() with ASSERT().

>   	}
>   	return ret;
>   }
> @@ -1639,7 +1659,8 @@ static void __run_delayed_extent_op(struct btrfs_delayed_extent_op *extent_op,
>   
>   	if (extent_op->update_key) {
>   		struct btrfs_tree_block_info *bi;
> -		BUG_ON(!(flags & BTRFS_EXTENT_FLAG_TREE_BLOCK));
> +		if (WARN_ON(!(flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)))
> +			return;

You can grab the fs_info from @leaf.

And I don't like just pure warning in this case.
You can enhance __run_delayed_extent_op() to return an error code.

Furthermore you can fix the code style problem here, no need for a 
dedicated patch just to fix those minor new line problems.

As it's already mentioned in our guideline:
https://btrfs.readthedocs.io/en/latest/dev/Developer-s-FAQ.html#how-not-to-start

 >> If you care so much about the whitespace/style issues, just fix them 
while doing a useful change as mentioned above that happens to touch the 
same code.



>   		bi = (struct btrfs_tree_block_info *)(ei + 1);
>   		btrfs_set_tree_block_key(leaf, bi, &extent_op->key);
>   	}
> @@ -1775,7 +1796,9 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans,
>   		else
>   			ret = __btrfs_free_extent(trans, href, node, extent_op);
>   	} else {
> -		BUG();
> +		btrfs_err(trans->fs_info, "unexpected delayed ref action %d",
> +			  node->action);
> +		return -EUCLEAN;

The same, do more validation at insert time, the earlier detection the 
more usefulness it provides.

>   	}
>   	return ret;
>   }
> @@ -2645,7 +2668,11 @@ int btrfs_pin_extent(struct btrfs_trans_handle *trans, u64 bytenr, u64 num_bytes
>   	struct btrfs_block_group *cache;
>   
>   	cache = btrfs_lookup_block_group(trans->fs_info, bytenr);
> -	BUG_ON(!cache); /* Logic error */
> +	if (unlikely(!cache)) {
> +		btrfs_err(trans->fs_info,
> +			  "unable to find block group for bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
>   
>   	pin_down_extent(trans, cache, bytenr, num_bytes, true);
>   
> @@ -4125,7 +4152,9 @@ static int do_allocation(struct btrfs_block_group *block_group,
>   	case BTRFS_EXTENT_ALLOC_ZONED:
>   		return do_allocation_zoned(block_group, ffe_ctl, bg_ret);
>   	default:
> -		BUG();
> +		btrfs_err(block_group->fs_info, "invalid allocation policy %d",
> +			  ffe_ctl->policy);
> +		return -EUCLEAN;

ASSERT() is more suitable here, the policy is not from on-disk data but 
purely from runtime code.

Unknown policy means some code is passing wrong numbers.
ASSERT() is more suitable.

This error message vs ASSERT() applies to all the remaining changes.

Thanks,
Qu

>   	}
>   }
>   
> @@ -4141,11 +4170,20 @@ static void release_block_group(struct btrfs_block_group *block_group,
>   		/* Nothing to do */
>   		break;
>   	default:
> -		BUG();
> +		btrfs_err(block_group->fs_info, "invalid allocation policy %d",
> +			  ffe_ctl->policy);
> +		goto release;
> +	}
> +
> +	if (unlikely(btrfs_bg_flags_to_raid_index(block_group->flags) !=
> +		     ffe_ctl->index)) {
> +		btrfs_err(block_group->fs_info,
> +			  "mismatched raid index, block group flags %llu index %d",
> +			  block_group->flags, ffe_ctl->index);
> +		goto release;
>   	}
>   
> -	BUG_ON(btrfs_bg_flags_to_raid_index(block_group->flags) !=
> -	       ffe_ctl->index);
> +release:
>   	btrfs_release_block_group(block_group, delalloc);
>   }
>   
> @@ -4172,7 +4210,8 @@ static void found_extent(struct find_free_extent_ctl *ffe_ctl,
>   		/* Nothing to do */
>   		break;
>   	default:
> -		BUG();
> +		WARN_ONCE(1, "invalid allocation policy %d", ffe_ctl->policy);
> +		return;
>   	}
>   }
>   
> @@ -4238,7 +4277,9 @@ static int can_allocate_chunk(struct btrfs_fs_info *fs_info,
>   	case BTRFS_EXTENT_ALLOC_ZONED:
>   		return can_allocate_chunk_zoned(fs_info, ffe_ctl);
>   	default:
> -		BUG();
> +		btrfs_err(fs_info, "invalid allocation policy %d",
> +			  ffe_ctl->policy);
> +		return -EUCLEAN;
>   	}
>   }
>   
> @@ -4448,7 +4489,9 @@ static int prepare_allocation(struct btrfs_fs_info *fs_info,
>   	case BTRFS_EXTENT_ALLOC_ZONED:
>   		return prepare_allocation_zoned(fs_info, ffe_ctl, space_info);
>   	default:
> -		BUG();
> +		btrfs_err(fs_info, "invalid allocation policy %d",
> +			  ffe_ctl->policy);
> +		return -EUCLEAN;
>   	}
>   }
>   
> @@ -5292,8 +5335,8 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
>   			parent = ins.objectid;
>   		flags |= BTRFS_BLOCK_FLAG_FULL_BACKREF;
>   		owning_root = reloc_src_root;
> -	} else
> -		BUG_ON(parent > 0);
> +	} else if (WARN_ON(parent > 0))
> +		return ERR_PTR(-EINVAL);
>   
>   	if (root_objectid != BTRFS_TREE_LOG_OBJECTID) {
>   		struct btrfs_delayed_extent_op *extent_op;
> @@ -6437,7 +6480,8 @@ int btrfs_drop_subtree(struct btrfs_trans_handle *trans,
>   	int parent_level;
>   	int ret = 0;
>   
> -	BUG_ON(btrfs_root_id(root) != BTRFS_TREE_RELOC_OBJECTID);
> +	if (WARN_ON(btrfs_root_id(root) != BTRFS_TREE_RELOC_OBJECTID))
> +		return -EINVAL;
>   
>   	path = btrfs_alloc_path();
>   	if (!path)

[PATCH 4/4] btrfs: clean coding style errors in extent-tree.c

[Adarsh Das]

As the previous commits are changing code in extent-tree, this patch
takes the oppurtunity to fix checkpatch errors in the same files. All
the errors were formatting related.

Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
 fs/btrfs/extent-tree.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 9748a4c5bc2d..8ea88174e4d5 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2111,7 +2111,7 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
 			 * head
 			 */
 			ret = cleanup_ref_head(trans, locked_ref, &bytes_processed);
-			if (ret > 0 ) {
+			if (ret > 0) {
 				/* We dropped our lock, we need to loop. */
 				ret = 0;
 				continue;
@@ -4351,8 +4351,7 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info,
 			if (ret == -ENOSPC) {
 				ret = 0;
 				ffe_ctl->loop++;
-			}
-			else if (ret < 0)
+			} else if (ret < 0)
 				btrfs_abort_transaction(trans, ret);
 			else
 				ret = 0;
@@ -5676,7 +5675,7 @@ static int check_ref_exists(struct btrfs_trans_handle *trans,
 		 * If we get 0 then we found our reference, return 1, else
 		 * return the error if it's not -ENOENT;
 		 */
-		return (ret < 0 ) ? ret : 1;
+		return (ret < 0) ? ret : 1;
 	}
 
 	/*
-- 
2.53.0

Re: [PATCH 4/4] btrfs: clean coding style errors in extent-tree.c

[Qu Wenruo]

在 2026/2/28 05:01, Adarsh Das 写道:
> As the previous commits are changing code in extent-tree, this patch
> takes the oppurtunity to fix checkpatch errors in the same files. All
> the errors were formatting related.
> 
> Signed-off-by: Adarsh Das <adarshdas950@gmail.com>

Just fold this into the previous patch.

Thanks,
Qu

> ---
>   fs/btrfs/extent-tree.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 9748a4c5bc2d..8ea88174e4d5 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2111,7 +2111,7 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
>   			 * head
>   			 */
>   			ret = cleanup_ref_head(trans, locked_ref, &bytes_processed);
> -			if (ret > 0 ) {
> +			if (ret > 0) {
>   				/* We dropped our lock, we need to loop. */
>   				ret = 0;
>   				continue;
> @@ -4351,8 +4351,7 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info,
>   			if (ret == -ENOSPC) {
>   				ret = 0;
>   				ffe_ctl->loop++;
> -			}
> -			else if (ret < 0)
> +			} else if (ret < 0)
>   				btrfs_abort_transaction(trans, ret);
>   			else
>   				ret = 0;
> @@ -5676,7 +5675,7 @@ static int check_ref_exists(struct btrfs_trans_handle *trans,
>   		 * If we get 0 then we found our reference, return 1, else
>   		 * return the error if it's not -ENOENT;
>   		 */
> -		return (ret < 0 ) ? ret : 1;
> +		return (ret < 0) ? ret : 1;
>   	}
>   
>   	/*

Re: [PATCH 0/4] btrfs: replace BUG() and BUG_ON() with error handling

[Qu Wenruo]

在 2026/2/28 05:01, Adarsh Das 写道:
> Replace BUG() and BUG_ON() calls in compression.c and extent-tree.c
> with proper error handling so the kernel does not crash on unexpected
> conditions.

I think one needs to distinguish sanity checks and real proper error 
handling.

I'd say, if some parameter/values are only generated from runtime code, 
you're doing sanity checks, and it applies to most of your fixes.

In that case, ASSERT() is preferred, and should be checked as early as 
possible (e.g. checks before adding something to a list/rbtree, other 
than checking when handling the list/rbtree entry).

And if some developer hits that ASSERT() frequently enough, we can later 
change it to a proper error handling later.



If something is read from the disk, it needs proper error handling, but 
in that case it should be done in tree-checker, which provides a more 
concentrated place to do such checks.

In the original location where there is a BUG_ON(), after making sure 
it's already covered by tree-checker (or adding the missing check), 
replacing the BUG_ON() with an ASSERT().


> Also fix checkpatch warnings and errors found in both files.

For such code style errors, fix them inside the real helpful patches 
instead.

Thanks,
Qu

> 
> Adarsh Das (4):
>    btrfs: replace BUG() with error handling in compression.c
>    btrfs: clean coding style errors and warnings in compression.c
>    btrfs: replace BUG() and BUG_ON() with error handling in extent-tree.c
>    btrfs: clean coding style errors in extent-tree.c
> 
>   fs/btrfs/compression.c | 66 ++++++++++++-------------------
>   fs/btrfs/extent-tree.c | 89 +++++++++++++++++++++++++++++++-----------
>   2 files changed, 91 insertions(+), 64 deletions(-)
>