* [PATCH] btrfs: decrease indendation of find_free_extent_update_loop
@ 2026-03-17 13:25 Johannes Thumshirn
2026-03-17 19:54 ` Filipe Manana
0 siblings, 1 reply; 2+ messages in thread
From: Johannes Thumshirn @ 2026-03-17 13:25 UTC (permalink / raw)
To: linux-btrfs; +Cc: Johannes Thumshirn
Decrease the indendation of find_free_extent_update_loop(), by inverting
the check if the loop state is smaller than LOOP_NO_EMPTY_SIZE.
This also allows for an early return from find_free_extent_update_loop(),
in case LOOP_NO_EMPTY_SIZE is already set at this point.
While at if change a
if () {
}
else if
else
pattern to all using curly braces and be consistent with the rest of btrfs
code.
No functional changes intended.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
fs/btrfs/extent-tree.c | 111 +++++++++++++++++++++--------------------
1 file changed, 57 insertions(+), 54 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a55b910c755b..062fc1beb26b 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4349,71 +4349,74 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info,
return 1;
/* See the comments for btrfs_loop_type for an explanation of the phases. */
- if (ffe_ctl->loop < LOOP_NO_EMPTY_SIZE) {
- ffe_ctl->index = 0;
- /*
- * We want to skip the LOOP_CACHING_WAIT step if we don't have
- * any uncached bgs and we've already done a full search
- * through.
- */
- if (ffe_ctl->loop == LOOP_CACHING_NOWAIT &&
- (!ffe_ctl->orig_have_caching_bg && full_search))
- ffe_ctl->loop++;
+ if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE)
+ return -ENOSPC;
+
+ ffe_ctl->index = 0;
+ /*
+ * We want to skip the LOOP_CACHING_WAIT step if we don't have
+ * any uncached bgs and we've already done a full search
+ * through.
+ */
+ if (ffe_ctl->loop == LOOP_CACHING_NOWAIT &&
+ (!ffe_ctl->orig_have_caching_bg && full_search))
ffe_ctl->loop++;
+ ffe_ctl->loop++;
- if (ffe_ctl->loop == LOOP_ALLOC_CHUNK) {
- struct btrfs_trans_handle *trans;
- int exist = 0;
+ if (ffe_ctl->loop == LOOP_ALLOC_CHUNK) {
+ struct btrfs_trans_handle *trans;
+ int exist = 0;
- /* Check if allocation policy allows to create a new chunk */
- ret = can_allocate_chunk(fs_info, ffe_ctl);
- if (ret)
- return ret;
+ /* Check if allocation policy allows to create a new chunk */
+ ret = can_allocate_chunk(fs_info, ffe_ctl);
+ if (ret)
+ return ret;
- trans = current->journal_info;
- if (trans)
- exist = 1;
- else
- trans = btrfs_join_transaction(root);
+ trans = current->journal_info;
+ if (trans)
+ exist = 1;
+ else
+ trans = btrfs_join_transaction(root);
- if (IS_ERR(trans))
- return PTR_ERR(trans);
+ if (IS_ERR(trans))
+ return PTR_ERR(trans);
- ret = btrfs_chunk_alloc(trans, space_info, ffe_ctl->flags,
- CHUNK_ALLOC_FORCE_FOR_EXTENT);
+ ret = btrfs_chunk_alloc(trans, space_info, ffe_ctl->flags,
+ CHUNK_ALLOC_FORCE_FOR_EXTENT);
- /* Do not bail out on ENOSPC since we can do more. */
- if (ret == -ENOSPC) {
- ret = 0;
- ffe_ctl->loop++;
- }
- else if (ret < 0)
- btrfs_abort_transaction(trans, ret);
- else
- ret = 0;
- if (!exist)
- btrfs_end_transaction(trans);
- if (ret)
- return ret;
+ /* Do not bail out on ENOSPC since we can do more. */
+ if (ret == -ENOSPC) {
+ ret = 0;
+ ffe_ctl->loop++;
+ } else if (ret < 0) {
+ btrfs_abort_transaction(trans, ret);
+ } else {
+ ret = 0;
}
- if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE) {
- if (ffe_ctl->policy != BTRFS_EXTENT_ALLOC_CLUSTERED)
- return -ENOSPC;
+ if (!exist)
+ btrfs_end_transaction(trans);
- /*
- * Don't loop again if we already have no empty_size and
- * no empty_cluster.
- */
- if (ffe_ctl->empty_size == 0 &&
- ffe_ctl->empty_cluster == 0)
- return -ENOSPC;
- ffe_ctl->empty_size = 0;
- ffe_ctl->empty_cluster = 0;
- }
- return 1;
+ if (ret)
+ return ret;
+ }
+
+ if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE) {
+ if (ffe_ctl->policy != BTRFS_EXTENT_ALLOC_CLUSTERED)
+ return -ENOSPC;
+
+ /*
+ * Don't loop again if we already have no empty_size and
+ * no empty_cluster.
+ */
+ if (ffe_ctl->empty_size == 0 &&
+ ffe_ctl->empty_cluster == 0)
+ return -ENOSPC;
+ ffe_ctl->empty_size = 0;
+ ffe_ctl->empty_cluster = 0;
}
- return -ENOSPC;
+
+ return 1;
}
static int prepare_allocation_clustered(struct btrfs_fs_info *fs_info,
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] btrfs: decrease indendation of find_free_extent_update_loop
2026-03-17 13:25 [PATCH] btrfs: decrease indendation of find_free_extent_update_loop Johannes Thumshirn
@ 2026-03-17 19:54 ` Filipe Manana
0 siblings, 0 replies; 2+ messages in thread
From: Filipe Manana @ 2026-03-17 19:54 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: linux-btrfs
On Tue, Mar 17, 2026 at 1:29 PM Johannes Thumshirn
<johannes.thumshirn@wdc.com> wrote:
>
> Decrease the indendation of find_free_extent_update_loop(), by inverting
> the check if the loop state is smaller than LOOP_NO_EMPTY_SIZE.
>
> This also allows for an early return from find_free_extent_update_loop(),
> in case LOOP_NO_EMPTY_SIZE is already set at this point.
>
> While at if change a
if -> it
>
> if () {
> }
> else if
> else
>
> pattern to all using curly braces and be consistent with the rest of btrfs
> code.
>
> No functional changes intended.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> fs/btrfs/extent-tree.c | 111 +++++++++++++++++++++--------------------
> 1 file changed, 57 insertions(+), 54 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a55b910c755b..062fc1beb26b 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -4349,71 +4349,74 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info,
> return 1;
>
> /* See the comments for btrfs_loop_type for an explanation of the phases. */
> - if (ffe_ctl->loop < LOOP_NO_EMPTY_SIZE) {
> - ffe_ctl->index = 0;
> - /*
> - * We want to skip the LOOP_CACHING_WAIT step if we don't have
> - * any uncached bgs and we've already done a full search
> - * through.
> - */
> - if (ffe_ctl->loop == LOOP_CACHING_NOWAIT &&
> - (!ffe_ctl->orig_have_caching_bg && full_search))
> - ffe_ctl->loop++;
> + if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE)
> + return -ENOSPC;
> +
> + ffe_ctl->index = 0;
> + /*
> + * We want to skip the LOOP_CACHING_WAIT step if we don't have
> + * any uncached bgs and we've already done a full search
> + * through.
> + */
> + if (ffe_ctl->loop == LOOP_CACHING_NOWAIT &&
> + (!ffe_ctl->orig_have_caching_bg && full_search))
> ffe_ctl->loop++;
> + ffe_ctl->loop++;
>
> - if (ffe_ctl->loop == LOOP_ALLOC_CHUNK) {
> - struct btrfs_trans_handle *trans;
> - int exist = 0;
> + if (ffe_ctl->loop == LOOP_ALLOC_CHUNK) {
> + struct btrfs_trans_handle *trans;
> + int exist = 0;
This is a good opportunity to change the variable type to bool and
give it a more meaningful and correct name:
bool have_trans = false;
>
> - /* Check if allocation policy allows to create a new chunk */
> - ret = can_allocate_chunk(fs_info, ffe_ctl);
> - if (ret)
> - return ret;
> + /* Check if allocation policy allows to create a new chunk */
A good opportunity too to add missing punctuation to comment.
> + ret = can_allocate_chunk(fs_info, ffe_ctl);
> + if (ret)
> + return ret;
>
> - trans = current->journal_info;
> - if (trans)
> - exist = 1;
> - else
> - trans = btrfs_join_transaction(root);
> + trans = current->journal_info;
> + if (trans)
> + exist = 1;
And then here:
have_trans = true;
> + else
> + trans = btrfs_join_transaction(root);
>
> - if (IS_ERR(trans))
> - return PTR_ERR(trans);
> + if (IS_ERR(trans))
> + return PTR_ERR(trans);
>
> - ret = btrfs_chunk_alloc(trans, space_info, ffe_ctl->flags,
> - CHUNK_ALLOC_FORCE_FOR_EXTENT);
> + ret = btrfs_chunk_alloc(trans, space_info, ffe_ctl->flags,
> + CHUNK_ALLOC_FORCE_FOR_EXTENT);
>
> - /* Do not bail out on ENOSPC since we can do more. */
> - if (ret == -ENOSPC) {
> - ret = 0;
> - ffe_ctl->loop++;
> - }
> - else if (ret < 0)
> - btrfs_abort_transaction(trans, ret);
> - else
> - ret = 0;
> - if (!exist)
> - btrfs_end_transaction(trans);
> - if (ret)
> - return ret;
> + /* Do not bail out on ENOSPC since we can do more. */
> + if (ret == -ENOSPC) {
> + ret = 0;
> + ffe_ctl->loop++;
> + } else if (ret < 0) {
> + btrfs_abort_transaction(trans, ret);
> + } else {
> + ret = 0;
> }
>
> - if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE) {
> - if (ffe_ctl->policy != BTRFS_EXTENT_ALLOC_CLUSTERED)
> - return -ENOSPC;
> + if (!exist)
And here:
if (!have_trans)
And have such change mentioned in the changelog too.
Otherwise it looks good:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> + btrfs_end_transaction(trans);
>
> - /*
> - * Don't loop again if we already have no empty_size and
> - * no empty_cluster.
> - */
> - if (ffe_ctl->empty_size == 0 &&
> - ffe_ctl->empty_cluster == 0)
> - return -ENOSPC;
> - ffe_ctl->empty_size = 0;
> - ffe_ctl->empty_cluster = 0;
> - }
> - return 1;
> + if (ret)
> + return ret;
> + }
> +
> + if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE) {
> + if (ffe_ctl->policy != BTRFS_EXTENT_ALLOC_CLUSTERED)
> + return -ENOSPC;
> +
> + /*
> + * Don't loop again if we already have no empty_size and
> + * no empty_cluster.
> + */
> + if (ffe_ctl->empty_size == 0 &&
> + ffe_ctl->empty_cluster == 0)
> + return -ENOSPC;
> + ffe_ctl->empty_size = 0;
> + ffe_ctl->empty_cluster = 0;
> }
> - return -ENOSPC;
> +
> + return 1;
> }
>
> static int prepare_allocation_clustered(struct btrfs_fs_info *fs_info,
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-03-17 19:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 13:25 [PATCH] btrfs: decrease indendation of find_free_extent_update_loop Johannes Thumshirn
2026-03-17 19:54 ` Filipe Manana
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox