btrfs: remove unnecessary -ENOMEM BUG_ON check in extent-tree.c/btrfs_alloc_logged_file_extent
PATCH 2/3 btrfs: remove unnecessary -ENOMEM BUG_ON check in extent-tree.c/btrfs_alloc_logged_file_extent
The memory allocation failure is BUG_ON in add_excluded_extent (following the code path). No need to BUG_ON -ENOMEM inside btrfs_alloc_logged_file_extent.
--- fs/btrfs/extent-tree.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
fs/btrfs/extent-tree.c +2 -4
@@ -6207,8 +6207,7 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,mutex_lock (&caching_ctl->mutex);if (start >= caching_ctl->progress) { - ret =add_excluded_extent (root, start, num_bytes); -BUG_ON (ret);/* -ENOMEM */ +add_excluded_extent (root, start, num_bytes); }else if (start + num_bytes <= caching_ctl->progress) { ret =btrfs_remove_free_space (block_group, start, num_bytes);
@@ -6222,8 +6221,7 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans, start = caching_ctl->progress; num_bytes = ins->objectid + ins->offset - caching_ctl->progress; - ret =add_excluded_extent (root, start, num_bytes); -BUG_ON (ret);/* -ENOMEM */ +add_excluded_extent (root, start, num_bytes); }mutex_unlock (&caching_ctl->mutex);
Signature · 1 line
1.7.1
Quoted 3 lines · Wang Sheng-Hui
The memory allocation failure is
BUG_ONinadd_excluded_extent(following the code path). No need toBUG_ON-ENOMEMinsidebtrfs_alloc_logged_file_extent.
This indirectly calls __set_extent_bit that does BUG_ON on memory allocation failures. This type of error condition is hard to fix, as it usually needs to do non-trivial cleanups in the function before returning -ENOMEM, so the easiset way to "handle" it is to do BUG_ON and then separately deal with it in another patch (so it does not mix with the original patch).
Your patches remove (from my POV) useful marks that we have an error condition to handle, not to hide it.
So, NAK from me for anything that looks like this. I'm of course glad to look at patches that replace the BUG_ON with proper error handling :)
david
Quoted 31 lines
Signed-off-by Wang Sheng-Hui <[email protected]>--- fs/btrfs/extent-tree.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)fs/btrfs/extent-tree.c +1
-2@@ -6207,8 +6207,7 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,mutex_lock (&caching_ctl->mutex);if (start >= caching_ctl->progress) { - ret =add_excluded_extent (root, start, num_bytes); -BUG_ON (ret);/* -ENOMEM */ +add_excluded_extent (root, start, num_bytes); }else if (start + num_bytes <= caching_ctl->progress) { ret = btrfs_remove_free_space(block_group,fs/btrfs/extent-tree.c +1
-2start, num_bytes);@@ -6222,8 +6221,7 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans, start = caching_ctl->progress; num_bytes = ins->objectid + ins->offset - caching_ctl->progress; - ret =add_excluded_extent (root, start, num_bytes); -BUG_ON (ret);/* -ENOMEM */ +add_excluded_extent (root, start, num_bytes); }mutex_unlock (&caching_ctl->mutex);