Re: [PATCH 06/19] xfs: return -ENOSPC from xfs_rtallocate_*

Darrick J. Wong
2023-12-14T20:50:39Z

On Thu, Dec 14, 2023 at 07:34:25AM +0100, Christoph Hellwig wrote:
> Just return -ENOSPC instead of returning 0 and setting the return rt
> extent number to NULLRTEXTNO. This is turn removes all users of
> NULLRTEXTNO, so remove that as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_types.h | 1 -
> fs/xfs/xfs_rtalloc.c | 211 +++++++++++++-------------------------
> 2 files changed, 71 insertions(+), 141 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 533200c4ccc25a..c3636ea21ecd05 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -51,7 +51,6 @@ typedef void * xfs_failaddr_t;
> #define NULLRFSBLOCK ((xfs_rfsblock_t)-1)
> #define NULLRTBLOCK ((xfs_rtblock_t)-1)
> #define NULLFILEOFF ((xfs_fileoff_t)-1)
> -#define NULLRTEXTNO ((xfs_rtxnum_t)-1)

Getting rid of this cleans out a nice quantity of if test cruft in the
rt modernization series, so...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>
> #define NULLAGBLOCK ((xfs_agblock_t)-1)
> #define NULLAGNUMBER ((xfs_agnumber_t)-1)
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 74edea8579818d..dac148d53af3ec 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -156,17 +156,17 @@ xfs_rtallocate_range(
> * properly update the summary.
> */
> error = xfs_rtfind_back(args, start, 0, &preblock);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> /*
> * Find the next allocated block (end of free extent).
> */
> error = xfs_rtfind_forw(args, end, mp->m_sb.sb_rextents - 1,
> &postblock);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> /*
> * Decrement the summary information corresponding to the entire
> * (old) free extent.
> @@ -174,9 +174,9 @@ xfs_rtallocate_range(
> error = xfs_rtmodify_summary(args,
> XFS_RTBLOCKLOG(postblock + 1 - preblock),
> xfs_rtx_to_rbmblock(mp, preblock), -1);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> /*
> * If there are blocks not being allocated at the front of the
> * old extent, add summary data for them to be free.
> @@ -185,10 +185,10 @@ xfs_rtallocate_range(
> error = xfs_rtmodify_summary(args,
> XFS_RTBLOCKLOG(start - preblock),
> xfs_rtx_to_rbmblock(mp, preblock), 1);
> - if (error) {
> + if (error)
> return error;
> - }
> }
> +
> /*
> * If there are blocks not being allocated at the end of the
> * old extent, add summary data for them to be free.
> @@ -197,15 +197,14 @@ xfs_rtallocate_range(
> error = xfs_rtmodify_summary(args,
> XFS_RTBLOCKLOG(postblock - end),
> xfs_rtx_to_rbmblock(mp, end + 1), 1);
> - if (error) {
> + if (error)
> return error;
> - }
> }
> +
> /*
> * Modify the bitmap to mark this extent allocated.
> */
> - error = xfs_rtmodify_range(args, start, len, 0);
> - return error;
> + return xfs_rtmodify_range(args, start, len, 0);
> }
>
> /*
> @@ -267,17 +266,17 @@ xfs_rtallocate_extent_block(
> * If it's not so then next will contain the first non-free.
> */
> error = xfs_rtcheck_range(args, i, maxlen, 1, &next, &stat);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> if (stat) {
> /*
> * i for maxlen is all free, allocate and return that.
> */
> error = xfs_rtallocate_range(args, i, maxlen);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> *len = maxlen;
> *rtx = i;
> return 0;
> @@ -302,9 +301,8 @@ xfs_rtallocate_extent_block(
> */
> if (next < end) {
> error = xfs_rtfind_forw(args, next, end, &i);
> - if (error) {
> + if (error)
> return error;
> - }
> } else
> break;
> }
> @@ -327,9 +325,8 @@ xfs_rtallocate_extent_block(
> * Allocate besti for bestlen & return that.
> */
> error = xfs_rtallocate_range(args, besti, bestlen);
> - if (error) {
> + if (error)
> return error;
> - }
> *len = bestlen;
> *rtx = besti;
> return 0;
> @@ -338,8 +335,7 @@ xfs_rtallocate_extent_block(
> * Allocation failed. Set *nextp to the next block to try.
> */
> *nextp = next;
> - *rtx = NULLRTEXTNO;
> - return 0;
> + return -ENOSPC;
> }
>
> /*
> @@ -369,17 +365,16 @@ xfs_rtallocate_extent_exact(
> * Check if the range in question (for maxlen) is free.
> */
> error = xfs_rtcheck_range(args, start, maxlen, 1, &next, &isfree);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> if (isfree) {
> /*
> * If it is, allocate it and return success.
> */
> error = xfs_rtallocate_range(args, start, maxlen);
> - if (error) {
> + if (error)
> return error;
> - }
> *len = maxlen;
> *rtx = start;
> return 0;
> @@ -388,33 +383,23 @@ xfs_rtallocate_extent_exact(
> * If not, allocate what there is, if it's at least minlen.
> */
> maxlen = next - start;
> - if (maxlen < minlen) {
> - /*
> - * Failed, return failure status.
> - */
> - *rtx = NULLRTEXTNO;
> - return 0;
> - }
> + if (maxlen < minlen)
> + return -ENOSPC;
> +
> /*
> * Trim off tail of extent, if prod is specified.
> */
> if (prod > 1 && (i = maxlen % prod)) {
> maxlen -= i;
> - if (maxlen < minlen) {
> - /*
> - * Now we can't do it, return failure status.
> - */
> - *rtx = NULLRTEXTNO;
> - return 0;
> - }
> + if (maxlen < minlen)
> + return -ENOSPC;
> }
> /*
> * Allocate what we can and return it.
> */
> error = xfs_rtallocate_range(args, start, maxlen);
> - if (error) {
> + if (error)
> return error;
> - }
> *len = maxlen;
> *rtx = start;
> return 0;
> @@ -443,7 +428,6 @@ xfs_rtallocate_extent_near(
> int j; /* secondary loop control */
> int log2len; /* log2 of minlen */
> xfs_rtxnum_t n; /* next rtext to try */
> - xfs_rtxnum_t r; /* result rtext */
>
> ASSERT(minlen % prod == 0);
> ASSERT(maxlen % prod == 0);
> @@ -457,26 +441,18 @@ xfs_rtallocate_extent_near(
>
> /* Make sure we don't run off the end of the rt volume. */
> maxlen = xfs_rtallocate_clamp_len(mp, start, maxlen, prod);
> - if (maxlen < minlen) {
> - *rtx = NULLRTEXTNO;
> - return 0;
> - }
> + if (maxlen < minlen)
> + return -ENOSPC;
>
> /*
> * Try the exact allocation first.
> */
> error = xfs_rtallocate_extent_exact(args, start, minlen, maxlen, len,
> - prod, &r);
> - if (error) {
> + prod, rtx);
> + if (error != -ENOSPC)
> return error;
> - }
> - /*
> - * If the exact allocation worked, return that.
> - */
> - if (r != NULLRTEXTNO) {
> - *rtx = r;
> - return 0;
> - }
> +
> +
> bbno = xfs_rtx_to_rbmblock(mp, start);
> i = 0;
> j = -1;
> @@ -492,9 +468,9 @@ xfs_rtallocate_extent_near(
> */
> error = xfs_rtany_summary(args, log2len, mp->m_rsumlevels - 1,
> bbno + i, &maxlog);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> /*
> * If there are any useful extents starting here, try
> * allocating one.
> @@ -513,17 +489,9 @@ xfs_rtallocate_extent_near(
> */
> error = xfs_rtallocate_extent_block(args,
> bbno + i, minlen, maxavail, len,
> - &n, prod, &r);
> - if (error) {
> + &n, prod, rtx);
> + if (error != -ENOSPC)
> return error;
> - }
> - /*
> - * If it worked, return it.
> - */
> - if (r != NULLRTEXTNO) {
> - *rtx = r;
> - return 0;
> - }
> }
> /*
> * On the negative side of the starting location.
> @@ -557,17 +525,9 @@ xfs_rtallocate_extent_near(
> error = xfs_rtallocate_extent_block(args,
> bbno + j, minlen,
> maxavail, len, &n, prod,
> - &r);
> - if (error) {
> + rtx);
> + if (error != -ENOSPC)
> return error;
> - }
> - /*
> - * If it works, return the extent.
> - */
> - if (r != NULLRTEXTNO) {
> - *rtx = r;
> - return 0;
> - }
> }
> }
> }
> @@ -601,8 +561,7 @@ xfs_rtallocate_extent_near(
> else
> break;
> }
> - *rtx = NULLRTEXTNO;
> - return 0;
> + return -ENOSPC;
> }
>
> /*
> @@ -624,7 +583,6 @@ xfs_rtallocate_extent_size(
> xfs_fileoff_t i; /* bitmap block number */
> int l; /* level number (loop control) */
> xfs_rtxnum_t n; /* next rtext to be tried */
> - xfs_rtxnum_t r; /* result rtext number */
> xfs_suminfo_t sum; /* summary information for extents */
>
> ASSERT(minlen % prod == 0);
> @@ -647,9 +605,8 @@ xfs_rtallocate_extent_size(
> * Get the summary for this level/block.
> */
> error = xfs_rtget_summary(args, l, i, &sum);
> - if (error) {
> + if (error)
> return error;
> - }
> /*
> * Nothing there, on to the next block.
> */
> @@ -659,17 +616,9 @@ xfs_rtallocate_extent_size(
> * Try allocating the extent.
> */
> error = xfs_rtallocate_extent_block(args, i, maxlen,
> - maxlen, len, &n, prod, &r);
> - if (error) {
> + maxlen, len, &n, prod, rtx);
> + if (error != -ENOSPC)
> return error;
> - }
> - /*
> - * If it worked, return that.
> - */
> - if (r != NULLRTEXTNO) {
> - *rtx = r;
> - return 0;
> - }
> /*
> * If the "next block to try" returned from the
> * allocator is beyond the next bitmap block,
> @@ -683,10 +632,8 @@ xfs_rtallocate_extent_size(
> * Didn't find any maxlen blocks. Try smaller ones, unless
> * we're asking for a fixed size extent.
> */
> - if (minlen > --maxlen) {
> - *rtx = NULLRTEXTNO;
> - return 0;
> - }
> + if (minlen > --maxlen)
> + return -ENOSPC;
> ASSERT(minlen != 0);
> ASSERT(maxlen != 0);
>
> @@ -705,9 +652,9 @@ xfs_rtallocate_extent_size(
> * Get the summary information for this level/block.
> */
> error = xfs_rtget_summary(args, l, i, &sum);
> - if (error) {
> + if (error)
> return error;
> - }
> +
> /*
> * If nothing there, go on to next.
> */
> @@ -721,17 +668,10 @@ xfs_rtallocate_extent_size(
> error = xfs_rtallocate_extent_block(args, i,
> XFS_RTMAX(minlen, 1 << l),
> XFS_RTMIN(maxlen, (1 << (l + 1)) - 1),
> - len, &n, prod, &r);
> - if (error) {
> + len, &n, prod, rtx);
> + if (error != -ENOSPC)
> return error;
> - }
> - /*
> - * If it worked, return that extent.
> - */
> - if (r != NULLRTEXTNO) {
> - *rtx = r;
> - return 0;
> - }
> +
> /*
> * If the "next block to try" returned from the
> * allocator is beyond the next bitmap block,
> @@ -744,8 +684,7 @@ xfs_rtallocate_extent_size(
> /*
> * Got nothing, return failure.
> */
> - *rtx = NULLRTEXTNO;
> - return 0;
> + return -ENOSPC;
> }
>
> /*
> @@ -1177,14 +1116,13 @@ xfs_rtallocate_extent(
> xfs_rtxlen_t *len, /* out: actual length allocated */
> int wasdel, /* was a delayed allocation extent */
> xfs_rtxlen_t prod, /* extent product factor */
> - xfs_rtxnum_t *rtblock) /* out: start rtext allocated */
> + xfs_rtxnum_t *rtx) /* out: start rtext allocated */
> {
> struct xfs_rtalloc_args args = {
> .mp = tp->t_mountp,
> .tp = tp,
> };
> int error; /* error value */
> - xfs_rtxnum_t r; /* result allocated rtext */
>
> ASSERT(xfs_isilocked(args.mp->m_rbmip, XFS_ILOCK_EXCL));
> ASSERT(minlen > 0 && minlen <= maxlen);
> @@ -1199,42 +1137,35 @@ xfs_rtallocate_extent(
> maxlen -= i;
> if ((i = minlen % prod))
> minlen += prod - i;
> - if (maxlen < minlen) {
> - *rtblock = NULLRTEXTNO;
> - return 0;
> - }
> + if (maxlen < minlen)
> + return -ENOSPC;
> }
>
> retry:
> if (start == 0) {
> error = xfs_rtallocate_extent_size(&args, minlen,
> - maxlen, len, prod, &r);
> + maxlen, len, prod, rtx);
> } else {
> error = xfs_rtallocate_extent_near(&args, start, minlen,
> - maxlen, len, prod, &r);
> + maxlen, len, prod, rtx);
> }
> -
> xfs_rtbuf_cache_relse(&args);
> - if (error)
> + if (error) {
> + if (error == -ENOSPC && prod > 1) {
> + prod = 1;
> + goto retry;
> + }
> return error;
> + }
>
> /*
> * If it worked, update the superblock.
> */
> - if (r != NULLRTEXTNO) {
> - long slen = (long)*len;
> -
> - ASSERT(*len >= minlen && *len <= maxlen);
> - if (wasdel)
> - xfs_trans_mod_sb(tp, XFS_TRANS_SB_RES_FREXTENTS, -slen);
> - else
> - xfs_trans_mod_sb(tp, XFS_TRANS_SB_FREXTENTS, -slen);
> - } else if (prod > 1) {
> - prod = 1;
> - goto retry;
> - }
> -
> - *rtblock = r;
> + ASSERT(*len >= minlen && *len <= maxlen);
> + if (wasdel)
> + xfs_trans_mod_sb(tp, XFS_TRANS_SB_RES_FREXTENTS, -(long)*len);
> + else
> + xfs_trans_mod_sb(tp, XFS_TRANS_SB_FREXTENTS, -(long)*len);
> return 0;
> }
>
> @@ -1548,16 +1479,16 @@ xfs_bmap_rtalloc(
> raminlen = max_t(xfs_rtxlen_t, 1, xfs_extlen_to_rtxlen(mp, minlen));
> error = xfs_rtallocate_extent(ap->tp, rtx, raminlen, ralen, &ralen,
> ap->wasdel, prod, &rtx);
> - if (error)
> - return error;
> -
> - if (rtx != NULLRTEXTNO) {
> + if (!error) {
> ap->blkno = xfs_rtx_to_rtb(mp, rtx);
> ap->length = xfs_rtxlen_to_extlen(mp, ralen);
> xfs_bmap_alloc_account(ap);
> return 0;
> }
>
> + if (error != -ENOSPC)
> + return error;
> +
> if (align > mp->m_sb.sb_rextsize) {
> /*
> * We previously enlarged the request length to try to satisfy
> --
> 2.39.2
>
>