xfs: replace min & max with clamp() in xfs_max_open_zones()

[PATCH] xfs: replace min & max with clamp() in xfs_max_open_zones()

Posted by George Hu 2 months, 3 weeks ago

Refactor the xfs_max_open_zones() function by replacing the usage
of min() and max() macro with clamp() to simplify the code and
improve readability.

Signed-off-by: George Hu <integral@archlinux.org>
---
 fs/xfs/xfs_zone_alloc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 01315ed75502..58997afb1a14 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -1133,9 +1133,7 @@ xfs_max_open_zones(
 	/*
 	 * Cap the max open limit to 1/4 of available space
 	 */
-	max_open = min(max_open, mp->m_sb.sb_rgcount / 4);
-
-	return max(XFS_MIN_OPEN_ZONES, max_open);
+	return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);
 }
 
 /*
-- 
2.50.0

Re: [PATCH] xfs: replace min & max with clamp() in xfs_max_open_zones()

Posted by John Garry 2 months, 3 weeks ago

On 12/07/2025 15:57, George Hu wrote:
> Refactor the xfs_max_open_zones() function by replacing the usage
> of min() and max() macro with clamp() to simplify the code and
> improve readability.
> 
> Signed-off-by: George Hu <integral@archlinux.org>

FWIW,

Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   fs/xfs/xfs_zone_alloc.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 01315ed75502..58997afb1a14 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -1133,9 +1133,7 @@ xfs_max_open_zones(
>   	/*
>   	 * Cap the max open limit to 1/4 of available space
>   	 */

Maybe you can keep this comment, but it was pretty much describing the 
code and not explaining the rationale.

> -	max_open = min(max_open, mp->m_sb.sb_rgcount / 4);
> -
> -	return max(XFS_MIN_OPEN_ZONES, max_open);
> +	return clamp(max_open, XFS_MIN_OPEN_ZONES, mp->m_sb.sb_rgcount / 4);
>   }
>   
>   /*

Re: [PATCH] xfs: replace min & max with clamp() in xfs_max_open_zones()

Posted by Christoph Hellwig 2 months, 3 weeks ago

On Mon, Jul 14, 2025 at 08:58:59AM +0100, John Garry wrote:
> > @@ -1133,9 +1133,7 @@ xfs_max_open_zones(
> >   	/*
> >   	 * Cap the max open limit to 1/4 of available space
> >   	 */
> 
> Maybe you can keep this comment, but it was pretty much describing the code
> and not explaining the rationale.

Let me send an incremental patch to explain the rational as long as I
can still remember it :)

Re: [PATCH] xfs: replace min & max with clamp() in xfs_max_open_zones()

Posted by Carlos Maiolino 2 months, 3 weeks ago

On Mon, Jul 14, 2025 at 03:59:27AM -0700, Christoph Hellwig wrote:
> On Mon, Jul 14, 2025 at 08:58:59AM +0100, John Garry wrote:
> > > @@ -1133,9 +1133,7 @@ xfs_max_open_zones(
> > >   	/*
> > >   	 * Cap the max open limit to 1/4 of available space
> > >   	 */
> >
> > Maybe you can keep this comment, but it was pretty much describing the code
> > and not explaining the rationale.
> 
> Let me send an incremental patch to explain the rational as long as I
> can still remember it :)

Sounds good, I'll wait and merge both together.

> 
>

Re: [PATCH] xfs: replace min & max with clamp() in xfs_max_open_zones()

Posted by Christoph Hellwig 2 months, 3 weeks ago

On Sat, Jul 12, 2025 at 10:57:41PM +0800, George Hu wrote:
> Refactor the xfs_max_open_zones() function by replacing the usage
> of min() and max() macro with clamp() to simplify the code and
> improve readability.

Nit: you can use up to 73 characters in each commit message line:

Refactor the xfs_max_open_zones() function by replacing the usage of
min() and max() macro with clamp() to simplify the code and improve
readability.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>