stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem()
@ 2025-11-14 21:19 Chuck Lever
  2025-11-14 22:14 ` David Laight
  2025-12-01 19:28 ` Chuck Lever
  0 siblings, 2 replies; 4+ messages in thread
From: Chuck Lever @ 2025-11-14 21:19 UTC (permalink / raw)
  To: Andrew Morten, david.laight.linux, NeilBrown, Jeff Layton,
	Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, linux-kernel, stable, speedcracker

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 in nfsd4_get_drc_mem() 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 caused a compile failure if it could be statically
determined that "max < min".

The relevant code was no longer present upstream when commit 1519fbc8832b
("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
landed there, so there is no upstream change to nfsd4_get_drc_mem() to
backport.

There is no clear case that the existing code in nfsd4_get_drc_mem()
is functioning incorrectly. The goal of this patch is to permit the clean
application of commit 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for
the lo < hi test in clamp()"), and any commits that depend on it, to LTS
kernels without affecting the ability to compile those kernels. This is
done by open-coding the __clamp() macro sans the built-in type checking.

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

Changes since Neil's post:
* Editorial changes to the commit message
* Attempt to address David's review comments
* Applied to linux-6.12.y, passed NFSD upstream CI suite

This patch is intended to be applied to linux-6.12.y, and should
apply cleanly to other LTS kernels since nfsd4_get_drc_mem hasn't
changed since v5.4.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7b0fabf8c657..41545933dd18 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1983,8 +1983,10 @@ 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);
+	if (avail > total_avail / scale_factor)
+		avail = total_avail / scale_factor;
+	else if (avail < slotsize)
+		avail = slotsize;
 	num = min_t(int, num, avail / slotsize);
 	num = max_t(int, num, 1);
 	nfsd_drc_mem_used += num * slotsize;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem()
  2025-11-14 21:19 [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem() Chuck Lever
@ 2025-11-14 22:14 ` David Laight
  2025-12-01 19:28 ` Chuck Lever
  1 sibling, 0 replies; 4+ messages in thread
From: David Laight @ 2025-11-14 22:14 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Andrew Morten, NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo,
	Tom Talpey, linux-nfs, linux-kernel, stable, speedcracker

On Fri, 14 Nov 2025 16:19:22 -0500
Chuck Lever <cel@kernel.org> 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 in nfsd4_get_drc_mem() 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 caused a compile failure if it could be statically
> determined that "max < min".
> 
> The relevant code was no longer present upstream when commit 1519fbc8832b
> ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> landed there, so there is no upstream change to nfsd4_get_drc_mem() to
> backport.
> 
> There is no clear case that the existing code in nfsd4_get_drc_mem()
> is functioning incorrectly. The goal of this patch is to permit the clean
> application of commit 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for
> the lo < hi test in clamp()"), and any commits that depend on it, to LTS
> kernels without affecting the ability to compile those kernels. This is
> done by open-coding the __clamp() macro sans the built-in type checking.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
> Signed-off-by: NeilBrown <neil@brown.name>
> Stable-dep-of: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Reviewed_by: David Laight <david.laight.linux@gmail.com>
> ---
>  fs/nfsd/nfs4state.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Changes since Neil's post:
> * Editorial changes to the commit message
> * Attempt to address David's review comments
> * Applied to linux-6.12.y, passed NFSD upstream CI suite
> 
> This patch is intended to be applied to linux-6.12.y, and should
> apply cleanly to other LTS kernels since nfsd4_get_drc_mem hasn't
> changed since v5.4.
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7b0fabf8c657..41545933dd18 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1983,8 +1983,10 @@ 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);
> +	if (avail > total_avail / scale_factor)
> +		avail = total_avail / scale_factor;
> +	else if (avail < slotsize)
> +		avail = slotsize;
>  	num = min_t(int, num, avail / slotsize);
>  	num = max_t(int, num, 1);
>  	nfsd_drc_mem_used += num * slotsize;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem()
  2025-11-14 21:19 [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem() Chuck Lever
  2025-11-14 22:14 ` David Laight
@ 2025-12-01 19:28 ` Chuck Lever
  2025-12-02 15:51   ` Greg KH
  1 sibling, 1 reply; 4+ messages in thread
From: Chuck Lever @ 2025-12-01 19:28 UTC (permalink / raw)
  To: Greg KH, Sasha Levin
  Cc: linux-nfs, Olga Kornievskaia, linux-kernel, speedcracker, Dai Ngo,
	Tom Talpey, Jeff Layton, NeilBrown, david.laight.linux,
	Andrew Morten, stable

On 11/14/25 4:19 PM, Chuck Lever 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 in nfsd4_get_drc_mem() 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 caused a compile failure if it could be statically
> determined that "max < min".
> 
> The relevant code was no longer present upstream when commit 1519fbc8832b
> ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> landed there, so there is no upstream change to nfsd4_get_drc_mem() to
> backport.
> 
> There is no clear case that the existing code in nfsd4_get_drc_mem()
> is functioning incorrectly. The goal of this patch is to permit the clean
> application of commit 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for
> the lo < hi test in clamp()"), and any commits that depend on it, to LTS
> kernels without affecting the ability to compile those kernels. This is
> done by open-coding the __clamp() macro sans the built-in type checking.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
> Signed-off-by: NeilBrown <neil@brown.name>
> Stable-dep-of: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4state.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Changes since Neil's post:
> * Editorial changes to the commit message
> * Attempt to address David's review comments
> * Applied to linux-6.12.y, passed NFSD upstream CI suite
> 
> This patch is intended to be applied to linux-6.12.y, and should
> apply cleanly to other LTS kernels since nfsd4_get_drc_mem hasn't
> changed since v5.4.
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7b0fabf8c657..41545933dd18 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1983,8 +1983,10 @@ 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);
> +	if (avail > total_avail / scale_factor)
> +		avail = total_avail / scale_factor;
> +	else if (avail < slotsize)
> +		avail = slotsize;
>  	num = min_t(int, num, avail / slotsize);
>  	num = max_t(int, num, 1);
>  	nfsd_drc_mem_used += num * slotsize;

Is something else needed from me to get this applied to v6.12 and older
LTS kernels?


-- 
Chuck Lever


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem()
  2025-12-01 19:28 ` Chuck Lever
@ 2025-12-02 15:51   ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2025-12-02 15:51 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Sasha Levin, linux-nfs, Olga Kornievskaia, linux-kernel,
	speedcracker, Dai Ngo, Tom Talpey, Jeff Layton, NeilBrown,
	david.laight.linux, Andrew Morten, stable

On Mon, Dec 01, 2025 at 02:28:50PM -0500, Chuck Lever wrote:
> On 11/14/25 4:19 PM, Chuck Lever 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 in nfsd4_get_drc_mem() 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 caused a compile failure if it could be statically
> > determined that "max < min".
> > 
> > The relevant code was no longer present upstream when commit 1519fbc8832b
> > ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> > landed there, so there is no upstream change to nfsd4_get_drc_mem() to
> > backport.
> > 
> > There is no clear case that the existing code in nfsd4_get_drc_mem()
> > is functioning incorrectly. The goal of this patch is to permit the clean
> > application of commit 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for
> > the lo < hi test in clamp()"), and any commits that depend on it, to LTS
> > kernels without affecting the ability to compile those kernels. This is
> > done by open-coding the __clamp() macro sans the built-in type checking.
> > 
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220745#c0
> > Signed-off-by: NeilBrown <neil@brown.name>
> > Stable-dep-of: 1519fbc8832b ("minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()")
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  fs/nfsd/nfs4state.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > Changes since Neil's post:
> > * Editorial changes to the commit message
> > * Attempt to address David's review comments
> > * Applied to linux-6.12.y, passed NFSD upstream CI suite
> > 
> > This patch is intended to be applied to linux-6.12.y, and should
> > apply cleanly to other LTS kernels since nfsd4_get_drc_mem hasn't
> > changed since v5.4.
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 7b0fabf8c657..41545933dd18 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1983,8 +1983,10 @@ 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);
> > +	if (avail > total_avail / scale_factor)
> > +		avail = total_avail / scale_factor;
> > +	else if (avail < slotsize)
> > +		avail = slotsize;
> >  	num = min_t(int, num, avail / slotsize);
> >  	num = max_t(int, num, 1);
> >  	nfsd_drc_mem_used += num * slotsize;
> 
> Is something else needed from me to get this applied to v6.12 and older
> LTS kernels?

Oops, sorry about that, now queued up.

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-02 15:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 21:19 [PATCH v1 v6.12.y] nfsd: Replace clamp_t in nfsd4_get_drc_mem() Chuck Lever
2025-11-14 22:14 ` David Laight
2025-12-01 19:28 ` Chuck Lever
2025-12-02 15:51   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).