[PATCH 1 1/3] SUNRPC: Remove unused svc_rqst::rq_lock field

[PATCH 1 1/3] SUNRPC: Remove unused svc_rqst::rq_lock field

[Chuck Lever]

Clean up after commit 22700f3c6df5 ("SUNRPC: Improve ordering of
transport processing").

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/svc.h |    1 -
 net/sunrpc/svc.c           |    1 -
 2 files changed, 2 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index daecb009c05b..3b59eb9cf884 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -311,7 +311,6 @@ struct svc_rqst {
 	struct auth_domain *	rq_gssclient;	/* "gss/"-style peer info */
 	struct svc_cacherep *	rq_cacherep;	/* cache info */
 	struct task_struct	*rq_task;	/* service thread */
-	spinlock_t		rq_lock;	/* per-request lock */
 	struct net		*rq_bc_net;	/* pointer to backchannel's
 						 * net namespace
 						 */
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 7c9a0d0b1230..d2bb1d04c524 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -638,7 +638,6 @@ svc_rqst_alloc(struct svc_serv *serv, struct svc_pool *pool, int node)
 		return rqstp;
 
 	__set_bit(RQ_BUSY, &rqstp->rq_flags);
-	spin_lock_init(&rqstp->rq_lock);
 	rqstp->rq_server = serv;
 	rqstp->rq_pool = pool;
 

[PATCH 1 2/3] NFSD: Finish converting the NFSv2 GETACL result encoder

[Chuck Lever]

The xdr_stream conversion inadvertently left some code that set the
page_len of the send buffer. The XDR stream encoders should handle
this automatically now.

This oversight adds garbage past the end of the Reply message.
Clients typically ignore the garbage, but NFSD does not need to send
it, as it leaks stale memory contents onto the wire.

Fixes: f8cba47344f7 ("NFSD: Update the NFSv2 GETACL result encoder to use struct xdr_stream")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs2acl.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 9edd3c1a30fb..87f224cd30a8 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -246,7 +246,6 @@ nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 	struct nfsd3_getaclres *resp = rqstp->rq_resp;
 	struct dentry *dentry = resp->fh.fh_dentry;
 	struct inode *inode;
-	int w;
 
 	if (!svcxdr_encode_stat(xdr, resp->status))
 		return false;
@@ -260,15 +259,6 @@ nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 	if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
 		return false;
 
-	rqstp->rq_res.page_len = w = nfsacl_size(
-		(resp->mask & NFS_ACL)   ? resp->acl_access  : NULL,
-		(resp->mask & NFS_DFACL) ? resp->acl_default : NULL);
-	while (w > 0) {
-		if (!*(rqstp->rq_next_page++))
-			return true;
-		w -= PAGE_SIZE;
-	}
-
 	if (!nfs_stream_encode_acl(xdr, inode, resp->acl_access,
 				   resp->mask & NFS_ACL, 0))
 		return false;

Re: [PATCH 1 2/3] NFSD: Finish converting the NFSv2 GETACL result encoder

[Jeff Layton]

On Sun, 2022-10-16 at 11:47 -0400, Chuck Lever wrote:
> The xdr_stream conversion inadvertently left some code that set the
> page_len of the send buffer. The XDR stream encoders should handle
> this automatically now.
> 
> This oversight adds garbage past the end of the Reply message.
> Clients typically ignore the garbage, but NFSD does not need to send
> it, as it leaks stale memory contents onto the wire.
> 
> Fixes: f8cba47344f7 ("NFSD: Update the NFSv2 GETACL result encoder to use struct xdr_stream")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs2acl.c |   10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
> index 9edd3c1a30fb..87f224cd30a8 100644
> --- a/fs/nfsd/nfs2acl.c
> +++ b/fs/nfsd/nfs2acl.c
> @@ -246,7 +246,6 @@ nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
>  	struct nfsd3_getaclres *resp = rqstp->rq_resp;
>  	struct dentry *dentry = resp->fh.fh_dentry;
>  	struct inode *inode;
> -	int w;
>  
>  	if (!svcxdr_encode_stat(xdr, resp->status))
>  		return false;
> @@ -260,15 +259,6 @@ nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
>  	if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
>  		return false;
>  
> -	rqstp->rq_res.page_len = w = nfsacl_size(
> -		(resp->mask & NFS_ACL)   ? resp->acl_access  : NULL,
> -		(resp->mask & NFS_DFACL) ? resp->acl_default : NULL);
> -	while (w > 0) {
> -		if (!*(rqstp->rq_next_page++))
> -			return true;
> -		w -= PAGE_SIZE;
> -	}
> -
>  	if (!nfs_stream_encode_acl(xdr, inode, resp->acl_access,
>  				   resp->mask & NFS_ACL, 0))
>  		return false;
> 
> 

That makes a lot more sense now.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

[PATCH 1 3/3] NFSD: Finish converting the NFSv3 GETACL result encoder

[Chuck Lever]

For some reason, the NFSv2 GETACL result encoder was fully converted
to use the new nfs_stream_encode_acl(), but the NFSv3 equivalent was
not similarly converted.

Fixes: 20798dfe249a ("NFSD: Update the NFSv3 GETACL result encoder to use struct xdr_stream")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs3acl.c |   30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
index 9446c6743664..7c798b5f4ec6 100644
--- a/fs/nfsd/nfs3acl.c
+++ b/fs/nfsd/nfs3acl.c
@@ -171,11 +171,7 @@ nfs3svc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 {
 	struct nfsd3_getaclres *resp = rqstp->rq_resp;
 	struct dentry *dentry = resp->fh.fh_dentry;
-	struct kvec *head = rqstp->rq_res.head;
 	struct inode *inode;
-	unsigned int base;
-	int n;
-	int w;
 
 	if (!svcxdr_encode_nfsstat3(xdr, resp->status))
 		return false;
@@ -187,26 +183,12 @@ nfs3svc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 		if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
 			return false;
 
-		base = (char *)xdr->p - (char *)head->iov_base;
-
-		rqstp->rq_res.page_len = w = nfsacl_size(
-			(resp->mask & NFS_ACL)   ? resp->acl_access  : NULL,
-			(resp->mask & NFS_DFACL) ? resp->acl_default : NULL);
-		while (w > 0) {
-			if (!*(rqstp->rq_next_page++))
-				return false;
-			w -= PAGE_SIZE;
-		}
-
-		n = nfsacl_encode(&rqstp->rq_res, base, inode,
-				  resp->acl_access,
-				  resp->mask & NFS_ACL, 0);
-		if (n > 0)
-			n = nfsacl_encode(&rqstp->rq_res, base + n, inode,
-					  resp->acl_default,
-					  resp->mask & NFS_DFACL,
-					  NFS_ACL_DEFAULT);
-		if (n <= 0)
+		if (!nfs_stream_encode_acl(xdr, inode, resp->acl_access,
+					   resp->mask & NFS_ACL, 0))
+			return false;
+		if (!nfs_stream_encode_acl(xdr, inode, resp->acl_default,
+					   resp->mask & NFS_DFACL,
+					   NFS_ACL_DEFAULT))
 			return false;
 		break;
 	default:

Re: [PATCH 1 3/3] NFSD: Finish converting the NFSv3 GETACL result encoder

[Jeff Layton]

On Sun, 2022-10-16 at 11:47 -0400, Chuck Lever wrote:
> For some reason, the NFSv2 GETACL result encoder was fully converted
> to use the new nfs_stream_encode_acl(), but the NFSv3 equivalent was
> not similarly converted.
> 
> Fixes: 20798dfe249a ("NFSD: Update the NFSv3 GETACL result encoder to use struct xdr_stream")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs3acl.c |   30 ++++++------------------------
>  1 file changed, 6 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
> index 9446c6743664..7c798b5f4ec6 100644
> --- a/fs/nfsd/nfs3acl.c
> +++ b/fs/nfsd/nfs3acl.c
> @@ -171,11 +171,7 @@ nfs3svc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
>  {
>  	struct nfsd3_getaclres *resp = rqstp->rq_resp;
>  	struct dentry *dentry = resp->fh.fh_dentry;
> -	struct kvec *head = rqstp->rq_res.head;
>  	struct inode *inode;
> -	unsigned int base;
> -	int n;
> -	int w;
>  
>  	if (!svcxdr_encode_nfsstat3(xdr, resp->status))
>  		return false;
> @@ -187,26 +183,12 @@ nfs3svc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
>  		if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
>  			return false;
>  
> -		base = (char *)xdr->p - (char *)head->iov_base;
> -
> -		rqstp->rq_res.page_len = w = nfsacl_size(
> -			(resp->mask & NFS_ACL)   ? resp->acl_access  : NULL,
> -			(resp->mask & NFS_DFACL) ? resp->acl_default : NULL);
> -		while (w > 0) {
> -			if (!*(rqstp->rq_next_page++))
> -				return false;
> -			w -= PAGE_SIZE;
> -		}
> -
> -		n = nfsacl_encode(&rqstp->rq_res, base, inode,
> -				  resp->acl_access,
> -				  resp->mask & NFS_ACL, 0);
> -		if (n > 0)
> -			n = nfsacl_encode(&rqstp->rq_res, base + n, inode,
> -					  resp->acl_default,
> -					  resp->mask & NFS_DFACL,
> -					  NFS_ACL_DEFAULT);
> -		if (n <= 0)
> +		if (!nfs_stream_encode_acl(xdr, inode, resp->acl_access,
> +					   resp->mask & NFS_ACL, 0))
> +			return false;
> +		if (!nfs_stream_encode_acl(xdr, inode, resp->acl_default,
> +					   resp->mask & NFS_DFACL,
> +					   NFS_ACL_DEFAULT))
>  			return false;
>  		break;
>  	default:
> 
> 

Much cleaner.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 1 1/3] SUNRPC: Remove unused svc_rqst::rq_lock field

[Jeff Layton]

On Sun, 2022-10-16 at 11:46 -0400, Chuck Lever wrote:
> Clean up after commit 22700f3c6df5 ("SUNRPC: Improve ordering of
> transport processing").
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  include/linux/sunrpc/svc.h |    1 -
>  net/sunrpc/svc.c           |    1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index daecb009c05b..3b59eb9cf884 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -311,7 +311,6 @@ struct svc_rqst {
>  	struct auth_domain *	rq_gssclient;	/* "gss/"-style peer info */
>  	struct svc_cacherep *	rq_cacherep;	/* cache info */
>  	struct task_struct	*rq_task;	/* service thread */
> -	spinlock_t		rq_lock;	/* per-request lock */
>  	struct net		*rq_bc_net;	/* pointer to backchannel's
>  						 * net namespace
>  						 */
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 7c9a0d0b1230..d2bb1d04c524 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -638,7 +638,6 @@ svc_rqst_alloc(struct svc_serv *serv, struct svc_pool *pool, int node)
>  		return rqstp;
>  
>  	__set_bit(RQ_BUSY, &rqstp->rq_flags);
> -	spin_lock_init(&rqstp->rq_lock);
>  	rqstp->rq_server = serv;
>  	rqstp->rq_pool = pool;
>  
> 
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>