| Message ID | 20240801163057.3981192-5-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | forcealign for xfs | expand |
On Thu, Aug 01, 2024 at 04:30:47PM +0000, John Garry wrote: > From: Dave Chinner <dchinner@redhat.com> > > 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@redhat.com> > Signed-off-by: John Garry <john.g.garry@oracle.com> > --- > 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 > >
On Tue, Aug 06, 2024 at 11:58:53AM -0700, Darrick J. Wong wrote: > On Thu, Aug 01, 2024 at 04:30:47PM +0000, John Garry wrote: > > @@ -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? No. It should always be zero here to begin with. It is the responsibility of the allocation attempt that modifies args->alignment and args->alignslop to reset them to original values on allocation failure. The only places we use alignslop are xfs_bmap_btalloc_at_eof() and xfs_ialloc_ag_alloc(). They both zero it and reset args->alignment on allocation failure before falling through to the next allocation attempt. -Dave.
On Wed, Aug 07, 2024 at 10:00:46AM +1000, Dave Chinner wrote: > On Tue, Aug 06, 2024 at 11:58:53AM -0700, Darrick J. Wong wrote: > > On Thu, Aug 01, 2024 at 04:30:47PM +0000, John Garry wrote: > > > @@ -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? > > No. It should always be zero here to begin with. It is the > responsibility of the allocation attempt that modifies > args->alignment and args->alignslop to reset them to original values > on allocation failure. > > The only places we use alignslop are xfs_bmap_btalloc_at_eof() and > xfs_ialloc_ag_alloc(). They both zero it and reset args->alignment > on allocation failure before falling through to the next allocation > attempt. Aha, that's even in this patch. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > -Dave. > -- > Dave Chinner > david@fromorbit.com >
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); + } + 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;