On Thu, Dec 14, 2023 at 07:34:31AM +0100, Christoph Hellwig wrote:
> Share the xfs_rtallocate_range logic by breaking out of the loop
> instead of duplicating it, invert a check so that the early
> return case comes first instead of in an else, and handle the
> successful case in the straight line instead a branch in the tail
> of the function.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_rtalloc.c | 63 +++++++++++++++++++++-----------------------
> 1 file changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index fbc60658ef24bf..5f42422a976a3e 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -257,13 +257,9 @@ xfs_rtallocate_extent_block(
> /*
> * i for maxlen is all free, allocate and return that.
> */
> - error = xfs_rtallocate_range(args, i, maxlen);
> - if (error)
> - return error;
> -
> - *len = maxlen;
> - *rtx = i;
> - return 0;
> + bestlen = maxlen;
> + besti = i;
> + break;
This return->break change gave me pause for a while...
> }
> /*
> * In the case where we have a variable-sized allocation
> @@ -283,43 +279,44 @@ xfs_rtallocate_extent_block(
> /*
> * If not done yet, find the start of the next free space.
> */
> - if (next < end) {
> - error = xfs_rtfind_forw(args, next, end, &i);
> - if (error)
> - return error;
> - } else
> + if (next >= end)
> break;
> + error = xfs_rtfind_forw(args, next, end, &i);
> + if (error)
> + return error;
> }
> +
> /*
> * Searched the whole thing & didn't find a maxlen free extent.
> */
> - if (minlen <= maxlen && besti != -1) {
> - xfs_rtxlen_t p; /* amount to trim length by */
> -
> + if (maxlen < minlen || besti == -1) {
...because I was worried about accidentally ending up in this clause
if maxlen < minlen. I /think/ it's the case that this function never
gets called with that true, right?
(Would this be a good place to add a ebugging assertion?)
--D
> /*
> - * If size should be a multiple of prod, make that so.
> + * Allocation failed. Set *nextp to the next block to try.
> */
> - if (prod > 1) {
> - div_u64_rem(bestlen, prod, &p);
> - if (p)
> - bestlen -= p;
> - }
> + *nextp = next;
> + return -ENOSPC;
> + }
>
> - /*
> - * Allocate besti for bestlen & return that.
> - */
> - error = xfs_rtallocate_range(args, besti, bestlen);
> - if (error)
> - return error;
> - *len = bestlen;
> - *rtx = besti;
> - return 0;
> + /*
> + * If size should be a multiple of prod, make that so.
> + */
> + if (prod > 1) {
> + xfs_rtxlen_t p; /* amount to trim length by */
> +
> + div_u64_rem(bestlen, prod, &p);
> + if (p)
> + bestlen -= p;
> }
> +
> /*
> - * Allocation failed. Set *nextp to the next block to try.
> + * Allocate besti for bestlen & return that.
> */
> - *nextp = next;
> - return -ENOSPC;
> + error = xfs_rtallocate_range(args, besti, bestlen);
> + if (error)
> + return error;
> + *len = bestlen;
> + *rtx = besti;
> + return 0;
> }
>
> /*
> --
> 2.39.2
>
>
