[PATCH] ocfs2: replace BUG_ON() at ocfs2_num_free_extents() with ocfs2_error()

[PATCH] ocfs2: replace BUG_ON() at ocfs2_num_free_extents() with ocfs2_error()

[Jia Rui]

The BUG_ON() at ocfs2_num_free_extents() handles the error that
l_tree_deepth of leaf extent block just read form disk is invalid.
This error is mostly caused by file system metadata corruption on
the disk. There is no need to call BUG_ON() to handle such errors.
We can return error code, since the caller can deal with errors
from ocfs2_num_free_extents(). Also, we should make the file
system read-only to avoid the damage from expanding.

Therefore, BUG_ON() is removed and ocfs2_error() is called instead.

Signed-off-by: Jia Rui <jindui71@gmail.com>
---
 fs/ocfs2/alloc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index aef58f1395c8..d6bcf00f6843 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -967,7 +967,14 @@ int ocfs2_num_free_extents(struct ocfs2_extent_tree *et)
 		el = &eb->h_list;
 	}
 
-	BUG_ON(el->l_tree_depth != 0);
+	if (el->l_tree_depth != 0) {
+		retval = ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci),
+				"Owner %llu has leaf extent block %llu with an invalid l_tree_depth of %u\n",
+				(unsigned long long)ocfs2_metadata_cache_owner(et->et_ci),
+				(unsigned long long)last_eb_blk,
+				le16_to_cpu(el->l_tree_depth));
+		goto bail;
+	}
 
 	retval = le16_to_cpu(el->l_count) - le16_to_cpu(el->l_next_free_rec);
 bail:
-- 
2.25.1

Re: [PATCH] ocfs2: replace BUG_ON() at ocfs2_num_free_extents() with ocfs2_error()

[Joseph Qi]

On 10/19/23 3:18 AM, Jia Rui wrote:
> The BUG_ON() at ocfs2_num_free_extents() handles the error that
> l_tree_deepth of leaf extent block just read form disk is invalid.
> This error is mostly caused by file system metadata corruption on
> the disk. There is no need to call BUG_ON() to handle such errors.
> We can return error code, since the caller can deal with errors
> from ocfs2_num_free_extents(). Also, we should make the file
> system read-only to avoid the damage from expanding.
> 
> Therefore, BUG_ON() is removed and ocfs2_error() is called instead.
> 
> Signed-off-by: Jia Rui <jindui71@gmail.com>

Looks reasonable.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/alloc.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index aef58f1395c8..d6bcf00f6843 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -967,7 +967,14 @@ int ocfs2_num_free_extents(struct ocfs2_extent_tree *et)
>  		el = &eb->h_list;
>  	}
>  
> -	BUG_ON(el->l_tree_depth != 0);
> +	if (el->l_tree_depth != 0) {
> +		retval = ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci),
> +				"Owner %llu has leaf extent block %llu with an invalid l_tree_depth of %u\n",
> +				(unsigned long long)ocfs2_metadata_cache_owner(et->et_ci),
> +				(unsigned long long)last_eb_blk,
> +				le16_to_cpu(el->l_tree_depth));
> +		goto bail;
> +	}
>  
>  	retval = le16_to_cpu(el->l_count) - le16_to_cpu(el->l_next_free_rec);
>  bail: