[PATCH 0/2] nfsd/sunrpc: fix up some layering violations

[PATCH 0/2] nfsd/sunrpc: fix up some layering violations

[Jeff Layton]

The rq_lease_breaker and rq_cachetype fields are only used by nfsd.
These patches shrink struct svc_rqst slightly and makes the layering
between the two a little cleaner. In the case of rq_lease_breaker, I
think this also gives us a little more type safety.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (2):
      nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker
      nfsd/sunrpc: move rq_cachetype into struct nfsd_thread_local_info

 fs/nfsd/nfs4proc.c         | 3 ++-
 fs/nfsd/nfs4state.c        | 9 ++++++---
 fs/nfsd/nfs4xdr.c          | 3 ++-
 fs/nfsd/nfscache.c         | 3 ++-
 fs/nfsd/nfsd.h             | 5 +++++
 fs/nfsd/nfssvc.c           | 8 ++++++--
 include/linux/sunrpc/svc.h | 6 ++++--
 7 files changed, 27 insertions(+), 10 deletions(-)
---
base-commit: e42954bff52b9538de21267c1bde2567b13b6632
change-id: 20260113-rq_private-05f22aac0c20

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker

[Jeff Layton]

rq_lease_breaker has always been a NFSv4 specific layering violation in
svc_rqst. The reason it's there though is that we need a place that is
thread-local, and accessible from the svc_rqst pointer.

Add a new rq_private pointer to struct svc_rqst. This is intended for
use by the threads that are handling the service. sunrpc code doesn't
touch it.

In nfsd, define a new struct nfsd_thread_local_info. nfsd declares one
of these on the stack and puts a pointer to it in rq_private.

Add a new ntli_lease_breaker field to the new struct and convert all of
the places that access rq_lease_breaker to use the new field instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c         | 3 ++-
 fs/nfsd/nfs4state.c        | 9 ++++++---
 fs/nfsd/nfsd.h             | 4 ++++
 fs/nfsd/nfssvc.c           | 3 +++
 include/linux/sunrpc/svc.h | 5 ++++-
 5 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index d476d108f6e107ad5310e4ea504daabd750cf450..5c30ea99daad2825ba62fefc456a6d2cc41b6063 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -3038,6 +3038,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 	struct svc_fh *current_fh = &cstate->current_fh;
 	struct svc_fh *save_fh = &cstate->save_fh;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+	struct nfsd_thread_local_info *ntli = rqstp->rq_private;
 	__be32		status;
 
 	resp->xdr = &rqstp->rq_res_stream;
@@ -3076,7 +3077,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 	}
 	check_if_stalefh_allowed(args);
 
-	rqstp->rq_lease_breaker = (void **)&cstate->clp;
+	ntli->ntli_lease_breaker = &cstate->clp;
 
 	trace_nfsd_compound(rqstp, args->tag, args->taglen, args->client_opcnt);
 	while (!status && resp->opcnt < args->opcnt) {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index af2b307ac78cf3733a173f9024aced87fe94603f..583c13b5aaf3cd12a87c7aae62fe6a8223368f55 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5542,13 +5542,15 @@ nfsd_break_deleg_cb(struct file_lease *fl)
 static bool nfsd_breaker_owns_lease(struct file_lease *fl)
 {
 	struct nfs4_delegation *dl = fl->c.flc_owner;
+	struct nfsd_thread_local_info *ntli;
 	struct svc_rqst *rqst;
 	struct nfs4_client *clp;
 
 	rqst = nfsd_current_rqst();
 	if (!nfsd_v4client(rqst))
 		return false;
-	clp = *(rqst->rq_lease_breaker);
+	ntli = rqst->rq_private;
+	clp = *ntli->ntli_lease_breaker;
 	return dl->dl_stid.sc_client == clp;
 }
 
@@ -9360,13 +9362,14 @@ __be32
 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 			     struct nfs4_delegation **pdp)
 {
-	__be32 status;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+	struct nfsd_thread_local_info *ntli = rqstp->rq_private;
 	struct file_lock_context *ctx;
 	struct nfs4_delegation *dp = NULL;
 	struct file_lease *fl;
 	struct nfs4_cb_fattr *ncf;
 	struct inode *inode = d_inode(dentry);
+	__be32 status;
 
 	ctx = locks_inode_context(inode);
 	if (!ctx)
@@ -9387,7 +9390,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 		break;
 	}
 	if (dp == NULL || dp == NON_NFSD_LEASE ||
-	    dp->dl_recall.cb_clp == *(rqstp->rq_lease_breaker)) {
+	    dp->dl_recall.cb_clp == *(ntli->ntli_lease_breaker)) {
 		spin_unlock(&ctx->flc_lock);
 		if (dp == NON_NFSD_LEASE) {
 			status = nfserrno(nfsd_open_break_lease(inode,
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index a01d709533585f09df1399b85eecc36ea7c466c5..938906c6d10cd65e7e3a1bc889b4fdcb56918f6f 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -82,6 +82,10 @@ extern atomic_t			nfsd_th_cnt;		/* number of available threads */
 
 extern const struct seq_operations nfs_exports_op;
 
+struct nfsd_thread_local_info {
+	struct nfs4_client	**ntli_lease_breaker;
+};
+
 /*
  * Common void argument and result helpers
  */
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -900,6 +900,7 @@ nfsd(void *vrqstp)
 	struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
 	struct net *net = perm_sock->xpt_net;
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct nfsd_thread_local_info ntli = { };
 	bool have_mutex = false;
 
 	/* At this point, the thread shares current->fs
@@ -914,6 +915,8 @@ nfsd(void *vrqstp)
 
 	set_freezable();
 
+	rqstp->rq_private = &ntli;
+
 	/*
 	 * The main request loop
 	 */
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 4dc14c7a711b010473bf03fc401df0e66d9aa4bd..ab8237ba9596e9f31e2c42abedec435a23162b40 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -175,6 +175,9 @@ static inline unsigned long svc_serv_maxpages(const struct svc_serv *serv)
 /*
  * The context of a single thread, including the request currently being
  * processed.
+ *
+ * RPC programs are free to use rq_private to stash thread-local information.
+ * The sunrpc layer will not access it.
  */
 struct svc_rqst {
 	struct list_head	rq_all;		/* all threads list */
@@ -251,7 +254,7 @@ struct svc_rqst {
 	unsigned long		bc_to_initval;
 	unsigned int		bc_to_retries;
 	unsigned int		rq_status_counter; /* RPC processing counter */
-	void			**rq_lease_breaker; /* The v4 client breaking a lease */
+	void			*rq_private;	/* For use by the service thread */
 };
 
 /* bits for rq_flags */

-- 
2.52.0

[PATCH 2/2] nfsd/sunrpc: move rq_cachetype into struct nfsd_thread_local_info

[Jeff Layton]

The svc_rqst->rq_cachetype field is only accessed by nfsd. Move it
into the nfsd_thread_local_info instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4xdr.c          | 3 ++-
 fs/nfsd/nfscache.c         | 3 ++-
 fs/nfsd/nfsd.h             | 1 +
 fs/nfsd/nfssvc.c           | 5 +++--
 include/linux/sunrpc/svc.h | 1 -
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5172dbd0cb05956f8d5465e08fec93b5133ec55f..884b792c95a387fba24b6f540063c90d7fc1f1b0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2598,6 +2598,7 @@ nfsd4_opnum_in_range(struct nfsd4_compoundargs *argp, struct nfsd4_op *op)
 static bool
 nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
 {
+	struct nfsd_thread_local_info *ntli = argp->rqstp->rq_private;
 	struct nfsd4_op *op;
 	bool cachethis = false;
 	int auth_slack= argp->rqstp->rq_auth_slack;
@@ -2690,7 +2691,7 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
 	if (argp->minorversion)
 		cachethis = false;
 	svc_reserve_auth(argp->rqstp, max_reply + readbytes);
-	argp->rqstp->rq_cachetype = cachethis ? RC_REPLBUFF : RC_NOCACHE;
+	ntli->ntli_cachetype = cachethis ? RC_REPLBUFF : RC_NOCACHE;
 
 	argp->splice_ok = nfsd_read_splice_ok(argp->rqstp);
 	if (readcount > 1 || max_reply > PAGE_SIZE - auth_slack)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index ab13ee9c7fd8421ce6f66e3dc20d657b6442fbb8..154468ceccdc12dbeab3a1f19086e8ac9d37a223 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -467,10 +467,11 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
 		      unsigned int len, struct nfsd_cacherep **cacherep)
 {
 	struct nfsd_net		*nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+	struct nfsd_thread_local_info *ntli = rqstp->rq_private;
 	struct nfsd_cacherep	*rp, *found;
 	__wsum			csum;
 	struct nfsd_drc_bucket	*b;
-	int type = rqstp->rq_cachetype;
+	int type = ntli->ntli_cachetype;
 	LIST_HEAD(dispose);
 	int rtn = RC_DOIT;
 
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 938906c6d10cd65e7e3a1bc889b4fdcb56918f6f..a2e35a4fa105380c2d99cb0865003e0f7f4a8e8d 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -84,6 +84,7 @@ extern const struct seq_operations nfs_exports_op;
 
 struct nfsd_thread_local_info {
 	struct nfs4_client	**ntli_lease_breaker;
+	int			ntli_cachetype;
 };
 
 /*
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 8ce366c9e49220e8baf475c2e5f3424fedc1cec1..294876910cf22b4271eb3b447d23b63866fae179 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -983,6 +983,7 @@ nfsd(void *vrqstp)
  */
 int nfsd_dispatch(struct svc_rqst *rqstp)
 {
+	struct nfsd_thread_local_info *ntli = rqstp->rq_private;
 	const struct svc_procedure *proc = rqstp->rq_procinfo;
 	__be32 *statp = rqstp->rq_accept_statp;
 	struct nfsd_cacherep *rp;
@@ -993,7 +994,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	 * Give the xdr decoder a chance to change this if it wants
 	 * (necessary in the NFSv4.0 compound case)
 	 */
-	rqstp->rq_cachetype = proc->pc_cachetype;
+	ntli->ntli_cachetype = proc->pc_cachetype;
 
 	/*
 	 * ->pc_decode advances the argument stream past the NFS
@@ -1038,7 +1039,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
 	 */
 	smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter + 1);
 
-	nfsd_cache_update(rqstp, rp, rqstp->rq_cachetype, nfs_reply);
+	nfsd_cache_update(rqstp, rp, ntli->ntli_cachetype, nfs_reply);
 out_cached_reply:
 	return 1;
 
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index ab8237ba9596e9f31e2c42abedec435a23162b40..62152e4f3bccee3e7d4d99ad08d6a50ca1252c1e 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -218,7 +218,6 @@ struct svc_rqst {
 	u32			rq_vers;	/* program version */
 	u32			rq_proc;	/* procedure number */
 	u32			rq_prot;	/* IP protocol */
-	int			rq_cachetype;	/* catering to nfsd */
 	unsigned long		rq_flags;	/* flags field */
 	ktime_t			rq_qtime;	/* enqueue time */
 

-- 
2.52.0

Re: [PATCH 0/2] nfsd/sunrpc: fix up some layering violations

[Benjamin Coddington]

On 13 Jan 2026, at 13:37, Jeff Layton wrote:

> The rq_lease_breaker and rq_cachetype fields are only used by nfsd.
> These patches shrink struct svc_rqst slightly and makes the layering
> between the two a little cleaner. In the case of rq_lease_breaker, I
> think this also gives us a little more type safety.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Jeff Layton (2):
>       nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker
>       nfsd/sunrpc: move rq_cachetype into struct nfsd_thread_local_info
>
>  fs/nfsd/nfs4proc.c         | 3 ++-
>  fs/nfsd/nfs4state.c        | 9 ++++++---
>  fs/nfsd/nfs4xdr.c          | 3 ++-
>  fs/nfsd/nfscache.c         | 3 ++-
>  fs/nfsd/nfsd.h             | 5 +++++
>  fs/nfsd/nfssvc.c           | 8 ++++++--
>  include/linux/sunrpc/svc.h | 6 ++++--
>  7 files changed, 27 insertions(+), 10 deletions(-)
> ---
> base-commit: e42954bff52b9538de21267c1bde2567b13b6632
> change-id: 20260113-rq_private-05f22aac0c20
>
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>

Both look good to me:

Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>

Ben

Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker

[Chuck Lever]

On Tue, Jan 13, 2026, at 1:37 PM, Jeff Layton wrote:
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -900,6 +900,7 @@ nfsd(void *vrqstp)
> struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
> struct net *net = perm_sock->xpt_net;
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> + struct nfsd_thread_local_info ntli = { };
> bool have_mutex = false;
>  
> /* At this point, the thread shares current->fs
> @@ -914,6 +915,8 @@ nfsd(void *vrqstp)
>  
> set_freezable();
>  
> + rqstp->rq_private = &ntli;
> +
> /*
> * The main request loop
> */

Thanks for tackling this one. Nits below...

This assumes sizeof(structure nfsd_thread_local_info) will always
be small enough that it is reasonable to keep on the stack. I
can't say that would be a good bet in the long run.

And we don't need the perfect reliability of not doing a dynamic
allocation here. If kmalloc(sizeof(struct nfsd_thread_local_info))
fails, the thread exits immediately, no harm.

Also, scatter-gather lists could not be stored directly in this
object because it is on the stack. If Ben wanted to stick a
32-byte buffer in struct nfsd_thread_local_info to be used with
the crypto API, it would have to be a pointer to it, not the
buffer itself.


Chuck Lever

Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker

[Jeff Layton]

On Tue, 2026-01-13 at 14:21 -0500, Chuck Lever wrote:
> 
> On Tue, Jan 13, 2026, at 1:37 PM, Jeff Layton wrote:
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -900,6 +900,7 @@ nfsd(void *vrqstp)
> > struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
> > struct net *net = perm_sock->xpt_net;
> > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > + struct nfsd_thread_local_info ntli = { };
> > bool have_mutex = false;
> >  
> > /* At this point, the thread shares current->fs
> > @@ -914,6 +915,8 @@ nfsd(void *vrqstp)
> >  
> > set_freezable();
> >  
> > + rqstp->rq_private = &ntli;
> > +
> > /*
> > * The main request loop
> > */
> 
> Thanks for tackling this one. Nits below...
> 
> This assumes sizeof(structure nfsd_thread_local_info) will always
> be small enough that it is reasonable to keep on the stack. I
> can't say that would be a good bet in the long run.
> 
> And we don't need the perfect reliability of not doing a dynamic
> allocation here. If kmalloc(sizeof(struct nfsd_thread_local_info))
> fails, the thread exits immediately, no harm.
> 

Not sure how much space Ben will need (if any).

We certainly could have nfsd allocate this separately. I didn't see the
point for something that is only a few bytes though.

> Also, scatter-gather lists could not be stored directly in this
> object because it is on the stack. If Ben wanted to stick a
> 32-byte buffer in struct nfsd_thread_local_info to be used with
> the crypto API, it would have to be a pointer to it, not the
> buffer itself.
> 

*nod*
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker

[Chuck Lever]

On Tue, Jan 13, 2026, at 2:31 PM, Jeff Layton wrote:
> On Tue, 2026-01-13 at 14:21 -0500, Chuck Lever wrote:
>> 
>> On Tue, Jan 13, 2026, at 1:37 PM, Jeff Layton wrote:
>> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> > index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
>> > --- a/fs/nfsd/nfssvc.c
>> > +++ b/fs/nfsd/nfssvc.c
>> > @@ -900,6 +900,7 @@ nfsd(void *vrqstp)
>> > struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
>> > struct net *net = perm_sock->xpt_net;
>> > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>> > + struct nfsd_thread_local_info ntli = { };
>> > bool have_mutex = false;
>> >  
>> > /* At this point, the thread shares current->fs
>> > @@ -914,6 +915,8 @@ nfsd(void *vrqstp)
>> >  
>> > set_freezable();
>> >  
>> > + rqstp->rq_private = &ntli;
>> > +
>> > /*
>> > * The main request loop
>> > */
>> 
>> Thanks for tackling this one. Nits below...
>> 
>> This assumes sizeof(structure nfsd_thread_local_info) will always
>> be small enough that it is reasonable to keep on the stack. I
>> can't say that would be a good bet in the long run.
>> 
>> And we don't need the perfect reliability of not doing a dynamic
>> allocation here. If kmalloc(sizeof(struct nfsd_thread_local_info))
>> fails, the thread exits immediately, no harm.
>> 
>
> Not sure how much space Ben will need (if any).
>
> We certainly could have nfsd allocate this separately. I didn't see the
> point for something that is only a few bytes though.

If we are designing for today, another approach would be to set up
a BUILD_WARN_ON or other type of static build failure if this
structure grows larger than, say 256 bytes -- then add dynamic
allocation at that point.


>> Also, scatter-gather lists could not be stored directly in this
>> object because it is on the stack. If Ben wanted to stick a
>> 32-byte buffer in struct nfsd_thread_local_info to be used with
>> the crypto API, it would have to be a pointer to it, not the
>> buffer itself.
>> 
>
> *nod*
> -- 
> Jeff Layton <jlayton@kernel.org>

-- 
Chuck Lever

Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker

[Jeff Layton]

On Tue, 2026-01-13 at 14:41 -0500, Chuck Lever wrote:
> 
> On Tue, Jan 13, 2026, at 2:31 PM, Jeff Layton wrote:
> > On Tue, 2026-01-13 at 14:21 -0500, Chuck Lever wrote:
> > > 
> > > On Tue, Jan 13, 2026, at 1:37 PM, Jeff Layton wrote:
> > > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > > > index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
> > > > --- a/fs/nfsd/nfssvc.c
> > > > +++ b/fs/nfsd/nfssvc.c
> > > > @@ -900,6 +900,7 @@ nfsd(void *vrqstp)
> > > > struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
> > > > struct net *net = perm_sock->xpt_net;
> > > > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > > > + struct nfsd_thread_local_info ntli = { };
> > > > bool have_mutex = false;
> > > >  
> > > > /* At this point, the thread shares current->fs
> > > > @@ -914,6 +915,8 @@ nfsd(void *vrqstp)
> > > >  
> > > > set_freezable();
> > > >  
> > > > + rqstp->rq_private = &ntli;
> > > > +
> > > > /*
> > > > * The main request loop
> > > > */
> > > 
> > > Thanks for tackling this one. Nits below...
> > > 
> > > This assumes sizeof(structure nfsd_thread_local_info) will always
> > > be small enough that it is reasonable to keep on the stack. I
> > > can't say that would be a good bet in the long run.
> > > 
> > > And we don't need the perfect reliability of not doing a dynamic
> > > allocation here. If kmalloc(sizeof(struct nfsd_thread_local_info))
> > > fails, the thread exits immediately, no harm.
> > > 
> > 
> > Not sure how much space Ben will need (if any).
> > 
> > We certainly could have nfsd allocate this separately. I didn't see the
> > point for something that is only a few bytes though.
> 
> If we are designing for today, another approach would be to set up
> a BUILD_WARN_ON or other type of static build failure if this
> structure grows larger than, say 256 bytes -- then add dynamic
> allocation at that point.
> 

I don't see us growing this to huge proportions. I think that's the
sort of thing we just have to watch out for as maintainers.

If you really insist on a BUILD_WARN_ON tripwire, I'll add one, but it
seems like overkill to me.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker

[Chuck Lever]

On Tue, Jan 13, 2026, at 3:12 PM, Jeff Layton wrote:
> On Tue, 2026-01-13 at 14:41 -0500, Chuck Lever wrote:
>> 
>> On Tue, Jan 13, 2026, at 2:31 PM, Jeff Layton wrote:
>> > On Tue, 2026-01-13 at 14:21 -0500, Chuck Lever wrote:
>> > > 
>> > > On Tue, Jan 13, 2026, at 1:37 PM, Jeff Layton wrote:
>> > > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> > > > index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
>> > > > --- a/fs/nfsd/nfssvc.c
>> > > > +++ b/fs/nfsd/nfssvc.c
>> > > > @@ -900,6 +900,7 @@ nfsd(void *vrqstp)
>> > > > struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
>> > > > struct net *net = perm_sock->xpt_net;
>> > > > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>> > > > + struct nfsd_thread_local_info ntli = { };
>> > > > bool have_mutex = false;
>> > > >  
>> > > > /* At this point, the thread shares current->fs
>> > > > @@ -914,6 +915,8 @@ nfsd(void *vrqstp)
>> > > >  
>> > > > set_freezable();
>> > > >  
>> > > > + rqstp->rq_private = &ntli;
>> > > > +
>> > > > /*
>> > > > * The main request loop
>> > > > */
>> > > 
>> > > Thanks for tackling this one. Nits below...
>> > > 
>> > > This assumes sizeof(structure nfsd_thread_local_info) will always
>> > > be small enough that it is reasonable to keep on the stack. I
>> > > can't say that would be a good bet in the long run.
>> > > 
>> > > And we don't need the perfect reliability of not doing a dynamic
>> > > allocation here. If kmalloc(sizeof(struct nfsd_thread_local_info))
>> > > fails, the thread exits immediately, no harm.
>> > > 
>> > 
>> > Not sure how much space Ben will need (if any).
>> > 
>> > We certainly could have nfsd allocate this separately. I didn't see the
>> > point for something that is only a few bytes though.
>> 
>> If we are designing for today, another approach would be to set up
>> a BUILD_WARN_ON or other type of static build failure if this
>> structure grows larger than, say 256 bytes -- then add dynamic
>> allocation at that point.
>> 
>
> I don't see us growing this to huge proportions. I think that's the
> sort of thing we just have to watch out for as maintainers.
>
> If you really insist on a BUILD_WARN_ON tripwire, I'll add one, but it
> seems like overkill to me.

I think a tripwire documents the assumption made here in a single
line... no different than a lockdep_assert.

More critically, though, maintainers tend to have a very short memory.
Not a trustworthy lot, really.

static_assert(sizeof(struct nfsd_thread_local_info) < 256);


-- 
Chuck Lever

Re: [PATCH 0/2] nfsd/sunrpc: fix up some layering violations

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

On Tue, 13 Jan 2026 13:37:38 -0500, Jeff Layton wrote:
> The rq_lease_breaker and rq_cachetype fields are only used by nfsd.
> These patches shrink struct svc_rqst slightly and makes the layering
> between the two a little cleaner. In the case of rq_lease_breaker, I
> think this also gives us a little more type safety.
> 
> 

Applied to nfsd-testing, thanks!

[1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease_breaker
      commit: 2f9a5cd51d92cd95270edfa45c11ddfce1f8e169
[2/2] nfsd/sunrpc: move rq_cachetype into struct nfsd_thread_local_info
      commit: a5428a9e04ffc2b7907cd976202d9fc28c7455f1

--
Chuck Lever