From: NeilBrown <neil@brown.name>

A recent change to clamp_t() in 6.1.y caused fs/nfsd/nfs4state.c to fail
to compile with gcc-9.

The code was written with the assumption that when "max < min",
   clamp(val, min, max)
would return max.  This assumption is not documented as an API promise
and the change cause a compile failure if it could be statically
determined that "max < min".

The relevant code was no longer present upstream when the clamp() change
landed there, so there is no upstream change to backport.

As there is no clear case that the code is functioning incorrectly, the
patch aims to restore the behaviour to exactly that before the clamp
change, and to match what compilers other than gcc-9 produce.

clamp_t(type,v,min,max) is replaced with
  __clamp((type)v, (type)min, (type)max)

Some of those type casts are unnecessary but they are included to make
the code obviously correct.
(__clamp() is the same as clamp(), but without the static API usage
test).

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
Fixes: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/nfsd/nfs4state.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 08bfc2b29b65..d485a140d36d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1822,8 +1822,9 @@ static u32 nfsd4_get_drc_mem(struct nfsd4_channel_attrs *ca, struct nfsd_net *nn
 	 */
 	scale_factor = max_t(unsigned int, 8, nn->nfsd_serv->sv_nrthreads);
 
-	avail = clamp_t(unsigned long, avail, slotsize,
-			total_avail/scale_factor);
+	avail = __clamp((unsigned long)avail,
+			(unsigned long)slotsize,
+			(unsigned long)(total_avail/scale_factor));
 	num = min_t(int, num, avail / slotsize);
 	num = max_t(int, num, 1);
 	nfsd_drc_mem_used += num * slotsize;
-- 
2.50.0.107.gf914562f5916.dirty

Hi Neil -

On 11/9/25 4:45 PM, NeilBrown wrote:
> 
> From: NeilBrown <neil@brown.name>
> 
> A recent change to clamp_t() in 6.1.y caused fs/nfsd/nfs4state.c to fail
> to compile with gcc-9.

I have a comment on merge process:

Reported on 6.1.y, but might be present in other LTS releases, since
2030ca560c5f exists in every LTS kernel since v5.4.y.

At least, my understanding of the stable rules is that they prefer this
kind of patch be applied to all relevant LTS kernels. I strongly prefer
that NFSD experts review and test this change /before/ it is merged,
since nfsd4_get_drc_mem() is part of the NFSv4.1 session slot
implementation, and since in this case we don't get the benefit of
/any/ soak time in linux-next or an upstream -rc release.

So IMHO this patch needs to target v6.12.y, not v6.1.y, and it should be
marked

Fixes: 2030ca560c5f ("nfsd: degraded slot-count more gracefully as
allocation nears exhaustion.")

(Since the patched code hasn't changed in many years, I think the final
patch ought to apply cleanly to both 6.12.y and 6.1.y).

I need to take the fix into nfsd-6.12.y and run NFSD CI against it, then
it can be sent along to stable@, and they will put it back into the
older LTS kernels for us.


> The code was written with the assumption that when "max < min",
>    clamp(val, min, max)
> would return max.  This assumption is not documented as an API promise
> and the change cause a compile failure if it could be statically
> determined that "max < min".
> 
> The relevant code was no longer present upstream when the clamp() change
> landed there, so there is no upstream change to backport.
> 
> As there is no clear case that the code is functioning incorrectly, the
> patch aims to restore the behaviour to exactly that before the clamp
> change, and to match what compilers other than gcc-9 produce.

> clamp_t(type,v,min,max) is replaced with
>   __clamp((type)v, (type)min, (type)max)
> 
> Some of those type casts are unnecessary but they are included to make
> the code obviously correct.
> (__clamp() is the same as clamp(), but without the static API usage
> test).
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
> Fixes: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")

Stable-dep-of: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the
lo < hi test in clamp()")

might be more appropriate.


> Signed-off-by: NeilBrown <neil@brown.name>

-- 
Chuck Lever

On Mon, 10 Nov 2025, Chuck Lever wrote:
> Hi Neil -
> 
> On 11/9/25 4:45 PM, NeilBrown wrote:
> > 
> > From: NeilBrown <neil@brown.name>
> > 
> > A recent change to clamp_t() in 6.1.y caused fs/nfsd/nfs4state.c to fail
> > to compile with gcc-9.
> 
> I have a comment on merge process:
> 
> Reported on 6.1.y, but might be present in other LTS releases, since
> 2030ca560c5f exists in every LTS kernel since v5.4.y.

I thought this might be likely but I didn't have enough motivation to check.

> 
> At least, my understanding of the stable rules is that they prefer this
> kind of patch be applied to all relevant LTS kernels. I strongly prefer
> that NFSD experts review and test this change /before/ it is merged,
> since nfsd4_get_drc_mem() is part of the NFSv4.1 session slot
> implementation, and since in this case we don't get the benefit of
> /any/ soak time in linux-next or an upstream -rc release.

The patch is deliberately written to transparent without requiring any
(export or otherwise) understand of the NFS or even of the code being
changed.
It purely removes the BUILD_BUG_ON().

> 
> So IMHO this patch needs to target v6.12.y, not v6.1.y, and it should be
> marked

Can I leave the process management to you.
Though as you say later, the same patch should apply equally to both.

> 
> Fixes: 2030ca560c5f ("nfsd: degraded slot-count more gracefully as
> allocation nears exhaustion.")

There is no evidence that patch is broken so it is hard to justify
saying that we fixed it.  But I honestly don't care.

> 
> (Since the patched code hasn't changed in many years, I think the final
> patch ought to apply cleanly to both 6.12.y and 6.1.y).
> 
> I need to take the fix into nfsd-6.12.y and run NFSD CI against it, then
> it can be sent along to stable@, and they will put it back into the
> older LTS kernels for us.
> 
> 
> > The code was written with the assumption that when "max < min",
> >    clamp(val, min, max)
> > would return max.  This assumption is not documented as an API promise
> > and the change cause a compile failure if it could be statically
> > determined that "max < min".
> > 
> > The relevant code was no longer present upstream when the clamp() change
> > landed there, so there is no upstream change to backport.
> > 
> > As there is no clear case that the code is functioning incorrectly, the
> > patch aims to restore the behaviour to exactly that before the clamp
> > change, and to match what compilers other than gcc-9 produce.
> 
> > clamp_t(type,v,min,max) is replaced with
> >   __clamp((type)v, (type)min, (type)max)
> > 
> > Some of those type casts are unnecessary but they are included to make
> > the code obviously correct.
> > (__clamp() is the same as clamp(), but without the static API usage
> > test).
> > 
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
> > Fixes: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> 
> Stable-dep-of: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the
> lo < hi test in clamp()")
> 

I haven't come across Stable-dep-of before.  I can't find it in
Documentation.  Looking at some examples I guess it makes sense.
Except that Stable-dep-of normally comes before, and Fixes normally
comes after the target...

Thanks,
NeilBrown


> might be more appropriate.
> 
> 
> > Signed-off-by: NeilBrown <neil@brown.name>
> 
> -- 
> Chuck Lever
> 

On 11/10/25 6:18 PM, NeilBrown wrote:
>> So IMHO this patch needs to target v6.12.y, not v6.1.y, and it should be
>> marked
> Can I leave the process management to you.

Yes.

-- 
Chuck Lever

On Mon, 10 Nov 2025 08:45:35 +1100
NeilBrown <neilb@ownmail.net> wrote:

> From: NeilBrown <neil@brown.name>
> 
> A recent change to clamp_t() in 6.1.y caused fs/nfsd/nfs4state.c to fail
> to compile with gcc-9.
> 
> The code was written with the assumption that when "max < min",
>    clamp(val, min, max)
> would return max.  This assumption is not documented as an API promise
> and the change cause a compile failure if it could be statically
> determined that "max < min".
> 
> The relevant code was no longer present upstream when the clamp() change
> landed there, so there is no upstream change to backport.
> 
> As there is no clear case that the code is functioning incorrectly, the
> patch aims to restore the behaviour to exactly that before the clamp
> change, and to match what compilers other than gcc-9 produce.
> 
> clamp_t(type,v,min,max) is replaced with
>   __clamp((type)v, (type)min, (type)max)
> 
> Some of those type casts are unnecessary but they are included to make
> the code obviously correct.

I beg to differ.
If the values are all positive the casts aren't needed.
If and one the values are ever negative the code is broken.
(I think that is a bug in the old version without the initial check
that sets 'total_avail' to zero.)

> (__clamp() is the same as clamp(), but without the static API usage
> test).

And it is really an internal define that shouldn't be used outside
on minmax.h itself.

Replace the clamp() with the actual comparisons you want:
The code should always have been:
	if (avail < slotsize)
		avail = slotsize;
	else if (avail > total_avail/scale_factor)
		avail = total_avail/scale_factor;
(The compiler will CSE to two divides.)
I think that actually works best if both 'avail' and 'slotsize' are signed.
Then it doesn't matter if 'avail' is negative - and lots of tests above it
can be deleted (as well as the max() later then ensures it isn't zero).

But for bug compatibility swap the order of the tests over:
	if (avail > total_avail/scale_factor)
		avail = total_avail/scale_factor;
	else if (avail < slotsize)
		avail = slotsize;

    David

> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
> Fixes: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/nfsd/nfs4state.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 08bfc2b29b65..d485a140d36d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1822,8 +1822,9 @@ static u32 nfsd4_get_drc_mem(struct nfsd4_channel_attrs *ca, struct nfsd_net *nn
>  	 */
>  	scale_factor = max_t(unsigned int, 8, nn->nfsd_serv->sv_nrthreads);
>  
> -	avail = clamp_t(unsigned long, avail, slotsize,
> -			total_avail/scale_factor);
> +	avail = __clamp((unsigned long)avail,
> +			(unsigned long)slotsize,
> +			(unsigned long)(total_avail/scale_factor));
>  	num = min_t(int, num, avail / slotsize);
>  	num = max_t(int, num, 1);
>  	nfsd_drc_mem_used += num * slotsize;