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

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

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 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

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

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 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

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

[Trond Myklebust]

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

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Chuck Lever <Chuck.Lever@oracle.com>
---
 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

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

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Bryan Schumaker <bjschuma@netapp.com>
---
 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

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

[Trond Myklebust]

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

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Andy Adamson <andros@netapp.com>
---
 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

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

[Trond Myklebust]

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 <Trond.Myklebust@netapp.com>
---
 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

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

[Chuck Lever]

On Aug 21, 2013, at 8:26 PM, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> Technically, we don't really need to convert these time stamps,
> since they are actually cookies.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: Chuck Lever <Chuck.Lever@oracle.com>

Acked-by: Chuck Lever <chuck.lever@oracle.com>


> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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

[Bryan Schumaker]

On 08/21/2013 08:26 PM, Trond Myklebust wrote:
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: Bryan Schumaker <bjschuma@netapp.com>
> ---
>  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 <bjschuma@netapp.com>

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

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

[Peter Staubach]

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: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On Behalf Of Trond Myklebust
Sent: Wednesday, August 21, 2013 8:26 PM
To: linux-nfs@vger.kernel.org
Subject: [PATCH 1/6] NFSv3: Deal with a sparse warning in nfs3_proc_create

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html