On Thu, Aug 01, 2024 at 04:30:47PM +0000, John Garry wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Currently the allocation at EOF is broken into two cases - when the
> offset is zero and when the offset is non-zero. When the offset is
> non-zero, we try to do exact block allocation for contiguous
> extent allocation. When the offset is zero, the allocation is simply
> an aligned allocation.
>
> We want aligned allocation as the fallback when exact block
> allocation fails, but that complicates the EOF allocation in that it
> now has to handle two different allocation cases. The
> caller also has to handle allocation when not at EOF, and for the
> upcoming forced alignment changes we need that to also be aligned
> allocation.
>
> To simplify all this, pull the aligned allocation cases back into
> the callers and leave the EOF allocation path for exact block
> allocation only. This means that the EOF exact block allocation
> fallback path is the normal aligned allocation path and that ends up
> making things a lot simpler when forced alignment is introduced.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 129 +++++++++++++++----------------------
> fs/xfs/libxfs/xfs_ialloc.c | 2 +-
> 2 files changed, 54 insertions(+), 77 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 25a87e1154bb..42a75d257b35 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3310,12 +3310,12 @@ xfs_bmap_select_minlen(
> static int
> xfs_bmap_btalloc_select_lengths(
> struct xfs_bmalloca *ap,
> - struct xfs_alloc_arg *args,
> - xfs_extlen_t *blen)
> + struct xfs_alloc_arg *args)
> {
> struct xfs_mount *mp = args->mp;
> struct xfs_perag *pag;
> xfs_agnumber_t agno, startag;
> + xfs_extlen_t blen = 0;
> int error = 0;
>
> if (ap->tp->t_flags & XFS_TRANS_LOWMODE) {
> @@ -3329,19 +3329,18 @@ xfs_bmap_btalloc_select_lengths(
> if (startag == NULLAGNUMBER)
> startag = 0;
>
> - *blen = 0;
> for_each_perag_wrap(mp, startag, agno, pag) {
> - error = xfs_bmap_longest_free_extent(pag, args->tp, blen);
> + error = xfs_bmap_longest_free_extent(pag, args->tp, &blen);
> if (error && error != -EAGAIN)
> break;
> error = 0;
> - if (*blen >= args->maxlen)
> + if (blen >= args->maxlen)
> break;
> }
> if (pag)
> xfs_perag_rele(pag);
>
> - args->minlen = xfs_bmap_select_minlen(ap, args, *blen);
> + args->minlen = xfs_bmap_select_minlen(ap, args, blen);
> return error;
> }
>
> @@ -3551,78 +3550,40 @@ xfs_bmap_exact_minlen_extent_alloc(
> * If we are not low on available data blocks and we are allocating at
> * EOF, optimise allocation for contiguous file extension and/or stripe
> * alignment of the new extent.
> - *
> - * NOTE: ap->aeof is only set if the allocation length is >= the
> - * stripe unit and the allocation offset is at the end of file.
> */
> static int
> xfs_bmap_btalloc_at_eof(
> struct xfs_bmalloca *ap,
> - struct xfs_alloc_arg *args,
> - xfs_extlen_t blen,
> - bool ag_only)
> + struct xfs_alloc_arg *args)
> {
> struct xfs_mount *mp = args->mp;
> struct xfs_perag *caller_pag = args->pag;
> + xfs_extlen_t alignment = args->alignment;
> int error;
>
> + ASSERT(ap->aeof && ap->offset);
> + ASSERT(args->alignment >= 1);
> +
> /*
> - * If there are already extents in the file, try an exact EOF block
> - * allocation to extend the file as a contiguous extent. If that fails,
> - * or it's the first allocation in a file, just try for a stripe aligned
> - * allocation.
> + * Compute the alignment slop for the fallback path so we ensure
> + * we account for the potential alignemnt space required by the
> + * fallback paths before we modify the AGF and AGFL here.
> */
> - if (ap->offset) {
> - xfs_extlen_t alignment = args->alignment;
> -
> - /*
> - * Compute the alignment slop for the fallback path so we ensure
> - * we account for the potential alignment space required by the
> - * fallback paths before we modify the AGF and AGFL here.
> - */
> - args->alignment = 1;
> - args->alignslop = alignment - args->alignment;
> -
> - if (!caller_pag)
> - args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
> - error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
> - if (!caller_pag) {
> - xfs_perag_put(args->pag);
> - args->pag = NULL;
> - }
> - if (error)
> - return error;
> -
> - if (args->fsbno != NULLFSBLOCK)
> - return 0;
> - /*
> - * Exact allocation failed. Reset to try an aligned allocation
> - * according to the original allocation specification.
> - */
> - args->alignment = alignment;
> - args->alignslop = 0;
> - }
> + args->alignment = 1;
> + args->alignslop = alignment - args->alignment;
>
> - if (ag_only) {
> - error = xfs_alloc_vextent_near_bno(args, ap->blkno);
> - } else {
> + if (!caller_pag)
> + args->pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, ap->blkno));
> + error = xfs_alloc_vextent_exact_bno(args, ap->blkno);
> + if (!caller_pag) {
> + xfs_perag_put(args->pag);
> args->pag = NULL;
> - error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> - ASSERT(args->pag == NULL);
> - args->pag = caller_pag;
> }
> - if (error)
> - return error;
>
> - if (args->fsbno != NULLFSBLOCK)
> - return 0;
> -
> - /*
> - * Aligned allocation failed, so all fallback paths from here drop the
> - * start alignment requirement as we know it will not succeed.
> - */
> - args->alignment = 1;
> - return 0;
> + /* Reset alignment to original specifications. */
> + args->alignment = alignment;
> + args->alignslop = 0;
> + return error;
> }
>
> /*
> @@ -3688,12 +3649,19 @@ xfs_bmap_btalloc_filestreams(
> }
>
> args->minlen = xfs_bmap_select_minlen(ap, args, blen);
> - if (ap->aeof)
> - error = xfs_bmap_btalloc_at_eof(ap, args, blen, true);
> + if (ap->aeof && ap->offset)
> + error = xfs_bmap_btalloc_at_eof(ap, args);
>
> + /* This may be an aligned allocation attempt. */
> if (!error && args->fsbno == NULLFSBLOCK)
> error = xfs_alloc_vextent_near_bno(args, ap->blkno);
>
> + /* Attempt non-aligned allocation if we haven't already. */
> + if (!error && args->fsbno == NULLFSBLOCK && args->alignment > 1) {
> + args->alignment = 1;
> + error = xfs_alloc_vextent_near_bno(args, ap->blkno);
Oops, I just replied to the v2 thread instead of this.
From
https://lore.kernel.org/linux-xfs/20240621203556.GU3058325@frogsfrogsfrogs/
Do we have to zero args->alignslop here?
--D
> + }
> +
> out_low_space:
> /*
> * We are now done with the perag reference for the filestreams
> @@ -3715,7 +3683,6 @@ xfs_bmap_btalloc_best_length(
> struct xfs_bmalloca *ap,
> struct xfs_alloc_arg *args)
> {
> - xfs_extlen_t blen = 0;
> int error;
>
> ap->blkno = XFS_INO_TO_FSB(args->mp, ap->ip->i_ino);
> @@ -3726,23 +3693,33 @@ xfs_bmap_btalloc_best_length(
> * the request. If one isn't found, then adjust the minimum allocation
> * size to the largest space found.
> */
> - error = xfs_bmap_btalloc_select_lengths(ap, args, &blen);
> + error = xfs_bmap_btalloc_select_lengths(ap, args);
> if (error)
> return error;
>
> /*
> - * Don't attempt optimal EOF allocation if previous allocations barely
> - * succeeded due to being near ENOSPC. It is highly unlikely we'll get
> - * optimal or even aligned allocations in this case, so don't waste time
> - * trying.
> + * If we are in low space mode, then optimal allocation will fail so
> + * prepare for minimal allocation and run the low space algorithm
> + * immediately.
> */
> - if (ap->aeof && !(ap->tp->t_flags & XFS_TRANS_LOWMODE)) {
> - error = xfs_bmap_btalloc_at_eof(ap, args, blen, false);
> - if (error || args->fsbno != NULLFSBLOCK)
> - return error;
> + if (ap->tp->t_flags & XFS_TRANS_LOWMODE) {
> + ASSERT(args->fsbno == NULLFSBLOCK);
> + return xfs_bmap_btalloc_low_space(ap, args);
> + }
> +
> + if (ap->aeof && ap->offset)
> + error = xfs_bmap_btalloc_at_eof(ap, args);
> +
> + /* This may be an aligned allocation attempt. */
> + if (!error && args->fsbno == NULLFSBLOCK)
> + error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> +
> + /* Attempt non-aligned allocation if we haven't already. */
> + if (!error && args->fsbno == NULLFSBLOCK && args->alignment > 1) {
> + args->alignment = 1;
> + error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> }
>
> - error = xfs_alloc_vextent_start_ag(args, ap->blkno);
> if (error || args->fsbno != NULLFSBLOCK)
> return error;
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 2fa29d2f004e..c5d220d51757 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -780,7 +780,7 @@ xfs_ialloc_ag_alloc(
> * the exact agbno requirement and increase the alignment
> * instead. It is critical that the total size of the request
> * (len + alignment + slop) does not increase from this point
> - * on, so reset minalignslop to ensure it is not included in
> + * on, so reset alignslop to ensure it is not included in
> * subsequent requests.
> */
> args.alignslop = 0;
> --
> 2.31.1
>
>
