Re: [PATCH 1/9] xfs: split xfs_bmap_btalloc_at_eof()

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

The split looks good, and much easier to understand than before.

I have a minor nitpick on the callsites below:

> @@ -3612,8 +3612,14 @@ xfs_bmap_btalloc_filestreams(
>  	}
>  
>  	args->minlen = xfs_bmap_select_minlen(ap, args, blen);
> +	if (ap->aeof && ap->offset)
> +		error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align);
> +
> +	if (error || args->fsbno != NULLFSBLOCK)
> +		goto out_low_space;
> +
>  	if (ap->aeof)
> -		error = xfs_bmap_btalloc_at_eof(ap, args, blen, stripe_align,
> +		error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
>  				true);
>  
>  	if (!error && args->fsbno == NULLFSBLOCK)

I find the way how this is structured not very helpful to the read,
although most of the blame lies with the pre-existing code.  If we'd
check the error where it happens I think it would be way easier to read:

 	args->minlen = xfs_bmap_select_minlen(ap, args, blen);
	if (ap->aeof) {
		if (ap->offset) {
			error = xfs_bmap_btalloc_at_eof(ap, args, blen,
					stripe_align);
			if (error || args->fsbno != NULLFSBLOCK)
				goto out_low_space;
		}

		error = xfs_bmap_btalloc_aligned(ap, args, blen, stripe_align,
 				true);
		if (error || args->fsbno != NULLFSBLOCK)
			goto out_low_space;
	}

	error = xfs_alloc_vextent_near_bno(args, ap->blkno);

The same applies to xfs_bmap_btalloc_best_length.