Replace min(max()) with clamp(). No functional change.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/infiniband/hw/mlx5/umr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/umr.c b/drivers/infiniband/hw/mlx5/umr.c
index 4e562e0dd9e1..1a6b0ac5c24d 100644
--- a/drivers/infiniband/hw/mlx5/umr.c
+++ b/drivers/infiniband/hw/mlx5/umr.c
@@ -1013,7 +1013,7 @@ static int _mlx5r_umr_zap_mkey(struct mlx5_ib_mr *mr,
 		 MLX5_IB_UPD_XLT_ATOMIC;
 	max_log_size = get_max_log_entity_size_cap(dev, access_mode);
 	max_page_shift = order_base_2(mr->ibmr.length);
-	max_page_shift = min(max(max_page_shift, page_shift), max_log_size);
+	max_page_shift = clamp(max_page_shift, page_shift, max_log_size);
 	/* Count blocks in units of max_page_shift, we will zap exactly this
 	 * many to make the whole MR non-present.
 	 * Block size must be aligned to MLX5_UMR_FLEX_ALIGNMENT since it may

On Tue, 10 Mar 2026 15:57:28 +0100
Thorsten Blum <thorsten.blum@linux.dev> wrote:

> Replace min(max()) with clamp(). No functional change.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/infiniband/hw/mlx5/umr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/umr.c b/drivers/infiniband/hw/mlx5/umr.c
> index 4e562e0dd9e1..1a6b0ac5c24d 100644
> --- a/drivers/infiniband/hw/mlx5/umr.c
> +++ b/drivers/infiniband/hw/mlx5/umr.c
> @@ -1013,7 +1013,7 @@ static int _mlx5r_umr_zap_mkey(struct mlx5_ib_mr *mr,
>  		 MLX5_IB_UPD_XLT_ATOMIC;
>  	max_log_size = get_max_log_entity_size_cap(dev, access_mode);
>  	max_page_shift = order_base_2(mr->ibmr.length);
> -	max_page_shift = min(max(max_page_shift, page_shift), max_log_size);
> +	max_page_shift = clamp(max_page_shift, page_shift, max_log_size);

Is page_shift absolutely guaranteed to be less than (or equal to)
max_log_size?
clamp() doesn't guarantee the order of the comparisons.
If I read the code correctly it changed a few years back - on a commit
that said it didn't change it!
I think clamp() currently uses the other order, I looked at proposing to
swap it so that clamp(int_var, 0, unsigned_limit) could be valid (returning
0 for negative values).

	David


>  	/* Count blocks in units of max_page_shift, we will zap exactly this
>  	 * many to make the whole MR non-present.
>  	 * Block size must be aligned to MLX5_UMR_FLEX_ALIGNMENT since it may
>

On Tue, Mar 10, 2026 at 09:24:40PM +0000, David Laight wrote:
> On Tue, 10 Mar 2026 15:57:28 +0100
> Thorsten Blum <thorsten.blum@linux.dev> wrote:
> 
> > Replace min(max()) with clamp(). No functional change.
> > 
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  drivers/infiniband/hw/mlx5/umr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/hw/mlx5/umr.c b/drivers/infiniband/hw/mlx5/umr.c
> > index 4e562e0dd9e1..1a6b0ac5c24d 100644
> > --- a/drivers/infiniband/hw/mlx5/umr.c
> > +++ b/drivers/infiniband/hw/mlx5/umr.c
> > @@ -1013,7 +1013,7 @@ static int _mlx5r_umr_zap_mkey(struct mlx5_ib_mr *mr,
> >  		 MLX5_IB_UPD_XLT_ATOMIC;
> >  	max_log_size = get_max_log_entity_size_cap(dev, access_mode);
> >  	max_page_shift = order_base_2(mr->ibmr.length);
> > -	max_page_shift = min(max(max_page_shift, page_shift), max_log_size);
> > +	max_page_shift = clamp(max_page_shift, page_shift, max_log_size);
> 
> Is page_shift absolutely guaranteed to be less than (or equal to)
> max_log_size?

I asked same question here.
https://lore.kernel.org/all/20260310184744.GI12611@unreal/

Thanks

On Tue, Mar 10, 2026 at 03:57:28PM +0100, Thorsten Blum wrote:
> Replace min(max()) with clamp(). No functional change.

Are you certain about that?
For the values to match, page_shift must be guaranteed to remain
less or equal than max_log_size.

Thanks

> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/infiniband/hw/mlx5/umr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/umr.c b/drivers/infiniband/hw/mlx5/umr.c
> index 4e562e0dd9e1..1a6b0ac5c24d 100644
> --- a/drivers/infiniband/hw/mlx5/umr.c
> +++ b/drivers/infiniband/hw/mlx5/umr.c
> @@ -1013,7 +1013,7 @@ static int _mlx5r_umr_zap_mkey(struct mlx5_ib_mr *mr,
>  		 MLX5_IB_UPD_XLT_ATOMIC;
>  	max_log_size = get_max_log_entity_size_cap(dev, access_mode);
>  	max_page_shift = order_base_2(mr->ibmr.length);
> -	max_page_shift = min(max(max_page_shift, page_shift), max_log_size);
> +	max_page_shift = clamp(max_page_shift, page_shift, max_log_size);
>  	/* Count blocks in units of max_page_shift, we will zap exactly this
>  	 * many to make the whole MR non-present.
>  	 * Block size must be aligned to MLX5_UMR_FLEX_ALIGNMENT since it may

On 10. Mar 2026, at 19:47, Leon Romanovsky wrote:
> On Tue, Mar 10, 2026 at 03:57:28PM +0100, Thorsten Blum wrote:
>> Replace min(max()) with clamp(). No functional change.
> 
> Are you certain about that?
> For the values to match, page_shift must be guaranteed to remain
> less or equal than max_log_size.

Yeah sorry, I didn't realize this conversion isn't straightforward.

I also just realized that clamp() is defined with if/else semantics in
linux/minmax.h, but uses min(max()) in tools/include/linux/kernel.h,
which seems confusing to me.

Nonetheless, in this case it doesn't seem safe to replace min(max())
with clamp(), and I've learned something today. Thanks for catching it,
and please drop this patch.