Re: [PATCH 3/3] btrfs: cycle through the RAID profiles as a last resort

[Boris Burkov <boris@bur.io>]

On Thu, Jul 20, 2023 at 04:12:16PM -0400, Josef Bacik wrote:
> Instead of looping through the RAID indices before advancing the FFE
> loop lets move this until after we've exhausted the entire FFE loop in
> order to give us the highest chance of success in satisfying the
> allocation based on its flags.

Doesn't this get screwed by the find_free_extent_update_loop setting
index to 0?

i.e., let's say we fail on the first pass with the correct raid flag.
then we go into find_free_extent_update_loop and intelligently don't do
the pointless raid loops. But then we set index to 0 and start doing an
even worse meta loop of doing every step (including allocating chunks)
with every raid index, most of which are doomed to fail by definition.

Not setting it to 0, OTOH, breaks the logic for setting "full_search",
but I do think that could be fixed one way or another.

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/extent-tree.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 8db2673948c9..ca4277ec1b19 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3953,10 +3953,6 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info,
>  		return 0;
>  	}
>  
> -	ffe_ctl->index++;
> -	if (ffe_ctl->index < BTRFS_NR_RAID_TYPES)
> -		return 1;
> -
>  	/*
>  	 * See the comment for btrfs_loop_type for an explanation of the phases.
>  	 */
> @@ -4026,6 +4022,11 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info,
>  		}
>  		return 1;
>  	}
> +
> +	ffe_ctl->index++;
> +	if (ffe_ctl->index < BTRFS_NR_RAID_TYPES)
> +		return 1;
> +
>  	return -ENOSPC;
>  }
>  
> -- 
> 2.41.0
>