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

["Darrick J. Wong" <djwong@kernel.org>]

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
>