Re: [PATCH 4/6] xfs: limit maxlen based on available space in xfs_rtallocate_extent_near()

Darrick J. Wong
2023-07-12T23:02:03Z

On Tue, Jun 20, 2023 at 02:32:14PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> xfs_rtallocate_extent_near() calls xfs_rtallocate_extent_block() with
> the minlen and maxlen that were passed to it.
> xfs_rtallocate_extent_block() then scans the bitmap block looking for a
> free range of size maxlen. If there is none, it has to scan the whole
> bitmap block before returning the largest range of at least size minlen.
> For a fragmented realtime device and a large allocation request, it's
> almost certain that this will have to search the whole bitmap block,
> leading to high CPU usage.
>
> However, the realtime summary tells us the maximum size available in the
> bitmap block. We can limit the search in xfs_rtallocate_extent_block()
> to that size and often stop before scanning the whole bitmap block.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> fs/xfs/xfs_rtalloc.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index ba7d42e0090f..d079dfb77c73 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -488,6 +488,8 @@ xfs_rtallocate_extent_near(
> * allocating one.
> */
> if (maxlog >= 0) {
> + xfs_extlen_t maxavail =
> + min(maxlen, ((xfs_extlen_t)1 << (maxlog + 1)) - 1);

There can be up to 2^52rtx (realtime extents) in the filesystem, right?
xfs_extlen_t is a u32, which will overflow this calculation if the
realtime volume is seriously huge. IOWs, doesn't this need to be:

xfs_extlen_t maxavail = max_t(xfs_rtblock_t, maxlen,
(1ULL << (maxlog + 1)) - 1);

(The rest of the patch looks ok)

--D

> /*
> * On the positive side of the starting location.
> */
> @@ -497,7 +499,7 @@ xfs_rtallocate_extent_near(
> * this block.
> */
> error = xfs_rtallocate_extent_block(mp, tp,
> - bbno + i, minlen, maxlen, len, &n,
> + bbno + i, minlen, maxavail, len, &n,
> rtbufc, prod, &r);
> if (error) {
> return error;
> @@ -542,7 +544,7 @@ xfs_rtallocate_extent_near(
> if (maxlog >= 0)
> continue;
> error = xfs_rtallocate_extent_block(mp,
> - tp, bbno + j, minlen, maxlen,
> + tp, bbno + j, minlen, maxavail,
> len, &n, rtbufc, prod, &r);
> if (error) {
> return error;
> @@ -564,7 +566,7 @@ xfs_rtallocate_extent_near(
> * that we found.
> */
> error = xfs_rtallocate_extent_block(mp, tp,
> - bbno + i, minlen, maxlen, len, &n,
> + bbno + i, minlen, maxavail, len, &n,
> rtbufc, prod, &r);
> if (error) {
> return error;
> --
> 2.41.0
>