Re: [PATCH 6/6] xfs: don't look for end of extent further than necessary in xfs_rtallocate_extent_near()

Darrick J. Wong
2023-08-01T23:40:17Z

On Tue, Jun 20, 2023 at 02:32:16PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> As explained in the previous commit, xfs_rtallocate_extent_near() looks
> for the end of a free extent when searching backwards from the target
> bitmap block. Since the previous commit, it searches from the last
> bitmap block it checked to the bitmap block containing the start of the
> extent.
>
> This may still be more than necessary, since the free extent may not be
> that long. We know the maximum size of the free extent from the realtime
> summary. Use that to compute how many bitmap blocks we actually need to
> check.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> fs/xfs/xfs_rtalloc.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 4d9d0be2e616..2e2eb7c4a648 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -517,12 +517,29 @@ xfs_rtallocate_extent_near(
> * On the negative side of the starting location.
> */
> else { /* i < 0 */
> + int maxblocks;
> +
> /*
> - * Loop backwards through the bitmap blocks from
> - * where we last checked up to where we are now.
> - * There should be an extent which ends in this
> - * bitmap block and is long enough.
> + * Loop backwards to find the end of the extent
> + * we found in the realtime summary.
> + *
> + * maxblocks is the maximum possible number of
> + * bitmap blocks from the start of the extent to
> + * the end of the extent.
> */
> + if (maxlog == 0)
> + maxblocks = 0;
> + else if (maxlog < mp->m_blkbit_log)
> + maxblocks = 1;
> + else
> + maxblocks = 2 << (maxlog - mp->m_blkbit_log);
> + /*
> + * We need to check bbno + i + maxblocks down to
> + * bbno + i. We already checked bbno down to
> + * bbno + j + 1, so we don't need to check those
> + * again.
> + */
> + j = min(i + maxblocks, j);

Makes sense now with a fresher head...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

What does the xfsprogs version of this patchset look like?

--D

> for (; j >= i; j--) {
> error = xfs_rtallocate_extent_block(mp,
> tp, bbno + j, minlen, maxavail,
> --
> 2.41.0
>