On Thu, Dec 14, 2023 at 07:34:28AM +0100, Christoph Hellwig wrote:
> Clean up the logical in xfs_bmap_rtalloc that tries to find a rtextent
> to start the search from by using a separate variable for the hint, not
> calling xfs_bmap_adjacent when we want to ignore the locality and avoid
> an extra roundtrip converting between block numbers and RT extent
> numbers.
Ahah, I had wondered about that...
> As a side-effect this doesn't pointlessly call xfs_rtpick_extent and
> increment the start rtextent hint if we are going to ignore the result
> anyway.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_rtalloc.c | 35 +++++++++++++++--------------------
> 1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 158a631379378e..2ce3bcf4b84b76 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1393,7 +1393,8 @@ xfs_bmap_rtalloc(
> {
> struct xfs_mount *mp = ap->ip->i_mount;
> xfs_fileoff_t orig_offset = ap->offset;
> - xfs_rtxnum_t rtx;
> + xfs_rtxnum_t start; /* allocation hint rtextent no */
> + xfs_rtxnum_t rtx; /* actually allocated rtextent no */
> xfs_rtxlen_t prod = 0; /* product factor for allocators */
> xfs_extlen_t mod = 0; /* product factor for allocators */
> xfs_rtxlen_t ralen = 0; /* realtime allocation length */
> @@ -1454,30 +1455,24 @@ xfs_bmap_rtalloc(
> rtlocked = true;
> }
>
> - /*
> - * If it's an allocation to an empty file at offset 0,
> - * pick an extent that will space things out in the rt area.
> - */
> - if (ap->eof && ap->offset == 0) {
> - error = xfs_rtpick_extent(mp, ap->tp, ralen, &rtx);
> + if (ignore_locality) {
> + start = 0;
> + } else if (xfs_bmap_adjacent(ap)) {
> + start = xfs_rtb_to_rtx(mp, ap->blkno);
> + } else if (ap->eof && ap->offset == 0) {
> + /*
> + * If it's an allocation to an empty file at offset 0, pick an
> + * extent that will space things out in the rt area.
> + */
> + error = xfs_rtpick_extent(mp, ap->tp, ralen, &start);
> if (error)
> return error;
> - ap->blkno = xfs_rtx_to_rtb(mp, rtx);
> } else {
> - ap->blkno = 0;
> + start = 0;
> }
It took me a while to wrap my head around the dual "start = 0" clauses
here, but eventually I figured out that the first one is from below, and
this one here is merely the continuation of the "!adjacent" and
"!emptyfileatoffset0" cases.
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
>
> - xfs_bmap_adjacent(ap);
> -
> - /*
> - * Realtime allocation, done through xfs_rtallocate_extent.
> - */
> - if (ignore_locality)
> - rtx = 0;
> - else
> - rtx = xfs_rtb_to_rtx(mp, ap->blkno);
> raminlen = max_t(xfs_rtxlen_t, 1, xfs_extlen_to_rtxlen(mp, minlen));
> - error = xfs_rtallocate_extent(ap->tp, rtx, raminlen, ralen, &ralen,
> + error = xfs_rtallocate_extent(ap->tp, start, raminlen, ralen, &ralen,
> ap->wasdel, prod, &rtx);
> if (error == -ENOSPC) {
> if (align > mp->m_sb.sb_rextsize) {
> @@ -1494,7 +1489,7 @@ xfs_bmap_rtalloc(
> goto retry;
> }
>
> - if (!ignore_locality && ap->blkno != 0) {
> + if (!ignore_locality && start != 0) {
> /*
> * If we can't allocate near a specific rt extent, try
> * again without locality criteria.
> --
> 2.39.2
>
>
