From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Sat, 21 Nov 2020 20:46:18 -0500
Subject: [PATCH] SUNRPC: Clean up the handling of page padding in
rpc_prepare_reply_pages()
Git-commit: 9ed5af268e88f6e5b65376be98d652b37cb20d7b
Patch-mainline: v5.11-rc1
References: for-next
rpc_prepare_reply_pages() currently expects the 'hdrsize' argument to
contain the length of the data that we expect to want placed in the head
kvec plus a count of 1 word of padding that is placed after the page data.
This is very confusing when trying to read the code, and sometimes leads
to callers adding an arbitrary value of '1' just in order to satisfy the
requirement (whether or not the page data actually needs such padding).
This patch aims to clarify the code by changing the 'hdrsize' argument
to remove that 1 word of padding. This means we need to subtract the
padding from all the existing callers.
Fixes: 02ef04e432ba ("NFS: Account for XDR pad of buf->pages")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Acked-by: NeilBrown <neilb@suse.com>
---
fs/nfs/nfs2xdr.c | 19 ++++++++++---------
fs/nfs/nfs3xdr.c | 29 ++++++++++++++++-------------
fs/nfs/nfs4xdr.c | 36 +++++++++++++++++++-----------------
net/sunrpc/clnt.c | 5 +----
net/sunrpc/xdr.c | 3 ---
5 files changed, 46 insertions(+), 46 deletions(-)
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -34,6 +34,7 @@
* Declare the space requirements for NFS arguments and replies as
* number of 32bit-words
*/
+#define NFS_pagepad_sz (1) /* Page padding */
#define NFS_fhandle_sz (8)
#define NFS_sattr_sz (8)
#define NFS_filename_sz (1+(NFS2_MAXNAMLEN>>2))
@@ -56,11 +57,11 @@
#define NFS_attrstat_sz (1+NFS_fattr_sz)
#define NFS_diropres_sz (1+NFS_fhandle_sz+NFS_fattr_sz)
-#define NFS_readlinkres_sz (2+1)
-#define NFS_readres_sz (1+NFS_fattr_sz+1+1)
+#define NFS_readlinkres_sz (2+NFS_pagepad_sz)
+#define NFS_readres_sz (1+NFS_fattr_sz+1+NFS_pagepad_sz)
#define NFS_writeres_sz (NFS_attrstat_sz)
#define NFS_stat_sz (1)
-#define NFS_readdirres_sz (1+1)
+#define NFS_readdirres_sz (1+NFS_pagepad_sz)
#define NFS_statfsres_sz (1+NFS_info_sz)
static int nfs_stat_to_errno(enum nfs_stat);
@@ -597,8 +598,8 @@ static void nfs2_xdr_enc_readlinkargs(st
const struct nfs_readlinkargs *args = data;
encode_fhandle(xdr, args->fh);
- rpc_prepare_reply_pages(req, args->pages, args->pgbase,
- args->pglen, NFS_readlinkres_sz);
+ rpc_prepare_reply_pages(req, args->pages, args->pgbase, args->pglen,
+ NFS_readlinkres_sz - NFS_pagepad_sz);
}
/*
@@ -633,8 +634,8 @@ static void nfs2_xdr_enc_readargs(struct
const struct nfs_pgio_args *args = data;
encode_readargs(xdr, args);
- rpc_prepare_reply_pages(req, args->pages, args->pgbase,
- args->count, NFS_readres_sz);
+ rpc_prepare_reply_pages(req, args->pages, args->pgbase, args->count,
+ NFS_readres_sz - NFS_pagepad_sz);
req->rq_rcv_buf.flags |= XDRBUF_READ;
}
@@ -791,8 +792,8 @@ static void nfs2_xdr_enc_readdirargs(str
const struct nfs_readdirargs *args = data;
encode_readdirargs(xdr, args);
- rpc_prepare_reply_pages(req, args->pages, 0,
- args->count, NFS_readdirres_sz);
+ rpc_prepare_reply_pages(req, args->pages, 0, args->count,
+ NFS_readdirres_sz - NFS_pagepad_sz);
}
/*
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -33,6 +33,7 @@
* Declare the space requirements for NFS arguments and replies as
* number of 32bit-words
*/
+#define NFS3_pagepad_sz (1) /* Page padding */
#define NFS3_fhandle_sz (1+16)
#define NFS3_fh_sz (NFS3_fhandle_sz) /* shorthand */
#define NFS3_sattr_sz (15)
@@ -69,13 +70,13 @@
#define NFS3_removeres_sz (NFS3_setattrres_sz)
#define NFS3_lookupres_sz (1+NFS3_fh_sz+(2 * NFS3_post_op_attr_sz))
#define NFS3_accessres_sz (1+NFS3_post_op_attr_sz+1)
-#define NFS3_readlinkres_sz (1+NFS3_post_op_attr_sz+1+1)
-#define NFS3_readres_sz (1+NFS3_post_op_attr_sz+3+1)
+#define NFS3_readlinkres_sz (1+NFS3_post_op_attr_sz+1+NFS3_pagepad_sz)
+#define NFS3_readres_sz (1+NFS3_post_op_attr_sz+3+NFS3_pagepad_sz)
#define NFS3_writeres_sz (1+NFS3_wcc_data_sz+4)
#define NFS3_createres_sz (1+NFS3_fh_sz+NFS3_post_op_attr_sz+NFS3_wcc_data_sz)
#define NFS3_renameres_sz (1+(2 * NFS3_wcc_data_sz))
#define NFS3_linkres_sz (1+NFS3_post_op_attr_sz+NFS3_wcc_data_sz)
-#define NFS3_readdirres_sz (1+NFS3_post_op_attr_sz+2+1)
+#define NFS3_readdirres_sz (1+NFS3_post_op_attr_sz+2+NFS3_pagepad_sz)
#define NFS3_fsstatres_sz (1+NFS3_post_op_attr_sz+13)
#define NFS3_fsinfores_sz (1+NFS3_post_op_attr_sz+12)
#define NFS3_pathconfres_sz (1+NFS3_post_op_attr_sz+6)
@@ -85,7 +86,8 @@
#define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE))
#define ACL3_getaclres_sz (1+NFS3_post_op_attr_sz+1+ \
- XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE)+1)
+ XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE)+\
+ NFS3_pagepad_sz)
#define ACL3_setaclres_sz (1+NFS3_post_op_attr_sz)
static int nfs3_stat_to_errno(enum nfs_stat);
@@ -913,8 +915,8 @@ static void nfs3_xdr_enc_readlink3args(s
const struct nfs3_readlinkargs *args = data;
encode_nfs_fh3(xdr, args->fh);
- rpc_prepare_reply_pages(req, args->pages, args->pgbase,
- args->pglen, NFS3_readlinkres_sz);
+ rpc_prepare_reply_pages(req, args->pages, args->pgbase, args->pglen,
+ NFS3_readlinkres_sz - NFS3_pagepad_sz);
}
/*
@@ -943,7 +945,8 @@ static void nfs3_xdr_enc_read3args(struc
const void *data)
{
const struct nfs_pgio_args *args = data;
- unsigned int replen = args->replen ? args->replen : NFS3_readres_sz;
+ unsigned int replen = args->replen ? args->replen :
+ NFS3_readres_sz - NFS3_pagepad_sz;
encode_read3args(xdr, args);
rpc_prepare_reply_pages(req, args->pages, args->pgbase,
@@ -1243,8 +1246,8 @@ static void nfs3_xdr_enc_readdir3args(st
const struct nfs3_readdirargs *args = data;
encode_readdir3args(xdr, args);
- rpc_prepare_reply_pages(req, args->pages, 0,
- args->count, NFS3_readdirres_sz);
+ rpc_prepare_reply_pages(req, args->pages, 0, args->count,
+ NFS3_readdirres_sz - NFS3_pagepad_sz);
}
/*
@@ -1285,8 +1288,8 @@ static void nfs3_xdr_enc_readdirplus3arg
const struct nfs3_readdirargs *args = data;
encode_readdirplus3args(xdr, args);
- rpc_prepare_reply_pages(req, args->pages, 0,
- args->count, NFS3_readdirres_sz);
+ rpc_prepare_reply_pages(req, args->pages, 0, args->count,
+ NFS3_readdirres_sz - NFS3_pagepad_sz);
}
/*
@@ -1332,7 +1335,7 @@ static void nfs3_xdr_enc_getacl3args(str
if (args->mask & (NFS_ACL | NFS_DFACL)) {
rpc_prepare_reply_pages(req, args->pages, 0,
NFSACL_MAXPAGES << PAGE_SHIFT,
- ACL3_getaclres_sz);
+ ACL3_getaclres_sz - NFS3_pagepad_sz);
req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
}
}
@@ -1652,7 +1655,7 @@ static int nfs3_xdr_dec_read3res(struct
result->op_status = status;
if (status != NFS3_OK)
goto out_status;
- result->replen = 4 + ((xdr_stream_pos(xdr) - pos) >> 2);
+ result->replen = 3 + ((xdr_stream_pos(xdr) - pos) >> 2);
error = decode_read3resok(xdr, result);
out:
return error;
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -84,6 +84,7 @@ static int decode_layoutget(struct xdr_s
/* lock,open owner id:
* we currently use size 2 (u64) out of (NFS4_OPAQUE_LIMIT >> 2)
*/
+#define pagepad_maxsz (1)
#define open_owner_id_maxsz (1 + 2 + 1 + 1 + 2)
#define lock_owner_id_maxsz (1 + 1 + 4)
#define decode_lockowner_maxsz (1 + XDR_QUADLEN(IDMAP_NAMESZ))
@@ -215,14 +216,14 @@ static int decode_layoutget(struct xdr_s
nfs4_fattr_bitmap_maxsz)
#define encode_read_maxsz (op_encode_hdr_maxsz + \
encode_stateid_maxsz + 3)
-#define decode_read_maxsz (op_decode_hdr_maxsz + 2 + 1)
+#define decode_read_maxsz (op_decode_hdr_maxsz + 2 + pagepad_maxsz)
#define encode_readdir_maxsz (op_encode_hdr_maxsz + \
2 + encode_verifier_maxsz + 5 + \
nfs4_label_maxsz)
#define decode_readdir_maxsz (op_decode_hdr_maxsz + \
- decode_verifier_maxsz + 1)
+ decode_verifier_maxsz + pagepad_maxsz)
#define encode_readlink_maxsz (op_encode_hdr_maxsz)
-#define decode_readlink_maxsz (op_decode_hdr_maxsz + 1 + 1)
+#define decode_readlink_maxsz (op_decode_hdr_maxsz + 1 + pagepad_maxsz)
#define encode_write_maxsz (op_encode_hdr_maxsz + \
encode_stateid_maxsz + 4)
#define decode_write_maxsz (op_decode_hdr_maxsz + \
@@ -284,14 +285,14 @@ static int decode_layoutget(struct xdr_s
#define decode_delegreturn_maxsz (op_decode_hdr_maxsz)
#define encode_getacl_maxsz (encode_getattr_maxsz)
#define decode_getacl_maxsz (op_decode_hdr_maxsz + \
- nfs4_fattr_bitmap_maxsz + 1 + 1)
+ nfs4_fattr_bitmap_maxsz + 1 + pagepad_maxsz)
#define encode_setacl_maxsz (op_encode_hdr_maxsz + \
encode_stateid_maxsz + 3)
#define decode_setacl_maxsz (decode_setattr_maxsz)
#define encode_fs_locations_maxsz \
(encode_getattr_maxsz)
#define decode_fs_locations_maxsz \
- (1)
+ (pagepad_maxsz)
#define encode_secinfo_maxsz (op_encode_hdr_maxsz + nfs4_name_maxsz)
#define decode_secinfo_maxsz (op_decode_hdr_maxsz + 1 + ((NFS_MAX_SECFLAVORS * (16 + GSS_OID_MAX_LEN)) / 4))
@@ -393,12 +394,13 @@ static int decode_layoutget(struct xdr_s
/* devaddr4 payload is read into page */ \
1 /* notification bitmap length */ + \
1 /* notification bitmap, word 0 */ + \
- 1 /* possible XDR padding */)
+ pagepad_maxsz /* possible XDR padding */)
#define encode_layoutget_maxsz (op_encode_hdr_maxsz + 10 + \
encode_stateid_maxsz)
#define decode_layoutget_maxsz (op_decode_hdr_maxsz + 8 + \
decode_stateid_maxsz + \
- XDR_QUADLEN(PNFS_LAYOUT_MAXSIZE) + 1)
+ XDR_QUADLEN(PNFS_LAYOUT_MAXSIZE) + \
+ pagepad_maxsz)
#define encode_layoutcommit_maxsz (op_encode_hdr_maxsz + \
2 /* offset */ + \
2 /* length */ + \
@@ -2345,7 +2347,7 @@ static void nfs4_xdr_enc_open(struct rpc
encode_layoutget(xdr, args->lg_args, &hdr);
rpc_prepare_reply_pages(req, args->lg_args->layout.pages, 0,
args->lg_args->layout.pglen,
- hdr.replen);
+ hdr.replen - pagepad_maxsz);
}
encode_nops(&hdr);
}
@@ -2391,7 +2393,7 @@ static void nfs4_xdr_enc_open_noattr(str
encode_layoutget(xdr, args->lg_args, &hdr);
rpc_prepare_reply_pages(req, args->lg_args->layout.pages, 0,
args->lg_args->layout.pglen,
- hdr.replen);
+ hdr.replen - pagepad_maxsz);
}
encode_nops(&hdr);
}
@@ -2502,7 +2504,7 @@ static void nfs4_xdr_enc_readlink(struct
encode_readlink(xdr, args, req, &hdr);
rpc_prepare_reply_pages(req, args->pages, args->pgbase,
- args->pglen, hdr.replen);
+ args->pglen, hdr.replen - pagepad_maxsz);
encode_nops(&hdr);
}
@@ -2523,7 +2525,7 @@ static void nfs4_xdr_enc_readdir(struct
encode_readdir(xdr, args, req, &hdr);
rpc_prepare_reply_pages(req, args->pages, args->pgbase,
- args->count, hdr.replen);
+ args->count, hdr.replen - pagepad_maxsz);
encode_nops(&hdr);
}
@@ -2544,7 +2546,7 @@ static void nfs4_xdr_enc_read(struct rpc
encode_read(xdr, args, &hdr);
rpc_prepare_reply_pages(req, args->pages, args->pgbase,
- args->count, hdr.replen);
+ args->count, hdr.replen - pagepad_maxsz);
req->rq_rcv_buf.flags |= XDRBUF_READ;
encode_nops(&hdr);
}
@@ -2591,7 +2593,7 @@ static void nfs4_xdr_enc_getacl(struct r
ARRAY_SIZE(nfs4_acl_bitmap), &hdr);
rpc_prepare_reply_pages(req, args->acl_pages, 0,
- args->acl_len, replen + 1);
+ args->acl_len, replen);
encode_nops(&hdr);
}
@@ -2813,7 +2815,7 @@ static void nfs4_xdr_enc_fs_locations(st
}
rpc_prepare_reply_pages(req, (struct page **)&args->page, 0,
- PAGE_SIZE, replen + 1);
+ PAGE_SIZE, replen);
encode_nops(&hdr);
}
@@ -3017,14 +3019,14 @@ static void nfs4_xdr_enc_getdeviceinfo(s
encode_compound_hdr(xdr, req, &hdr);
encode_sequence(xdr, &args->seq_args, &hdr);
- replen = hdr.replen + op_decode_hdr_maxsz;
+ replen = hdr.replen + op_decode_hdr_maxsz + 2;
encode_getdeviceinfo(xdr, args, &hdr);
/* set up reply kvec. device_addr4 opaque data is read into the
* pages */
rpc_prepare_reply_pages(req, args->pdev->pages, args->pdev->pgbase,
- args->pdev->pglen, replen + 2 + 1);
+ args->pdev->pglen, replen);
encode_nops(&hdr);
}
@@ -3046,7 +3048,7 @@ static void nfs4_xdr_enc_layoutget(struc
encode_layoutget(xdr, args, &hdr);
rpc_prepare_reply_pages(req, args->layout.pages, 0,
- args->layout.pglen, hdr.replen);
+ args->layout.pglen, hdr.replen - pagepad_maxsz);
encode_nops(&hdr);
}
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1244,10 +1244,7 @@ void rpc_prepare_reply_pages(struct rpc_
unsigned int base, unsigned int len,
unsigned int hdrsize)
{
- /* Subtract one to force an extra word of buffer space for the
- * payload's XDR pad to fall into the rcv_buf's tail iovec.
- */
- hdrsize += RPC_REPHDRSIZE + req->rq_cred->cr_auth->au_ralign - 1;
+ hdrsize += RPC_REPHDRSIZE + req->rq_cred->cr_auth->au_ralign;
xdr_inline_pages(&req->rq_rcv_buf, hdrsize << 2, pages, base, len);
trace_rpc_reply_pages(req);
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -190,9 +190,6 @@ xdr_inline_pages(struct xdr_buf *xdr, un
tail->iov_base = buf + offset;
tail->iov_len = buflen - offset;
- if ((xdr->page_len & 3) == 0)
- tail->iov_len -= sizeof(__be32);
-
xdr->buflen += len;
}
EXPORT_SYMBOL_GPL(xdr_inline_pages);