| Message ID | 20231214063438.290538-19-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/19] xfs: consider minlen sized extents in xfs_rtallocate_extent_block | expand |
On Thu, Dec 14, 2023 at 07:34:37AM +0100, Christoph Hellwig wrote: > There are currently multiple levels of fall back if an RT allocation > can not be satisfied: > > 1) xfs_rtallocate_extent extents the minlen and reduces the maxlen due ^^^^^^^ extends? > to the extent size hint. If that can't be done, it return -ENOSPC > and let's xfs_bmap_rtalloc retry, which then not only drops the > extent size hint based alignment, but also the minlen adjustment > 2) if xfs_rtallocate_extent gets -ENOSPC from the underlying functions, > it only drops the extent size hint based alignment and retries > 3) if that still does not succeed, xfs_rtallocate_extent drops the > extent size hint (which is a complex no-op at this point) and the > minlen using the same code as (1) above > 4) if that still doesn't success and the caller wanted an allocation > near a blkno, drop that blkno hint. > > The handling in 1 is rather inefficient as we could just drop the > alignment and continue, and 2/3 interact in really weird ways due to > the duplicate policy. > > Move aligning the min and maxlen out of xfs_rtallocate_extent and into > a helper called directly by xfs_bmap_rtalloc. This allows just > continuing with the allocation if we have to drop the alignment instead > of going through the retry loop and also dropping the perfectly the > minlen adjustment that didn't cause the problem, and then just use "...dropping the perfectly *usable* minlen adjustment..." ? > a single retry that drops both the minlen and alignment requirement > when we really are out of space, thus consolidating cases (2) and (3) > above. How can we drop the minlen requirement, won't that result in undersize mapping allocations? Or is the subtlety here that for realtime files, that's ok because we never have forced multi-rtx allocations like we do for the data device? --D > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_rtalloc.c | 58 ++++++++++++++++++++++++++------------------ > 1 file changed, 35 insertions(+), 23 deletions(-) > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c > index 595740d18dc4c3..16255617629ef5 100644 > --- a/fs/xfs/xfs_rtalloc.c > +++ b/fs/xfs/xfs_rtalloc.c > @@ -1088,21 +1088,6 @@ xfs_rtallocate_extent( > ASSERT(xfs_isilocked(args.mp->m_rbmip, XFS_ILOCK_EXCL)); > ASSERT(minlen > 0 && minlen <= maxlen); > > - /* > - * If prod is set then figure out what to do to minlen and maxlen. > - */ > - if (prod > 1) { > - xfs_rtxlen_t i; > - > - if ((i = maxlen % prod)) > - maxlen -= i; > - if ((i = minlen % prod)) > - minlen += prod - i; > - if (maxlen < minlen) > - return -ENOSPC; > - } > - > -retry: > if (start == 0) { > error = xfs_rtallocate_extent_size(&args, minlen, > maxlen, len, prod, rtx); > @@ -1111,13 +1096,8 @@ xfs_rtallocate_extent( > maxlen, len, prod, rtx); > } > xfs_rtbuf_cache_relse(&args); > - if (error) { > - if (error == -ENOSPC && prod > 1) { > - prod = 1; > - goto retry; > - } > + if (error) > return error; > - } > > /* > * If it worked, update the superblock. > @@ -1348,6 +1328,35 @@ xfs_rtpick_extent( > return 0; > } > > +static void > +xfs_rtalloc_align_minmax( > + xfs_rtxlen_t *raminlen, > + xfs_rtxlen_t *ramaxlen, > + xfs_rtxlen_t *prod) > +{ > + xfs_rtxlen_t newmaxlen = *ramaxlen; > + xfs_rtxlen_t newminlen = *raminlen; > + xfs_rtxlen_t slack; > + > + slack = newmaxlen % *prod; > + if (slack) > + newmaxlen -= slack; > + slack = newminlen % *prod; > + if (slack) > + newminlen += *prod - slack; > + > + /* > + * If adjusting for extent size hint alignment produces an invalid > + * min/max len combination, go ahead without it. > + */ > + if (newmaxlen < newminlen) { > + *prod = 1; > + return; > + } > + *ramaxlen = newmaxlen; > + *raminlen = newminlen; > +} > + > int > xfs_bmap_rtalloc( > struct xfs_bmalloca *ap) > @@ -1430,10 +1439,13 @@ xfs_bmap_rtalloc( > * perfectly aligned, otherwise it will just get us in trouble. > */ > div_u64_rem(ap->offset, align, &mod); > - if (mod || ap->length % align) > + if (mod || ap->length % align) { > prod = 1; > - else > + } else { > prod = xfs_extlen_to_rtxlen(mp, align); > + if (prod > 1) > + xfs_rtalloc_align_minmax(&raminlen, &ralen, &prod); > + } > > error = xfs_rtallocate_extent(ap->tp, start, raminlen, ralen, &ralen, > ap->wasdel, prod, &rtx); > -- > 2.39.2 > >
On Thu, Dec 14, 2023 at 01:32:21PM -0800, Darrick J. Wong wrote: > > 1) xfs_rtallocate_extent extents the minlen and reduces the maxlen due > > ^^^^^^^ extends? Yes. I'm definitively talking about extents too much in my life :) > > Move aligning the min and maxlen out of xfs_rtallocate_extent and into > > a helper called directly by xfs_bmap_rtalloc. This allows just > > continuing with the allocation if we have to drop the alignment instead > > of going through the retry loop and also dropping the perfectly the > > minlen adjustment that didn't cause the problem, and then just use > > "...dropping the perfectly *usable* minlen adjustment..." ? > > > a single retry that drops both the minlen and alignment requirement > > when we really are out of space, thus consolidating cases (2) and (3) > > above. > > How can we drop the minlen requirement, won't that result in undersize > mapping allocations? Or is the subtlety here that for realtime files, > that's ok because we never have forced multi-rtx allocations like we do > for the data device? The rtalloc minlen is different from the bmap minlen. The bmap minlen is always 1 except for metadata XFS_BMAPI_CONTIG allocations, which obviosuly can't happen for RT allocations. The rtalloc minlen starts out as a single rtextent and is increases when we adjust the physical allocation location to better align with the previous extent.
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c index 595740d18dc4c3..16255617629ef5 100644 --- a/fs/xfs/xfs_rtalloc.c +++ b/fs/xfs/xfs_rtalloc.c @@ -1088,21 +1088,6 @@ xfs_rtallocate_extent( ASSERT(xfs_isilocked(args.mp->m_rbmip, XFS_ILOCK_EXCL)); ASSERT(minlen > 0 && minlen <= maxlen); - /* - * If prod is set then figure out what to do to minlen and maxlen. - */ - if (prod > 1) { - xfs_rtxlen_t i; - - if ((i = maxlen % prod)) - maxlen -= i; - if ((i = minlen % prod)) - minlen += prod - i; - if (maxlen < minlen) - return -ENOSPC; - } - -retry: if (start == 0) { error = xfs_rtallocate_extent_size(&args, minlen, maxlen, len, prod, rtx); @@ -1111,13 +1096,8 @@ xfs_rtallocate_extent( maxlen, len, prod, rtx); } xfs_rtbuf_cache_relse(&args); - if (error) { - if (error == -ENOSPC && prod > 1) { - prod = 1; - goto retry; - } + if (error) return error; - } /* * If it worked, update the superblock. @@ -1348,6 +1328,35 @@ xfs_rtpick_extent( return 0; } +static void +xfs_rtalloc_align_minmax( + xfs_rtxlen_t *raminlen, + xfs_rtxlen_t *ramaxlen, + xfs_rtxlen_t *prod) +{ + xfs_rtxlen_t newmaxlen = *ramaxlen; + xfs_rtxlen_t newminlen = *raminlen; + xfs_rtxlen_t slack; + + slack = newmaxlen % *prod; + if (slack) + newmaxlen -= slack; + slack = newminlen % *prod; + if (slack) + newminlen += *prod - slack; + + /* + * If adjusting for extent size hint alignment produces an invalid + * min/max len combination, go ahead without it. + */ + if (newmaxlen < newminlen) { + *prod = 1; + return; + } + *ramaxlen = newmaxlen; + *raminlen = newminlen; +} + int xfs_bmap_rtalloc( struct xfs_bmalloca *ap) @@ -1430,10 +1439,13 @@ xfs_bmap_rtalloc( * perfectly aligned, otherwise it will just get us in trouble. */ div_u64_rem(ap->offset, align, &mod); - if (mod || ap->length % align) + if (mod || ap->length % align) { prod = 1; - else + } else { prod = xfs_extlen_to_rtxlen(mp, align); + if (prod > 1) + xfs_rtalloc_align_minmax(&raminlen, &ralen, &prod); + } error = xfs_rtallocate_extent(ap->tp, start, raminlen, ralen, &ralen, ap->wasdel, prod, &rtx);
There are currently multiple levels of fall back if an RT allocation can not be satisfied: 1) xfs_rtallocate_extent extents the minlen and reduces the maxlen due to the extent size hint. If that can't be done, it return -ENOSPC and let's xfs_bmap_rtalloc retry, which then not only drops the extent size hint based alignment, but also the minlen adjustment 2) if xfs_rtallocate_extent gets -ENOSPC from the underlying functions, it only drops the extent size hint based alignment and retries 3) if that still does not succeed, xfs_rtallocate_extent drops the extent size hint (which is a complex no-op at this point) and the minlen using the same code as (1) above 4) if that still doesn't success and the caller wanted an allocation near a blkno, drop that blkno hint. The handling in 1 is rather inefficient as we could just drop the alignment and continue, and 2/3 interact in really weird ways due to the duplicate policy. Move aligning the min and maxlen out of xfs_rtallocate_extent and into a helper called directly by xfs_bmap_rtalloc. This allows just continuing with the allocation if we have to drop the alignment instead of going through the retry loop and also dropping the perfectly the minlen adjustment that didn't cause the problem, and then just use a single retry that drops both the minlen and alignment requirement when we really are out of space, thus consolidating cases (2) and (3) above. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_rtalloc.c | 58 ++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 23 deletions(-)