From: Chuck Lever <chuck.lever@oracle.com>
To: trond.myklebust@netapp.com
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 14/15] NFS: Fix some minor problems with nfs4_verifiers
Date: Thu, 01 Mar 2012 17:02:14 -0500 [thread overview]
Message-ID: <20120301220213.2138.19498.stgit@degas.1015granger.net> (raw)
In-Reply-To: <20120301215755.2138.73488.stgit@degas.1015granger.net>
Clean up due to code review.
sizeof(nfs4_verifer) is the size of the in-core verifier data
structure, but NFS4_VERIFIER_SIZE is the number of octets in an XDR'd
verifier. The two are not interchangeable, even if they happen to
have the same value. If the nfs4_verifier struct is padded by the
compiler, sizeof(nfs4_verifier) may not be the same as the XDR'd
size. Not likely, but still.
Ensure that the data field in the nfs4_verifier structure is aligned
to the largest pointer type that is used to access it: in this case,
that's u32. Type-punning among types with different alignment has
been discouraged in user space and the kernel, to avoid unneeded
pointer aliasing. The use of a union is preferred instead.
As a micro-optimization, this change also ensures that the compiler
may perform memcpy() and memcmp() operations on these fields in
larger, more efficient, chunks.
Pull out some common nfs4_verifier XDR encoding logic into a helper.
I'm sure I missed a few spots.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfs/nfs4proc.c | 6 +++---
fs/nfs/nfs4xdr.c | 27 ++++++++++++++++-----------
fs/nfsd/nfs4proc.c | 4 ++--
fs/nfsd/nfs4state.c | 2 +-
fs/nfsd/nfs4xdr.c | 28 ++++++++++++++--------------
include/linux/nfs4.h | 5 ++++-
6 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 07bd1f3..cb08b1e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -837,7 +837,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->o_arg.u.attrs = &p->attrs;
memcpy(&p->attrs, attrs, sizeof(p->attrs));
- s = (u32 *) p->o_arg.u.verifier.data;
+ s = p->o_arg.u.verifier.data32;
s[0] = jiffies;
s[1] = current->pid;
}
@@ -3830,7 +3830,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
int loop = 0;
int status;
- p = (__be32*)sc_verifier.data;
+ p = (__be32*)sc_verifier.data32;
*p++ = htonl((u32)clp->cl_boot_time.tv_sec);
*p = htonl((u32)clp->cl_boot_time.tv_nsec);
@@ -4941,7 +4941,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
dprintk("--> %s\n", __func__);
BUG_ON(clp == NULL);
- p = (u32 *)verifier.data;
+ p = verifier.data32;
*p++ = htonl((u32)clp->cl_boot_time.tv_sec);
*p = htonl((u32)clp->cl_boot_time.tv_nsec);
args.verifier = &verifier;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 018bbd7..80ba010 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -906,13 +906,18 @@ static void encode_nops(struct compound_hdr *hdr)
*hdr->nops_p = htonl(hdr->nops);
}
+static __be32 *xdr_encode_nfs4_verifier(__be32 *p, const nfs4_verifier *verf)
+{
+ return xdr_encode_opaque_fixed(p, verf->data, NFS4_VERIFIER_SIZE);
+}
+
static void encode_nfs4_verifier(struct xdr_stream *xdr, const nfs4_verifier *verf)
{
__be32 *p;
p = xdr_reserve_space(xdr, NFS4_VERIFIER_SIZE);
BUG_ON(p == NULL);
- xdr_encode_opaque_fixed(p, verf->data, NFS4_VERIFIER_SIZE);
+ xdr_encode_nfs4_verifier(p, verf);
}
static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap, const struct nfs_server *server)
@@ -1571,7 +1576,7 @@ static void encode_readdir(struct xdr_stream *xdr, const struct nfs4_readdir_arg
p = reserve_space(xdr, 12+NFS4_VERIFIER_SIZE+20);
*p++ = cpu_to_be32(OP_READDIR);
p = xdr_encode_hyper(p, readdir->cookie);
- p = xdr_encode_opaque_fixed(p, readdir->verifier.data, NFS4_VERIFIER_SIZE);
+ p = xdr_encode_nfs4_verifier(p, &readdir->verifier);
*p++ = cpu_to_be32(dircount);
*p++ = cpu_to_be32(readdir->count);
*p++ = cpu_to_be32(2);
@@ -1583,8 +1588,8 @@ static void encode_readdir(struct xdr_stream *xdr, const struct nfs4_readdir_arg
dprintk("%s: cookie = %Lu, verifier = %08x:%08x, bitmap = %08x:%08x\n",
__func__,
(unsigned long long)readdir->cookie,
- ((u32 *)readdir->verifier.data)[0],
- ((u32 *)readdir->verifier.data)[1],
+ readdir->verifier.data32[0],
+ readdir->verifier.data32[1],
attrs[0] & readdir->bitmask[0],
attrs[1] & readdir->bitmask[1]);
}
@@ -1693,7 +1698,7 @@ static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclie
p = reserve_space(xdr, 4 + NFS4_VERIFIER_SIZE);
*p++ = cpu_to_be32(OP_SETCLIENTID);
- xdr_encode_opaque_fixed(p, setclientid->sc_verifier->data, NFS4_VERIFIER_SIZE);
+ xdr_encode_nfs4_verifier(p, setclientid->sc_verifier);
encode_string(xdr, setclientid->sc_name_len, setclientid->sc_name);
p = reserve_space(xdr, 4);
@@ -1713,7 +1718,7 @@ static void encode_setclientid_confirm(struct xdr_stream *xdr, const struct nfs4
p = reserve_space(xdr, 12 + NFS4_VERIFIER_SIZE);
*p++ = cpu_to_be32(OP_SETCLIENTID_CONFIRM);
p = xdr_encode_hyper(p, arg->clientid);
- xdr_encode_opaque_fixed(p, arg->confirm.data, NFS4_VERIFIER_SIZE);
+ xdr_encode_nfs4_verifier(p, &arg->confirm);
hdr->nops++;
hdr->replen += decode_setclientid_confirm_maxsz;
}
@@ -1770,9 +1775,9 @@ static void encode_exchange_id(struct xdr_stream *xdr,
{
__be32 *p;
- p = reserve_space(xdr, 4 + sizeof(args->verifier->data));
+ p = reserve_space(xdr, 4 + NFS4_VERIFIER_SIZE);
*p++ = cpu_to_be32(OP_EXCHANGE_ID);
- xdr_encode_opaque_fixed(p, args->verifier->data, sizeof(args->verifier->data));
+ xdr_encode_nfs4_verifier(p, args->verifier);
encode_string(xdr, args->id_len, args->id);
@@ -4205,7 +4210,7 @@ static int decode_close(struct xdr_stream *xdr, struct nfs_closeres *res)
static int decode_verifier(struct xdr_stream *xdr, void *verifier)
{
- return decode_opaque_fixed(xdr, verifier, 8);
+ return decode_opaque_fixed(xdr, verifier, NFS4_VERIFIER_SIZE);
}
static int decode_commit(struct xdr_stream *xdr, struct nfs_writeres *res)
@@ -4905,8 +4910,8 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n
return status;
dprintk("%s: verifier = %08x:%08x\n",
__func__,
- ((u32 *)readdir->verifier.data)[0],
- ((u32 *)readdir->verifier.data)[1]);
+ readdir->verifier.data32[0],
+ readdir->verifier.data32[1]);
hdrlen = (char *) xdr->p - (char *) iov->iov_base;
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 896da74..73ac8c6 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -221,7 +221,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o
status = do_nfsd_create(rqstp, current_fh, open->op_fname.data,
open->op_fname.len, &open->op_iattr,
&resfh, open->op_createmode,
- (u32 *)open->op_verf.data,
+ open->op_verf.data32,
&open->op_truncate, &open->op_created);
/*
@@ -485,7 +485,7 @@ static __be32
nfsd4_commit(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_commit *commit)
{
- u32 *p = (u32 *)commit->co_verf.data;
+ u32 *p = commit->co_verf.data32;
*p++ = nfssvc_boot.tv_sec;
*p++ = nfssvc_boot.tv_usec;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c5cddd6..c8ec181 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1141,7 +1141,7 @@ static void gen_confirm(struct nfs4_client *clp)
static u32 i;
u32 *p;
- p = (u32 *)clp->cl_confirm.data;
+ p = clp->cl_confirm.data32;
*p++ = get_seconds();
*p++ = i++;
}
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 0ec5a1b..f8dbc80 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -755,14 +755,14 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
goto out;
break;
case NFS4_CREATE_EXCLUSIVE:
- READ_BUF(8);
- COPYMEM(open->op_verf.data, 8);
+ READ_BUF(NFS4_VERIFIER_SIZE);
+ COPYMEM(open->verf.data, NFS4_VERIFIER_SIZE);
break;
case NFS4_CREATE_EXCLUSIVE4_1:
if (argp->minorversion < 1)
goto xdr_error;
- READ_BUF(8);
- COPYMEM(open->op_verf.data, 8);
+ READ_BUF(NFS4_VERIFIER_SIZE);
+ COPYMEM(open->verf.data, NFS4_VERIFIER_SIZE);
status = nfsd4_decode_fattr(argp, open->op_bmval,
&open->op_iattr, &open->op_acl);
if (status)
@@ -994,8 +994,8 @@ nfsd4_decode_setclientid(struct nfsd4_compoundargs *argp, struct nfsd4_setclient
{
DECODE_HEAD;
- READ_BUF(8);
- COPYMEM(setclientid->se_verf.data, 8);
+ READ_BUF(NFS4_VERIFIER_SIZE);
+ COPYMEM(setclientid->se_verf.data, NFS4_VERIFIER_SIZE);
status = nfsd4_decode_opaque(argp, &setclientid->se_name);
if (status)
@@ -1020,9 +1020,9 @@ nfsd4_decode_setclientid_confirm(struct nfsd4_compoundargs *argp, struct nfsd4_s
{
DECODE_HEAD;
- READ_BUF(8 + sizeof(nfs4_verifier));
+ READ_BUF(8 + NFS4_VERIFIER_SIZE);
COPYMEM(&scd_c->sc_clientid, 8);
- COPYMEM(&scd_c->sc_confirm, sizeof(nfs4_verifier));
+ COPYMEM(&scd_c->sc_confirm, NFS4_VERIFIER_SIZE);
DECODE_TAIL;
}
@@ -2661,8 +2661,8 @@ nfsd4_encode_commit(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
__be32 *p;
if (!nfserr) {
- RESERVE_SPACE(8);
- WRITEMEM(commit->co_verf.data, 8);
+ RESERVE_SPACE(NFS4_VERIFIER_SIZE);
+ WRITEMEM(commit->co_verf.data, NFS4_VERIFIER_SIZE);
ADJUST_ARGS();
}
return nfserr;
@@ -3008,7 +3008,7 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
if (resp->xbuf->page_len)
return nfserr_resource;
- RESERVE_SPACE(8); /* verifier */
+ RESERVE_SPACE(NFS4_VERIFIER_SIZE);
savep = p;
/* XXX: Following NFSv3, we ignore the READDIR verifier for now. */
@@ -3209,9 +3209,9 @@ nfsd4_encode_setclientid(struct nfsd4_compoundres *resp, __be32 nfserr, struct n
__be32 *p;
if (!nfserr) {
- RESERVE_SPACE(8 + sizeof(nfs4_verifier));
+ RESERVE_SPACE(8 + NFS4_VERIFIER_SIZE);
WRITEMEM(&scd->se_clientid, 8);
- WRITEMEM(&scd->se_confirm, sizeof(nfs4_verifier));
+ WRITEMEM(&scd->se_confirm, NFS4_VERIFIER_SIZE);
ADJUST_ARGS();
}
else if (nfserr == nfserr_clid_inuse) {
@@ -3232,7 +3232,7 @@ nfsd4_encode_write(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_w
RESERVE_SPACE(16);
WRITE32(write->wr_bytes_written);
WRITE32(write->wr_how_written);
- WRITEMEM(write->wr_verifier.data, 8);
+ WRITEMEM(write->wr_verifier.data, NFS4_VERIFIER_SIZE);
ADJUST_ARGS();
}
return nfserr;
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 32345c2..b1e6b64 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -181,7 +181,10 @@ struct nfs4_acl {
struct nfs4_ace aces[0];
};
-typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier;
+typedef union {
+ unsigned char data[NFS4_VERIFIER_SIZE];
+ u32 data32[NFS4_VERIFIER_SIZE / sizeof(u32)];
+} nfs4_verifier;
struct nfs41_stateid {
__be32 seqid;
next prev parent reply other threads:[~2012-03-01 22:02 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-01 22:00 [PATCH 00/15] For 3.4 (2nd take) Chuck Lever
2012-03-01 22:00 ` [PATCH 01/15] NFS: Make nfs_cache_array.size a signed integer Chuck Lever
2012-03-01 22:00 ` [PATCH 02/15] NFS: Clean up debugging in decode_pathname() Chuck Lever
2012-03-01 22:00 ` [PATCH 03/15] NFS: Add debugging messages to NFSv4's CLOSE procedure Chuck Lever
2012-03-01 22:00 ` [PATCH 04/15] NFS: Reduce debugging noise from encode_compound_hdr Chuck Lever
2012-03-01 22:15 ` Adamson, Dros
2012-03-01 22:19 ` Myklebust, Trond
2012-03-01 22:00 ` [PATCH 05/15] SUNRPC: Use RCU to dereference the rpc_clnt.cl_xprt field Chuck Lever
2012-03-01 22:01 ` [PATCH 06/15] SUNRPC: Move clnt->cl_server into struct rpc_xprt Chuck Lever
2012-03-01 22:01 ` [PATCH 07/15] SUNRPC: Add API to acquire source address Chuck Lever
2012-03-01 22:09 ` Jim Rees
2012-03-01 22:27 ` Chuck Lever
2012-03-01 22:01 ` [PATCH 08/15] commit 6f38b4ba433ac6494f83cb73dd07dcbde797e1e0 Chuck Lever
2012-03-01 22:01 ` [PATCH 09/15] NFS: Add a client-side function to display NFS file handles Chuck Lever
2012-03-01 22:28 ` Myklebust, Trond
2012-03-01 22:32 ` Chuck Lever
2012-03-02 17:17 ` Steve Dickson
2012-03-02 17:19 ` Chuck Lever
2012-03-02 18:50 ` Steve Dickson
2012-03-06 16:55 ` Adamson, Dros
2012-03-01 22:01 ` [PATCH 10/15] NFS: Save root file handle in nfs_server Chuck Lever
2012-03-01 22:30 ` Myklebust, Trond
2012-03-01 22:01 ` [PATCH 11/15] NFS: Simplify arguments of encode_renew() Chuck Lever
2012-03-01 22:01 ` [PATCH 12/15] NFS: Introduce NFS_ATTR_FATTR_V4_LOCATIONS Chuck Lever
2012-03-01 22:02 ` [PATCH 13/15] NFS: Request fh_expire_type attribute in "server caps" operation Chuck Lever
2012-03-01 22:02 ` Chuck Lever [this message]
2012-03-01 22:35 ` [PATCH 14/15] NFS: Fix some minor problems with nfs4_verifiers Myklebust, Trond
2012-03-01 22:43 ` Myklebust, Trond
2012-03-01 22:02 ` [PATCH 15/15] NFS: Fix some minor problems with nfs4_deviceids Chuck Lever
2012-03-01 22:39 ` Myklebust, Trond
2012-03-01 22:55 ` Chuck Lever
2012-03-02 18:10 ` Chuck Lever
2012-03-02 21:58 ` Boaz Harrosh
2012-03-02 22:00 ` Boaz Harrosh
2012-03-02 22:03 ` Chuck Lever
2012-03-02 22:06 ` Boaz Harrosh
2012-03-02 22:11 ` Chuck Lever
2012-03-02 22:52 ` Boaz Harrosh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120301220213.2138.19498.stgit@degas.1015granger.net \
--to=chuck.lever@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@netapp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).