2013-08-22 00:26:28

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 1/6] NFSv3: Deal with a sparse warning in nfs3_proc_create

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs3proc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index f5c84c3..1db588a 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -336,8 +336,8 @@ nfs3_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
data->arg.create.createmode = NFS3_CREATE_UNCHECKED;
if (flags & O_EXCL) {
data->arg.create.createmode = NFS3_CREATE_EXCLUSIVE;
- data->arg.create.verifier[0] = jiffies;
- data->arg.create.verifier[1] = current->pid;
+ data->arg.create.verifier[0] = cpu_to_be32(jiffies);
+ data->arg.create.verifier[1] = cpu_to_be32(current->pid);
}

sattr->ia_mode &= ~current_umask();
--
1.8.3.1



2013-08-22 12:55:11

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 4/6] NFSv4: Deal with a sparse warning in nfs_idmap_get_key()

On 08/21/2013 08:26 PM, Trond Myklebust wrote:
> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: Bryan Schumaker <[email protected]>
> ---
> fs/nfs/idmap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index c2c4163..65c7d91 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -310,7 +310,7 @@ static ssize_t nfs_idmap_get_key(const char *name, size_t namelen,
> if (ret < 0)
> goto out_up;
>
> - payload = rcu_dereference(rkey->payload.data);
> + payload = rcu_dereference(rkey->payload.rcudata);

Loks good to me!

Acked-by: Bryan Schumaker <[email protected]>

> if (IS_ERR_OR_NULL(payload)) {
> ret = PTR_ERR(payload);
> goto out_up;
>


2013-08-22 00:26:31

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 5/6] NFSv4: Fix an incorrect pointer declaration in decode_first_pnfs_layout_type

We always encode to __be32 format in XDR: silences a sparse warning.

Signed-off-by: Trond Myklebust <[email protected]>
Cc: Andy Adamson <[email protected]>
---
fs/nfs/nfs4xdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 1336263..4593728 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4630,7 +4630,7 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
static int decode_first_pnfs_layout_type(struct xdr_stream *xdr,
uint32_t *layouttype)
{
- uint32_t *p;
+ __be32 *p;
int num;

p = xdr_inline_decode(xdr, 4);
--
1.8.3.1


2013-08-22 12:49:28

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 3/6] NFSv4: Deal with some more sparse warnings


On Aug 21, 2013, at 8:26 PM, Trond Myklebust <[email protected]> wrote:

> Technically, we don't really need to convert these time stamps,
> since they are actually cookies.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: Chuck Lever <[email protected]>

Acked-by: Chuck Lever <[email protected]>


> ---
> fs/nfs/nfs4proc.c | 12 ++++++------
> fs/nfs/nfs4xdr.c | 2 +-
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 155c2fa..b577ebb 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1099,7 +1099,7 @@ static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stat
> goto no_delegation;
>
> spin_lock(&deleg_cur->lock);
> - if (nfsi->delegation != deleg_cur ||
> + if (rcu_dereference(nfsi->delegation) != deleg_cur ||
> test_bit(NFS_DELEGATION_RETURNING, &deleg_cur->flags) ||
> (deleg_cur->type & fmode) != fmode)
> goto no_delegation_unlock;
> @@ -4628,11 +4628,11 @@ static void nfs4_init_boot_verifier(const struct nfs_client *clp,
> /* An impossible timestamp guarantees this value
> * will never match a generated boot time. */
> verf[0] = 0;
> - verf[1] = (__be32)(NSEC_PER_SEC + 1);
> + verf[1] = cpu_to_be32(NSEC_PER_SEC + 1);
> } else {
> struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
> - verf[0] = (__be32)nn->boot_time.tv_sec;
> - verf[1] = (__be32)nn->boot_time.tv_nsec;
> + verf[0] = cpu_to_be32(nn->boot_time.tv_sec);
> + verf[1] = cpu_to_be32(nn->boot_time.tv_nsec);
> }
> memcpy(bootverf->data, verf, sizeof(bootverf->data));
> }
> @@ -7259,7 +7259,7 @@ static void nfs41_free_stateid_release(void *calldata)
> kfree(calldata);
> }
>
> -const struct rpc_call_ops nfs41_free_stateid_ops = {
> +static const struct rpc_call_ops nfs41_free_stateid_ops = {
> .rpc_call_prepare = nfs41_free_stateid_prepare,
> .rpc_call_done = nfs41_free_stateid_done,
> .rpc_release = nfs41_free_stateid_release,
> @@ -7479,7 +7479,7 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
> #endif
> };
>
> -const struct inode_operations nfs4_dir_inode_operations = {
> +static const struct inode_operations nfs4_dir_inode_operations = {
> .create = nfs_create,
> .lookup = nfs_lookup,
> .atomic_open = nfs_atomic_open,
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 1a4a3bd..1336263 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -1816,7 +1816,7 @@ static void encode_create_session(struct xdr_stream *xdr,
> *p++ = cpu_to_be32(RPC_AUTH_UNIX); /* auth_sys */
>
> /* authsys_parms rfc1831 */
> - *p++ = (__be32)nn->boot_time.tv_nsec; /* stamp */
> + *p++ = cpu_to_be32(nn->boot_time.tv_nsec); /* stamp */
> p = xdr_encode_opaque(p, machine_name, len);
> *p++ = cpu_to_be32(0); /* UID */
> *p++ = cpu_to_be32(0); /* GID */
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2013-08-22 14:21:35

by Peter Staubach

[permalink] [raw]
Subject: RE: [PATCH 1/6] NFSv3: Deal with a sparse warning in nfs3_proc_create

It seems a shame to byte swap something which doesn't really need to be byte swapped. It is pretty much just an opaque value that the server stores and then checks during possible retransmits. I suppose that it is cheap enough to do, not so many times per second.

Thanx...

ps


-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Trond Myklebust
Sent: Wednesday, August 21, 2013 8:26 PM
To: [email protected]
Subject: [PATCH 1/6] NFSv3: Deal with a sparse warning in nfs3_proc_create

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs3proc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c index f5c84c3..1db588a 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -336,8 +336,8 @@ nfs3_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
data->arg.create.createmode = NFS3_CREATE_UNCHECKED;
if (flags & O_EXCL) {
data->arg.create.createmode = NFS3_CREATE_EXCLUSIVE;
- data->arg.create.verifier[0] = jiffies;
- data->arg.create.verifier[1] = current->pid;
+ data->arg.create.verifier[0] = cpu_to_be32(jiffies);
+ data->arg.create.verifier[1] = cpu_to_be32(current->pid);
}

sattr->ia_mode &= ~current_umask();
--
1.8.3.1

2013-08-22 00:26:30

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 3/6] NFSv4: Deal with some more sparse warnings

Technically, we don't really need to convert these time stamps,
since they are actually cookies.

Signed-off-by: Trond Myklebust <[email protected]>
Cc: Chuck Lever <[email protected]>
---
fs/nfs/nfs4proc.c | 12 ++++++------
fs/nfs/nfs4xdr.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 155c2fa..b577ebb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1099,7 +1099,7 @@ static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stat
goto no_delegation;

spin_lock(&deleg_cur->lock);
- if (nfsi->delegation != deleg_cur ||
+ if (rcu_dereference(nfsi->delegation) != deleg_cur ||
test_bit(NFS_DELEGATION_RETURNING, &deleg_cur->flags) ||
(deleg_cur->type & fmode) != fmode)
goto no_delegation_unlock;
@@ -4628,11 +4628,11 @@ static void nfs4_init_boot_verifier(const struct nfs_client *clp,
/* An impossible timestamp guarantees this value
* will never match a generated boot time. */
verf[0] = 0;
- verf[1] = (__be32)(NSEC_PER_SEC + 1);
+ verf[1] = cpu_to_be32(NSEC_PER_SEC + 1);
} else {
struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
- verf[0] = (__be32)nn->boot_time.tv_sec;
- verf[1] = (__be32)nn->boot_time.tv_nsec;
+ verf[0] = cpu_to_be32(nn->boot_time.tv_sec);
+ verf[1] = cpu_to_be32(nn->boot_time.tv_nsec);
}
memcpy(bootverf->data, verf, sizeof(bootverf->data));
}
@@ -7259,7 +7259,7 @@ static void nfs41_free_stateid_release(void *calldata)
kfree(calldata);
}

-const struct rpc_call_ops nfs41_free_stateid_ops = {
+static const struct rpc_call_ops nfs41_free_stateid_ops = {
.rpc_call_prepare = nfs41_free_stateid_prepare,
.rpc_call_done = nfs41_free_stateid_done,
.rpc_release = nfs41_free_stateid_release,
@@ -7479,7 +7479,7 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
#endif
};

-const struct inode_operations nfs4_dir_inode_operations = {
+static const struct inode_operations nfs4_dir_inode_operations = {
.create = nfs_create,
.lookup = nfs_lookup,
.atomic_open = nfs_atomic_open,
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 1a4a3bd..1336263 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1816,7 +1816,7 @@ static void encode_create_session(struct xdr_stream *xdr,
*p++ = cpu_to_be32(RPC_AUTH_UNIX); /* auth_sys */

/* authsys_parms rfc1831 */
- *p++ = (__be32)nn->boot_time.tv_nsec; /* stamp */
+ *p++ = cpu_to_be32(nn->boot_time.tv_nsec); /* stamp */
p = xdr_encode_opaque(p, machine_name, len);
*p++ = cpu_to_be32(0); /* UID */
*p++ = cpu_to_be32(0); /* GID */
--
1.8.3.1


2013-08-22 00:26:30

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 4/6] NFSv4: Deal with a sparse warning in nfs_idmap_get_key()

Signed-off-by: Trond Myklebust <[email protected]>
Cc: Bryan Schumaker <[email protected]>
---
fs/nfs/idmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index c2c4163..65c7d91 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -310,7 +310,7 @@ static ssize_t nfs_idmap_get_key(const char *name, size_t namelen,
if (ret < 0)
goto out_up;

- payload = rcu_dereference(rkey->payload.data);
+ payload = rcu_dereference(rkey->payload.rcudata);
if (IS_ERR_OR_NULL(payload)) {
ret = PTR_ERR(payload);
goto out_up;
--
1.8.3.1


2013-08-22 00:26:29

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 2/6] NFSv4: Deal with a sparse warning in nfs4_opendata_alloc

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f50ad28..155c2fa 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -933,15 +933,11 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->o_arg.fh = NFS_FH(dentry->d_inode);
}
if (attrs != NULL && attrs->ia_valid != 0) {
- __be32 verf[2];
-
p->o_arg.u.attrs = &p->attrs;
memcpy(&p->attrs, attrs, sizeof(p->attrs));

- verf[0] = jiffies;
- verf[1] = current->pid;
- memcpy(p->o_arg.u.verifier.data, verf,
- sizeof(p->o_arg.u.verifier.data));
+ p->o_arg.u.verifier.data[0] = cpu_to_be32(jiffies);
+ p->o_arg.u.verifier.data[1] = cpu_to_be32(current->pid);
}
p->c_arg.fh = &p->o_res.fh;
p->c_arg.stateid = &p->o_res.stateid;
--
1.8.3.1


2013-08-22 00:26:31

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 6/6] NFS: Clean up nfs_sillyrename()

Optimise for the case where we only do one lookup.
Clean up the code so it is obvious that silly[] is not a dynamic array.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/unlink.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 60395ad..488fd16 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -444,6 +444,14 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
return rpc_run_task(&task_setup_data);
}

+#define SILLYNAME_PREFIX ".nfs"
+#define SILLYNAME_PREFIX_LEN ((unsigned)sizeof(SILLYNAME_PREFIX) - 1)
+#define SILLYNAME_FILEID_LEN ((unsigned)sizeof(u64) << 1)
+#define SILLYNAME_COUNTER_LEN ((unsigned)sizeof(unsigned int) << 1)
+#define SILLYNAME_LEN (SILLYNAME_PREFIX_LEN + \
+ SILLYNAME_FILEID_LEN + \
+ SILLYNAME_COUNTER_LEN)
+
/**
* nfs_sillyrename - Perform a silly-rename of a dentry
* @dir: inode of directory that contains dentry
@@ -469,10 +477,8 @@ int
nfs_sillyrename(struct inode *dir, struct dentry *dentry)
{
static unsigned int sillycounter;
- const int fileidsize = sizeof(NFS_FILEID(dentry->d_inode))*2;
- const int countersize = sizeof(sillycounter)*2;
- const int slen = sizeof(".nfs")+fileidsize+countersize-1;
- char silly[slen+1];
+ unsigned char silly[SILLYNAME_LEN + 1];
+ unsigned long long fileid;
struct dentry *sdentry;
struct rpc_task *task;
int error = -EIO;
@@ -489,20 +495,20 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
goto out;

- sprintf(silly, ".nfs%*.*Lx",
- fileidsize, fileidsize,
- (unsigned long long)NFS_FILEID(dentry->d_inode));
+ fileid = NFS_FILEID(dentry->d_inode);

/* Return delegation in anticipation of the rename */
NFS_PROTO(dentry->d_inode)->return_delegation(dentry->d_inode);

sdentry = NULL;
do {
- char *suffix = silly + slen - countersize;
-
+ int slen;
dput(sdentry);
sillycounter++;
- sprintf(suffix, "%*.*x", countersize, countersize, sillycounter);
+ slen = scnprintf(silly, sizeof(silly),
+ SILLYNAME_PREFIX "%0*llx%0*x",
+ SILLYNAME_FILEID_LEN, fileid,
+ SILLYNAME_COUNTER_LEN, sillycounter);

dfprintk(VFS, "NFS: trying to rename %s to %s\n",
dentry->d_name.name, silly);
--
1.8.3.1