[PATCH 00/13] For 3.4 (try 2)

[PATCH 00/13] For 3.4 (try 2)

[Chuck Lever]

Hi-

[ DSL crapped out on me halfway through posting this series.
  Let's try again. ]

I'm sure a hunk here or there is no longer necessary.  This is the
set of minor clean ups that have been hanging around in my tree for
about a year, as part of a patch series to support NFSv4.0 migration.

Please consider for inclusion in 3.4.

---

Chuck Lever (13):
      NFS: Request fh_expire_type attribute in "server caps" operation
      NFS: Introduce NFS_ATTR_FATTR_V4_LOCATIONS
      NFS: Simplify arguments of encode_renew()
      NFS: Save root file handle in nfs_server
      NFS: Add a client-side function to display NFS file handles
      SUNRPC: Add API to acquire source address
      NFS: Reduce debugging noise from encode_compound_hdr
      NFS: Add debugging messages to NFSv4's CLOSE procedure
      NFS: Clean up debugging in decode_pathname()
      nfs: Clean up debugging in nfs_follow_mountpoint()
      SUNRPC: Use KERN_DEFAULT for debugging printk's
      NFS: Fix comparison signage warnings with slot ID computations
      NFS: Make nfs_cache_array.size a signed integer


 fs/nfs/client.c              |    1 
 fs/nfs/dir.c                 |    2 -
 fs/nfs/getroot.c             |    5 +
 fs/nfs/inode.c               |   45 +++++++++++++
 fs/nfs/namespace.c           |    5 +
 fs/nfs/nfs4proc.c            |   24 ++++---
 fs/nfs/nfs4xdr.c             |   54 ++++++++++++---
 include/linux/nfs_fs.h       |   14 ++++
 include/linux/nfs_fs_sb.h    |    4 +
 include/linux/nfs_xdr.h      |    8 +-
 include/linux/sunrpc/clnt.h  |    1 
 include/linux/sunrpc/debug.h |    6 +-
 net/sunrpc/clnt.c            |  149 ++++++++++++++++++++++++++++++++++++++++++
 13 files changed, 290 insertions(+), 28 deletions(-)

-- 
Chuck Lever

[PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer

[Chuck Lever]

Eliminate a number of implicit type casts in comparisons, and these
compiler warnings:

fs/nfs/dir.c: In function ‘nfs_readdir_clear_array’:
fs/nfs/dir.c:264:16: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]
fs/nfs/dir.c: In function ‘nfs_readdir_search_for_cookie’:
fs/nfs/dir.c:352:16: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]
fs/nfs/dir.c: In function ‘nfs_do_filldir’:
fs/nfs/dir.c:769:38: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]
fs/nfs/dir.c:780:9: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]

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

 fs/nfs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ac28990..8661e13 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -207,7 +207,7 @@ struct nfs_cache_array_entry {
 };
 
 struct nfs_cache_array {
-	unsigned int size;
+	int size;
 	int eof_index;
 	u64 last_cookie;
 	struct nfs_cache_array_entry array[0];

[PATCH 02/13] NFS: Fix comparison signage warnings with slot ID computations

[Chuck Lever]

To make nfs4proc.c compile cleanly, address the following compiler
warnings:

fs/nfs/nfs4proc.c: In function ‘nfs4_free_slot’:
fs/nfs/nfs4proc.c:371:2: warning: comparison between signed and
		unsigned integer expressions [-Wsign-compare]
fs/nfs/nfs4proc.c: In function ‘nfs4_reset_slot_table’:
fs/nfs/nfs4proc.c:5000:15: warning: comparison between signed and
		unsigned integer expressions [-Wsign-compare]
fs/nfs/nfs4proc.c: In function ‘nfs4_init_slot_table’:
fs/nfs/nfs4proc.c:5064:2: warning: comparison between signed and
		unsigned integer expressions [-Wsign-compare]

Why is max_reqs always u32, but nfs4_slot_table.max_slots a signed
integer?

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

 fs/nfs/nfs4proc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d9f4d78..5ff9c5a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -368,7 +368,7 @@ nfs4_free_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *free_slot)
 	int free_slotid = free_slot - tbl->slots;
 	int slotid = free_slotid;
 
-	BUG_ON(slotid < 0 || slotid >= NFS4_MAX_SLOT_TABLE);
+	BUG_ON(slotid < 0 || slotid >= (int)NFS4_MAX_SLOT_TABLE);
 	/* clear used bit in bitmap */
 	__clear_bit(slotid, tbl->used_slots);
 
@@ -4997,7 +4997,7 @@ static int nfs4_reset_slot_table(struct nfs4_slot_table *tbl, u32 max_reqs,
 		max_reqs, tbl->max_slots);
 
 	/* Does the newly negotiated max_reqs match the existing slot table? */
-	if (max_reqs != tbl->max_slots) {
+	if (max_reqs != (u32)tbl->max_slots) {
 		ret = -ENOMEM;
 		new = kmalloc(max_reqs * sizeof(struct nfs4_slot),
 			      GFP_NOFS);
@@ -5056,7 +5056,7 @@ static void nfs4_destroy_slot_tables(struct nfs4_session *session)
  * Initialize slot table
  */
 static int nfs4_init_slot_table(struct nfs4_slot_table *tbl,
-		int max_slots, int ivalue)
+		u32 max_slots, int ivalue)
 {
 	struct nfs4_slot *slot;
 	int ret = -ENOMEM;

[PATCH 03/13] SUNRPC: Use KERN_DEFAULT for debugging printk's

[Chuck Lever]

Our dprintk() debugging facility doesn't specify any verbosity level
for it's printk() calls, but it should.

The default verbosity for printk's is KERN_DEFAULT.  You might argue
that these are debugging printk's and thus the verbosity should be
KERN_DEBUG.  That would mean that to see NFS and SUNRPC debugging
output an admin would also have to boost the syslog verbosity, which
would be insufferably noisy.

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

 include/linux/sunrpc/debug.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index c2786f2..e0927c2 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -51,7 +51,11 @@ extern unsigned int		nlm_debug;
 #undef ifdebug
 #ifdef RPC_DEBUG			
 # define ifdebug(fac)		if (unlikely(rpc_debug & RPCDBG_##fac))
-# define dfprintk(fac, args...)	do { ifdebug(fac) printk(args); } while(0)
+# define dfprintk(fac, args...)	\
+	do { \
+		ifdebug(fac) \
+			printk(KERN_DEFAULT args); \
+	} while (0)
 # define RPC_IFDEBUG(x)		x
 #else
 # define ifdebug(fac)		if (0)

[PATCH 04/13] nfs: Clean up debugging in nfs_follow_mountpoint()

[Chuck Lever]

Clean up: Fix a debugging message which had an obsolete function name
in it (nfs_follow_mountpoint).

Introduced by commit 36d43a43 "NFS: Use d_automount() rather than
abusing follow_link()" (January 14, 2011)

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

 fs/nfs/namespace.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 8102391..1807866 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -276,7 +276,10 @@ out:
 	nfs_free_fattr(fattr);
 	nfs_free_fhandle(fh);
 out_nofree:
-	dprintk("<-- nfs_follow_mountpoint() = %p\n", mnt);
+	if (IS_ERR(mnt))
+		dprintk("<-- %s(): error %ld\n", __func__, PTR_ERR(mnt));
+	else
+		dprintk("<-- %s() = %p\n", __func__, mnt);
 	return mnt;
 }
 

[PATCH 05/13] NFS: Clean up debugging in decode_pathname()

[Chuck Lever]

I noticed recently that decode_attr_fs_locations() is not generating
very pretty debugging output.  The pathname components each appear on
a separate line of output, though that does not appear to be the
intended display behavior.  The preferred way to generate continued
lines of output on the console is to use pr_cont().

Note that incoming pathname4 components contain a string that is not
necessarily NUL-terminated.  I did actually see some trailing garbage
on the console.  In addition to correcting the line continuation
problem, add a string precision format specifier to ensure that each
component string is displayed properly, and that vsnprintf() does
not Oops.

Someone pointed out that allowing incoming network data to possibly
generate a console line of unbounded length may not be such a good
idea.  Since this output will rarely be enabled, and there is a hard
upper bound (NFS4_PATHNAME_MAXCOMPONENTS) in our implementation, this
is probably not a major concern.

It might be useful to additionally sanity-check the length of each
incoming component, however.  RFC 3530bis15 does not suggest a maximum
number of UTF-8 characters per component for either the pathname4 or
component4 types.  However, we could invent one that is appropriate
for our implementation.

Another possibility is to scrap all of this and print these pathnames
in upper layers after a reasonable amount of sanity checking in the
XDR layer.  This would give us an opportunity to allocate a full
buffer so that the whole pathname would be output via a single
dprintk.

Introduced by commit 7aaa0b3b: "NFSv4: convert fs-locations-components
to conform to RFC3530," (June 9, 2006).

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

 fs/nfs/nfs4xdr.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 51ef966..606852a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -3512,16 +3512,17 @@ static int decode_pathname(struct xdr_stream *xdr, struct nfs4_pathname *path)
 	n = be32_to_cpup(p);
 	if (n == 0)
 		goto root_path;
-	dprintk("path ");
+	dprintk("pathname4: ");
 	path->ncomponents = 0;
 	while (path->ncomponents < n) {
 		struct nfs4_string *component = &path->components[path->ncomponents];
 		status = decode_opaque_inline(xdr, &component->len, &component->data);
 		if (unlikely(status != 0))
 			goto out_eio;
-		if (path->ncomponents != n)
-			dprintk("/");
-		dprintk("%s", component->data);
+		if (unlikely(nfs_debug & NFSDBG_XDR))
+			pr_cont("%s%.*s ",
+				(path->ncomponents != n ? "/ " : ""),
+				component->len, component->data);
 		if (path->ncomponents < NFS4_PATHNAME_MAXCOMPONENTS)
 			path->ncomponents++;
 		else {
@@ -3530,14 +3531,13 @@ static int decode_pathname(struct xdr_stream *xdr, struct nfs4_pathname *path)
 		}
 	}
 out:
-	dprintk("\n");
 	return status;
 root_path:
 /* a root pathname is sent as a zero component4 */
 	path->ncomponents = 1;
 	path->components[0].len=0;
 	path->components[0].data=NULL;
-	dprintk("path /\n");
+	dprintk("pathname4: /\n");
 	goto out;
 out_eio:
 	dprintk(" status %d", status);
@@ -3559,7 +3559,7 @@ static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, st
 	status = 0;
 	if (unlikely(!(bitmap[0] & FATTR4_WORD0_FS_LOCATIONS)))
 		goto out;
-	dprintk("%s: fsroot ", __func__);
+	dprintk("%s: fsroot:\n", __func__);
 	status = decode_pathname(xdr, &res->fs_path);
 	if (unlikely(status != 0))
 		goto out;
@@ -3580,7 +3580,7 @@ static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, st
 		m = be32_to_cpup(p);
 
 		loc->nservers = 0;
-		dprintk("%s: servers ", __func__);
+		dprintk("%s: servers:\n", __func__);
 		while (loc->nservers < m) {
 			struct nfs4_string *server = &loc->servers[loc->nservers];
 			status = decode_opaque_inline(xdr, &server->len, &server->data);

[PATCH 06/13] NFS: Add debugging messages to NFSv4's CLOSE procedure

[Chuck Lever]

CLOSE is new with NFSv4.  Sometimes it's important to know the timing
of this operation compared to things like lease renewal.

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

 fs/nfs/nfs4proc.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5ff9c5a..5629858 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1947,6 +1947,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 	struct nfs4_state *state = calldata->state;
 	struct nfs_server *server = NFS_SERVER(calldata->inode);
 
+	dprintk("%s: begin!\n", __func__);
 	if (!nfs4_sequence_done(task, &calldata->res.seq_res))
 		return;
         /* hmm. we are done with the inode, and in the process of freeing
@@ -1974,6 +1975,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 	}
 	nfs_release_seqid(calldata->arg.seqid);
 	nfs_refresh_inode(calldata->inode, calldata->res.fattr);
+	dprintk("%s: done, ret = %d!\n", __func__, task->tk_status);
 }
 
 static void nfs4_close_prepare(struct rpc_task *task, void *data)
@@ -1982,6 +1984,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 	struct nfs4_state *state = calldata->state;
 	int call_close = 0;
 
+	dprintk("%s: begin!\n", __func__);
 	if (nfs_wait_on_sequence(calldata->arg.seqid, task) != 0)
 		return;
 
@@ -2006,7 +2009,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 	if (!call_close) {
 		/* Note: exit _without_ calling nfs4_close_done */
 		task->tk_action = NULL;
-		return;
+		goto out;
 	}
 
 	if (calldata->arg.fmode == 0) {
@@ -2015,7 +2018,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 		    pnfs_roc_drain(calldata->inode, &calldata->roc_barrier)) {
 			rpc_sleep_on(&NFS_SERVER(calldata->inode)->roc_rpcwaitq,
 				     task, NULL);
-			return;
+			goto out;
 		}
 	}
 
@@ -2024,8 +2027,10 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 	if (nfs4_setup_sequence(NFS_SERVER(calldata->inode),
 				&calldata->arg.seq_args, &calldata->res.seq_res,
 				1, task))
-		return;
+		goto out;
 	rpc_call_start(task);
+out:
+	dprintk("%s: done!\n", __func__);
 }
 
 static const struct rpc_call_ops nfs4_close_ops = {

[PATCH 07/13] NFS: Reduce debugging noise from encode_compound_hdr

[Chuck Lever]

When NFSDBG_XDR debugging is enabled, I see a lot of

  encode_compound: tag=

on the console.  This is noise if the tag is empty.  Some might argue
that it is noise even if the tag isn't empty...

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

 fs/nfs/nfs4xdr.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 606852a..2fb129d 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -889,7 +889,9 @@ static void encode_compound_hdr(struct xdr_stream *xdr,
 	 * but this is not required as a MUST for the server to do so. */
 	hdr->replen = RPC_REPHDRSIZE + auth->au_rslack + 3 + hdr->taglen;
 
-	dprintk("encode_compound: tag=%.*s\n", (int)hdr->taglen, hdr->tag);
+	if (unlikely(hdr->taglen != 0))
+		dprintk("%s: tag=%.*s\n", __func__,
+				(int)hdr->taglen, hdr->tag);
 	BUG_ON(hdr->taglen > NFS4_MAXTAGLEN);
 	p = reserve_space(xdr, 4 + hdr->taglen + 8);
 	p = xdr_encode_opaque(p, hdr->tag, hdr->taglen);

[PATCH 08/13] SUNRPC: Add API to acquire source address

[Chuck Lever]

NFSv4.0 clients must send endpoint information for their callback
service to NFSv4.0 servers during their first contact with a server.
Traditionally on Linux, user space provides the callback endpoint IP
address via the "clientaddr=" mount option.

During an NFSv4 migration event, it is possible that an FSID may be
migrated to a destination server that is accessible via a different
source IP address than the source server was.  The client must update
callback endpoint information on the destination server so that it can
maintain leases and allow delegation.

Without a new "clientaddr=" option from user space, however, the
kernel itself must construct an appropriate IP address for the
callback update.  Provide an API in the RPC client for upper layer
RPC consumers to acquire a source address for a remote.

The mechanism used by the mount.nfs command is copied: set up a
connected UDP socket to the designated remote, then scrape the source
address off the socket.  We are careful to select the correct network
namespace when setting up the temporary UDP socket.

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

 include/linux/sunrpc/clnt.h |    1 
 net/sunrpc/clnt.c           |  149 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 0 deletions(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 3d8f9c4..6a869b4 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -160,6 +160,7 @@ size_t		rpc_max_payload(struct rpc_clnt *);
 void		rpc_force_rebind(struct rpc_clnt *);
 size_t		rpc_peeraddr(struct rpc_clnt *, struct sockaddr *, size_t);
 const char	*rpc_peeraddr2str(struct rpc_clnt *, enum rpc_display_format_t);
+int		rpc_localaddr(struct rpc_clnt *, struct sockaddr *, size_t);
 
 size_t		rpc_ntop(const struct sockaddr *, char *, const size_t);
 size_t		rpc_pton(const char *, const size_t,
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index f0268ea..ebd2192 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -806,6 +806,155 @@ const char *rpc_peeraddr2str(struct rpc_clnt *clnt,
 }
 EXPORT_SYMBOL_GPL(rpc_peeraddr2str);
 
+static const struct sockaddr_in rpc_inaddr_loopback = {
+	.sin_family		= AF_INET,
+	.sin_addr.s_addr	= htonl(INADDR_ANY),
+};
+
+static const struct sockaddr_in6 rpc_in6addr_loopback = {
+	.sin6_family		= AF_INET6,
+	.sin6_addr		= IN6ADDR_ANY_INIT,
+};
+
+/*
+ * Try a getsockname() on a connected datagram socket.  Using a
+ * connected datagram socket prevents leaving a socket in TIME_WAIT.
+ * This conserves the ephemeral port number space.
+ *
+ * Returns zero and fills in "buf" if successful; otherwise, a
+ * negative errno is returned.
+ */
+static int rpc_sockname(struct net *net, struct sockaddr *sap, size_t salen,
+			struct sockaddr *buf, int buflen)
+{
+	struct socket *sock;
+	int err;
+
+	err = __sock_create(net, sap->sa_family,
+				SOCK_DGRAM, IPPROTO_UDP, &sock, 1);
+	if (err < 0) {
+		dprintk("RPC:       can't create UDP socket (%d)\n", err);
+		goto out;
+	}
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		err = kernel_bind(sock,
+				(struct sockaddr *)&rpc_inaddr_loopback,
+				sizeof(rpc_inaddr_loopback));
+		break;
+	case AF_INET6:
+		err = kernel_bind(sock,
+				(struct sockaddr *)&rpc_in6addr_loopback,
+				sizeof(rpc_in6addr_loopback));
+		break;
+	default:
+		err = -EAFNOSUPPORT;
+		goto out;
+	}
+	if (err < 0) {
+		dprintk("RPC:       can't bind UDP socket (%d)\n", err);
+		goto out_release;
+	}
+
+	err = kernel_connect(sock, sap, salen, 0);
+	if (err < 0) {
+		dprintk("RPC:       can't connect UDP socket (%d)\n", err);
+		goto out_release;
+	}
+
+	err = kernel_getsockname(sock, buf, &buflen);
+	if (err < 0) {
+		dprintk("RPC:       getsockname failed (%d)\n", err);
+		goto out_release;
+	}
+
+	err = 0;
+	if (buf->sa_family == AF_INET6) {
+		struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)buf;
+		sin6->sin6_scope_id = 0;
+	}
+	dprintk("RPC:       %s succeeded\n", __func__);
+
+out_release:
+	sock_release(sock);
+out:
+	return err;
+}
+
+/*
+ * Scraping a connected socket failed, so we don't have a useable
+ * local address.  Fallback: generate an address that will prevent
+ * the server from calling us back.
+ *
+ * Returns zero and fills in "buf" if successful; otherwise, a
+ * negative errno is returned.
+ */
+static int rpc_anyaddr(int family, struct sockaddr *buf, size_t buflen)
+{
+	switch (family) {
+	case AF_INET:
+		if (buflen < sizeof(rpc_inaddr_loopback))
+			return -EINVAL;
+		memcpy(buf, &rpc_inaddr_loopback,
+				sizeof(rpc_inaddr_loopback));
+		break;
+	case AF_INET6:
+		if (buflen < sizeof(rpc_in6addr_loopback))
+			return -EINVAL;
+		memcpy(buf, &rpc_in6addr_loopback,
+				sizeof(rpc_in6addr_loopback));
+	default:
+		dprintk("RPC:       %s: address family not supported\n",
+			__func__);
+		return -EAFNOSUPPORT;
+	}
+	dprintk("RPC:       %s: succeeded\n", __func__);
+	return 0;
+}
+
+/**
+ * rpc_localaddr - discover local endpoint address for an RPC client
+ * @clnt: RPC client structure
+ * @buf: target buffer
+ * @buflen: size of target buffer, in bytes
+ *
+ * Returns zero and fills in "buf" and "buflen" if successful;
+ * otherwise, a negative errno is returned.
+ *
+ * This works even if the underlying transport is not currently connected,
+ * or if the upper layer never previously provided a source address.
+ *
+ * The result of this function call is transient: multiple calls in
+ * succession may give different results, depending on how local
+ * networking configuration changes over time.
+ */
+int rpc_localaddr(struct rpc_clnt *clnt, struct sockaddr *buf, size_t buflen)
+{
+	struct sockaddr_storage address;
+	struct sockaddr *sap = (struct sockaddr *)&address;
+	struct rpc_xprt *xprt;
+	struct net *net;
+	size_t salen;
+	int err;
+
+	rcu_read_lock();
+	xprt = rcu_dereference(clnt->cl_xprt);
+	salen = xprt->addrlen;
+	memcpy(sap, &xprt->addr, salen);
+	net = get_net(xprt->xprt_net);
+	rcu_read_unlock();
+
+	rpc_set_port(sap, 0);
+	err = rpc_sockname(net, sap, salen, buf, buflen);
+	put_net(net);
+	if (err != 0)
+		/* Couldn't discover local address, return ANYADDR */
+		return rpc_anyaddr(sap->sa_family, buf, buflen);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rpc_localaddr);
+
 void
 rpc_setbufsize(struct rpc_clnt *clnt, unsigned int sndsize, unsigned int rcvsize)
 {

[PATCH 09/13] NFS: Add a client-side function to display NFS file handles

[Chuck Lever]

For debugging, introduce a simplistic function to print NFS file
handles on the system console.  The main function is hooked into the
dprintk debugging facility, but you can directly call the helper,
_nfs_display_fhandle(), if you want to print a handle unconditionally.

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

 fs/nfs/inode.c         |   45 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs_fs.h |   14 ++++++++++++++
 2 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 5c9598f..d36c1cb 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1076,6 +1076,51 @@ struct nfs_fh *nfs_alloc_fhandle(void)
 }
 
 /**
+ * _nfs_display_fhandle - display an NFS file handle on the console
+ *
+ * @fh: file handle to display
+ * @caption: display caption
+ *
+ * For debugging only.
+ */
+#ifdef RPC_DEBUG
+void _nfs_display_fhandle(const struct nfs_fh *fh, const char *caption)
+{
+	unsigned short i;
+
+	if (fh->size == 0 || fh == NULL) {
+		printk(KERN_DEFAULT "%s at %p is empty\n", caption, fh);
+		return;
+	}
+
+	printk(KERN_DEFAULT "%s at %p is %u bytes:\n", caption, fh, fh->size);
+	for (i = 0; i < fh->size; i += 16) {
+		__be32 *pos = (__be32 *)&fh->data[i];
+
+		switch ((fh->size - i - 1) >> 2) {
+		case 0:
+			printk(KERN_DEFAULT " %08x\n",
+				be32_to_cpup(pos));
+			break;
+		case 1:
+			printk(KERN_DEFAULT " %08x %08x\n",
+				be32_to_cpup(pos), be32_to_cpup(pos + 1));
+			break;
+		case 2:
+			printk(KERN_DEFAULT " %08x %08x %08x\n",
+				be32_to_cpup(pos), be32_to_cpup(pos + 1),
+				be32_to_cpup(pos + 2));
+			break;
+		default:
+			printk(KERN_DEFAULT " %08x %08x %08x %08x\n",
+				be32_to_cpup(pos), be32_to_cpup(pos + 1),
+				be32_to_cpup(pos + 2), be32_to_cpup(pos + 3));
+		}
+	}
+}
+#endif
+
+/**
  * nfs_inode_attrs_need_update - check if the inode attributes need updating
  * @inode - pointer to inode
  * @fattr - attributes
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 2e4601a..43b9062 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -399,6 +399,20 @@ static inline void nfs_free_fhandle(const struct nfs_fh *fh)
 	kfree(fh);
 }
 
+#ifdef RPC_DEBUG
+extern void _nfs_display_fhandle(const struct nfs_fh *fh, const char *caption);
+#define nfs_display_fhandle(fh, caption)			\
+	do {							\
+		if (unlikely(nfs_debug & NFSDBG_FACILITY))	\
+			_nfs_display_fhandle(fh, caption);	\
+	} while (0)
+#else
+static inline void nfs_display_fhandle(const struct nfs_fh *fh,
+				       const char *caption)
+{
+}
+#endif
+
 /*
  * linux/fs/nfs/nfsroot.c
  */

[PATCH 10/13] NFS: Save root file handle in nfs_server

[Chuck Lever]

Save each FSID's root directory file handle in the FSID's nfs_server
structure on the client.  For now, only NFSv4 mounts save the root FH.
This is needed for migration recovery.

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

 fs/nfs/client.c           |    1 +
 fs/nfs/getroot.c          |    5 +++++
 include/linux/nfs_fs_sb.h |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 200e1d8..ada7a05 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1112,6 +1112,7 @@ void nfs_free_server(struct nfs_server *server)
 	nfs_put_client(server->nfs_client);
 
 	nfs_free_iostats(server->io_stats);
+	nfs_free_fhandle(server->rootfh);
 	bdi_destroy(&server->backing_dev_info);
 	kfree(server);
 	nfs_release_automount_timer();
diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
index dcb6154..8d4fbe1 100644
--- a/fs/nfs/getroot.c
+++ b/fs/nfs/getroot.c
@@ -232,6 +232,11 @@ struct dentry *nfs4_get_root(struct super_block *sb, struct nfs_fh *mntfh,
 		ret = ERR_CAST(inode);
 		goto out;
 	}
+	server->rootfh = nfs_alloc_fhandle();
+	if (server->rootfh != NULL) {
+		nfs_display_fhandle(mntfh, "nfs_get_root: new root FH");
+		nfs_copy_fh(server->rootfh, mntfh);
+	}
 
 	error = nfs_superblock_set_dummy_root(sb, inode);
 	if (error != 0) {
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index ba4d765..6532d7b 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -157,6 +157,7 @@ struct nfs_server {
 	struct list_head	layouts;
 	struct list_head	delegations;
 	void (*destroy)(struct nfs_server *);
+	struct nfs_fh		*rootfh;
 
 	atomic_t active; /* Keep trace of any activity to this server */
 

[PATCH 11/13] NFS: Simplify arguments of encode_renew()

[Chuck Lever]

Clean up: pass just the piece of data that is needed to encode_renew()
to enable it to be used by callers who might not have an nfs_client.

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

 fs/nfs/nfs4xdr.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 2fb129d..16ee1cf 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1622,13 +1622,13 @@ static void encode_rename(struct xdr_stream *xdr, const struct qstr *oldname, co
 	hdr->replen += decode_rename_maxsz;
 }
 
-static void encode_renew(struct xdr_stream *xdr, const struct nfs_client *client_stateid, struct compound_hdr *hdr)
+static void encode_renew(struct xdr_stream *xdr, clientid4 clid, struct compound_hdr *hdr)
 {
 	__be32 *p;
 
 	p = reserve_space(xdr, 12);
 	*p++ = cpu_to_be32(OP_RENEW);
-	xdr_encode_hyper(p, client_stateid->cl_clientid);
+	xdr_encode_hyper(p, clid);
 	hdr->nops++;
 	hdr->replen += decode_renew_maxsz;
 }
@@ -2651,7 +2651,7 @@ static void nfs4_xdr_enc_renew(struct rpc_rqst *req, struct xdr_stream *xdr,
 	};
 
 	encode_compound_hdr(xdr, req, &hdr);
-	encode_renew(xdr, clp, &hdr);
+	encode_renew(xdr, clp->cl_clientid, &hdr);
 	encode_nops(&hdr);
 }
 

[PATCH 12/13] NFS: Introduce NFS_ATTR_FATTR_V4_LOCATIONS

[Chuck Lever]

The Linux NFS client must distinguish between referral events (which
it currently supports) and migration events (which it does not yet
support).

In both types of events, an fs_locations array is returned.  But upper
layers, not the XDR layer, should make the distinction between a
referral and a migration.  There really isn't a way for an XDR decoder
function to distinguish the two, in general.

Slightly adjust the FATTR flags returned by decode_fs_locations()
to set NFS_ATTR_FATTR_V4_LOCATIONS only if a non-empty locations
array was returned from the server.  Then have logic in nfs4proc.c
distinguish whether the locations array is for a referral or
something else.

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

 fs/nfs/nfs4proc.c       |    6 +++---
 fs/nfs/nfs4xdr.c        |    2 +-
 include/linux/nfs_xdr.h |    7 ++++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5629858..aa0d2cd 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -75,6 +75,7 @@ static int _nfs4_proc_open(struct nfs4_opendata *data);
 static int _nfs4_recover_proc_open(struct nfs4_opendata *data);
 static int nfs4_do_fsinfo(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
 static int nfs4_async_handle_error(struct rpc_task *, const struct nfs_server *, struct nfs4_state *);
+static void nfs_fixup_referral_attributes(struct nfs_fattr *fattr);
 static int _nfs4_proc_getattr(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fattr *fattr);
 static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
 			    struct nfs_fattr *fattr, struct iattr *sattr,
@@ -2301,7 +2302,6 @@ static int nfs4_proc_get_root(struct nfs_server *server, struct nfs_fh *fhandle,
 	return nfs4_map_errors(status);
 }
 
-static void nfs_fixup_referral_attributes(struct nfs_fattr *fattr);
 /*
  * Get locations and (maybe) other attributes of a referral.
  * Note that we'll actually follow the referral later when
@@ -4707,11 +4707,11 @@ static void nfs_fixup_referral_attributes(struct nfs_fattr *fattr)
 	if (!(((fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID) ||
 	       (fattr->valid & NFS_ATTR_FATTR_FILEID)) &&
 	      (fattr->valid & NFS_ATTR_FATTR_FSID) &&
-	      (fattr->valid & NFS_ATTR_FATTR_V4_REFERRAL)))
+	      (fattr->valid & NFS_ATTR_FATTR_V4_LOCATIONS)))
 		return;
 
 	fattr->valid |= NFS_ATTR_FATTR_TYPE | NFS_ATTR_FATTR_MODE |
-		NFS_ATTR_FATTR_NLINK;
+		NFS_ATTR_FATTR_NLINK | NFS_ATTR_FATTR_V4_REFERRAL;
 	fattr->mode = S_IFDIR | S_IRUGO | S_IXUGO;
 	fattr->nlink = 2;
 }
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 16ee1cf..6b1052a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -3614,7 +3614,7 @@ static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, st
 			res->nlocations++;
 	}
 	if (res->nlocations != 0)
-		status = NFS_ATTR_FATTR_V4_REFERRAL;
+		status = NFS_ATTR_FATTR_V4_LOCATIONS;
 out:
 	dprintk("%s: fs_locations done, error = %d\n", __func__, status);
 	return status;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index d409f1c..d609631 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -89,9 +89,10 @@ struct nfs_fattr {
 #define NFS_ATTR_FATTR_PRECTIME		(1U << 16)
 #define NFS_ATTR_FATTR_CHANGE		(1U << 17)
 #define NFS_ATTR_FATTR_PRECHANGE	(1U << 18)
-#define NFS_ATTR_FATTR_V4_REFERRAL	(1U << 19)	/* NFSv4 referral */
-#define NFS_ATTR_FATTR_MOUNTPOINT	(1U << 20)	/* Treat as mountpoint */
-#define NFS_ATTR_FATTR_MOUNTED_ON_FILEID		(1U << 21)
+#define NFS_ATTR_FATTR_V4_LOCATIONS	(1U << 19)
+#define NFS_ATTR_FATTR_V4_REFERRAL	(1U << 20)	/* NFSv4 referral */
+#define NFS_ATTR_FATTR_MOUNTPOINT	(1U << 21) /* Treat as mountpoint */
+#define NFS_ATTR_FATTR_MOUNTED_ON_FILEID (1U << 22)
 
 #define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
 		| NFS_ATTR_FATTR_MODE \

[PATCH 13/13] NFS: Request fh_expire_type attribute in "server caps" operation

[Chuck Lever]

The fh_expire_type file attribute is a filesystem wide attribute that
consists of flags that indicate what characteristics file handles
on this FSID have.

Our client won't support volatile file handles, but it should find
out at mount time whether the server is going to play shenanighans
with file handles during a migration.

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

 fs/nfs/nfs4proc.c         |    1 +
 fs/nfs/nfs4xdr.c          |   26 ++++++++++++++++++++++++++
 include/linux/nfs_fs_sb.h |    3 +++
 include/linux/nfs_xdr.h   |    1 +
 4 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index aa0d2cd..037d4ca 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2181,6 +2181,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 		server->cache_consistency_bitmask[0] &= FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE;
 		server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY;
 		server->acl_bitmask = res.acl_bitmask;
+		server->fh_expire_type = res.fh_expire_type;
 	}
 
 	return status;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 6b1052a..25e82a6 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -2634,6 +2634,7 @@ static void nfs4_xdr_enc_server_caps(struct rpc_rqst *req,
 	encode_sequence(xdr, &args->seq_args, &hdr);
 	encode_putfh(xdr, args->fhandle, &hdr);
 	encode_getattr_one(xdr, FATTR4_WORD0_SUPPORTED_ATTRS|
+			   FATTR4_WORD0_FH_EXPIRE_TYPE|
 			   FATTR4_WORD0_LINK_SUPPORT|
 			   FATTR4_WORD0_SYMLINK_SUPPORT|
 			   FATTR4_WORD0_ACLSUPPORT, &hdr);
@@ -3181,6 +3182,28 @@ out_overflow:
 	return -EIO;
 }
 
+static int decode_attr_fh_expire_type(struct xdr_stream *xdr,
+				      uint32_t *bitmap, uint32_t *type)
+{
+	__be32 *p;
+
+	*type = 0;
+	if (unlikely(bitmap[0] & (FATTR4_WORD0_FH_EXPIRE_TYPE - 1U)))
+		return -EIO;
+	if (likely(bitmap[0] & FATTR4_WORD0_FH_EXPIRE_TYPE)) {
+		p = xdr_inline_decode(xdr, 4);
+		if (unlikely(!p))
+			goto out_overflow;
+		*type = be32_to_cpup(p);
+		bitmap[0] &= ~FATTR4_WORD0_FH_EXPIRE_TYPE;
+	}
+	dprintk("%s: expire type=0x%x\n", __func__, *type);
+	return 0;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
 static int decode_attr_change(struct xdr_stream *xdr, uint32_t *bitmap, uint64_t *change)
 {
 	__be32 *p;
@@ -4209,6 +4232,9 @@ static int decode_server_caps(struct xdr_stream *xdr, struct nfs4_server_caps_re
 		goto xdr_error;
 	if ((status = decode_attr_supported(xdr, bitmap, res->attr_bitmask)) != 0)
 		goto xdr_error;
+	if ((status = decode_attr_fh_expire_type(xdr, bitmap,
+						 &res->fh_expire_type)) != 0)
+		goto xdr_error;
 	if ((status = decode_attr_link_support(xdr, bitmap, &res->has_links)) != 0)
 		goto xdr_error;
 	if ((status = decode_attr_symlink_support(xdr, bitmap, &res->has_symlinks)) != 0)
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 6532d7b..f07d966 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -144,6 +144,9 @@ struct nfs_server {
 	u32			acl_bitmask;	/* V4 bitmask representing the ACEs
 						   that are supported on this
 						   filesystem */
+	u32			fh_expire_type;	/* V4 bitmask representing file
+						   handle volatility type for
+						   this filesystem */
 	struct pnfs_layoutdriver_type  *pnfs_curr_ld; /* Active layout driver */
 	struct rpc_wait_queue	roc_rpcwaitq;
 	void			*pnfs_ld_data;	/* per mount point data */
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index d609631..ff74d94 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -973,6 +973,7 @@ struct nfs4_server_caps_res {
 	u32				acl_bitmask;
 	u32				has_links;
 	u32				has_symlinks;
+	u32				fh_expire_type;
 	struct nfs4_sequence_res	seq_res;
 };
 

Re: [PATCH 03/13] SUNRPC: Use KERN_DEFAULT for debugging printk's

[Malahal Naineni]

Most of the dprintk() callers are using __func__. Not sure if adding
that to the dprintk itself is a good idea. In any case, it will have to
be a separate patchset though.

Regards, Malahal.

Chuck Lever [chuck.lever@oracle.com] wrote:
> Our dprintk() debugging facility doesn't specify any verbosity level
> for it's printk() calls, but it should.
> 
> The default verbosity for printk's is KERN_DEFAULT.  You might argue
> that these are debugging printk's and thus the verbosity should be
> KERN_DEBUG.  That would mean that to see NFS and SUNRPC debugging
> output an admin would also have to boost the syslog verbosity, which
> would be insufferably noisy.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  include/linux/sunrpc/debug.h |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index c2786f2..e0927c2 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -51,7 +51,11 @@ extern unsigned int		nlm_debug;
>  #undef ifdebug
>  #ifdef RPC_DEBUG			
>  # define ifdebug(fac)		if (unlikely(rpc_debug & RPCDBG_##fac))
> -# define dfprintk(fac, args...)	do { ifdebug(fac) printk(args); } while(0)
> +# define dfprintk(fac, args...)	\
> +	do { \
> +		ifdebug(fac) \
> +			printk(KERN_DEFAULT args); \
> +	} while (0)
>  # define RPC_IFDEBUG(x)		x
>  #else
>  # define ifdebug(fac)		if (0)
> 
> --
> 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
> 

Re: [PATCH 08/13] SUNRPC: Add API to acquire source address

[Malahal Naineni]

Chuck Lever [chuck.lever@oracle.com] wrote:
> +	rcu_read_lock();
> +	xprt = rcu_dereference(clnt->cl_xprt);
> +	salen = xprt->addrlen;
> +	memcpy(sap, &xprt->addr, salen);
> +	net = get_net(xprt->xprt_net);
> +	rcu_read_unlock();

Did I miss a patch that made cl_xprt go away unless accessed under RCU?

Thanks, Malahal.

Re: [PATCH 08/13] SUNRPC: Add API to acquire source address

[Chuck Lever]

On Feb 15, 2012, at 5:12 PM, Malahal Naineni wrote:

> Chuck Lever [chuck.lever@oracle.com] wrote:
>> +	rcu_read_lock();
>> +	xprt = rcu_dereference(clnt->cl_xprt);
>> +	salen = xprt->addrlen;
>> +	memcpy(sap, &xprt->addr, salen);
>> +	net = get_net(xprt->xprt_net);
>> +	rcu_read_unlock();
> 
> Did I miss a patch that made cl_xprt go away unless accessed under RCU?

Malahal, I re-ordered these so the series includes only clean ups.  The RCU patch is now later.

Trond, would you like me to include the cl_xprt RCU patches in this "for 3.4" series too?  If not, I can rework this hunk.

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

Re: [PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer

[Myklebust, Trond]

T24gV2VkLCAyMDEyLTAyLTE1IGF0IDE2OjM0IC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
RWxpbWluYXRlIGEgbnVtYmVyIG9mIGltcGxpY2l0IHR5cGUgY2FzdHMgaW4gY29tcGFyaXNvbnMs
IGFuZCB0aGVzZQ0KPiBjb21waWxlciB3YXJuaW5nczoNCj4gDQo+IGZzL25mcy9kaXIuYzogSW4g
ZnVuY3Rpb24g4oCYbmZzX3JlYWRkaXJfY2xlYXJfYXJyYXnigJk6DQo+IGZzL25mcy9kaXIuYzoy
NjQ6MTY6IHdhcm5pbmc6IGNvbXBhcmlzb24gYmV0d2VlbiBzaWduZWQgYW5kIHVuc2lnbmVkDQo+
IAkJaW50ZWdlciBleHByZXNzaW9ucyBbLVdzaWduLWNvbXBhcmVdDQo+IGZzL25mcy9kaXIuYzog
SW4gZnVuY3Rpb24g4oCYbmZzX3JlYWRkaXJfc2VhcmNoX2Zvcl9jb29raWXigJk6DQo+IGZzL25m
cy9kaXIuYzozNTI6MTY6IHdhcm5pbmc6IGNvbXBhcmlzb24gYmV0d2VlbiBzaWduZWQgYW5kIHVu
c2lnbmVkDQo+IAkJaW50ZWdlciBleHByZXNzaW9ucyBbLVdzaWduLWNvbXBhcmVdDQo+IGZzL25m
cy9kaXIuYzogSW4gZnVuY3Rpb24g4oCYbmZzX2RvX2ZpbGxkaXLigJk6DQo+IGZzL25mcy9kaXIu
Yzo3Njk6Mzg6IHdhcm5pbmc6IGNvbXBhcmlzb24gYmV0d2VlbiBzaWduZWQgYW5kIHVuc2lnbmVk
DQo+IAkJaW50ZWdlciBleHByZXNzaW9ucyBbLVdzaWduLWNvbXBhcmVdDQo+IGZzL25mcy9kaXIu
Yzo3ODA6OTogd2FybmluZzogY29tcGFyaXNvbiBiZXR3ZWVuIHNpZ25lZCBhbmQgdW5zaWduZWQN
Cj4gCQlpbnRlZ2VyIGV4cHJlc3Npb25zIFstV3NpZ24tY29tcGFyZV0NCj4gDQo+IFNpZ25lZC1v
ZmYtYnk6IENodWNrIExldmVyIDxjaHVjay5sZXZlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gDQo+
ICBmcy9uZnMvZGlyLmMgfCAgICAyICstDQo+ICAxIGZpbGVzIGNoYW5nZWQsIDEgaW5zZXJ0aW9u
cygrKSwgMSBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvZGlyLmMgYi9m
cy9uZnMvZGlyLmMNCj4gaW5kZXggYWMyODk5MC4uODY2MWUxMyAxMDA2NDQNCj4gLS0tIGEvZnMv
bmZzL2Rpci5jDQo+ICsrKyBiL2ZzL25mcy9kaXIuYw0KPiBAQCAtMjA3LDcgKzIwNyw3IEBAIHN0
cnVjdCBuZnNfY2FjaGVfYXJyYXlfZW50cnkgew0KPiAgfTsNCj4gIA0KPiAgc3RydWN0IG5mc19j
YWNoZV9hcnJheSB7DQo+IC0JdW5zaWduZWQgaW50IHNpemU7DQo+ICsJaW50IHNpemU7DQo+ICAJ
aW50IGVvZl9pbmRleDsNCj4gIAl1NjQgbGFzdF9jb29raWU7DQo+ICAJc3RydWN0IG5mc19jYWNo
ZV9hcnJheV9lbnRyeSBhcnJheVswXTsNCj4gDQoNCk5vcGUuIFRoYXQncyBhIGNvcC1vdXQ6IGFy
cmF5IHNpemVzIGNhbm5vdCBiZSBuZWdhdGl2ZS4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxp
bnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRh
cHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=

Re: [PATCH 02/13] NFS: Fix comparison signage warnings with slot ID computations

[Myklebust, Trond]

T24gV2VkLCAyMDEyLTAyLTE1IGF0IDE2OjM1IC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
VG8gbWFrZSBuZnM0cHJvYy5jIGNvbXBpbGUgY2xlYW5seSwgYWRkcmVzcyB0aGUgZm9sbG93aW5n
IGNvbXBpbGVyDQo+IHdhcm5pbmdzOg0KPiANCj4gZnMvbmZzL25mczRwcm9jLmM6IEluIGZ1bmN0
aW9uIOKAmG5mczRfZnJlZV9zbG904oCZOg0KPiBmcy9uZnMvbmZzNHByb2MuYzozNzE6Mjogd2Fy
bmluZzogY29tcGFyaXNvbiBiZXR3ZWVuIHNpZ25lZCBhbmQNCj4gCQl1bnNpZ25lZCBpbnRlZ2Vy
IGV4cHJlc3Npb25zIFstV3NpZ24tY29tcGFyZV0NCj4gZnMvbmZzL25mczRwcm9jLmM6IEluIGZ1
bmN0aW9uIOKAmG5mczRfcmVzZXRfc2xvdF90YWJsZeKAmToNCj4gZnMvbmZzL25mczRwcm9jLmM6
NTAwMDoxNTogd2FybmluZzogY29tcGFyaXNvbiBiZXR3ZWVuIHNpZ25lZCBhbmQNCj4gCQl1bnNp
Z25lZCBpbnRlZ2VyIGV4cHJlc3Npb25zIFstV3NpZ24tY29tcGFyZV0NCj4gZnMvbmZzL25mczRw
cm9jLmM6IEluIGZ1bmN0aW9uIOKAmG5mczRfaW5pdF9zbG90X3RhYmxl4oCZOg0KPiBmcy9uZnMv
bmZzNHByb2MuYzo1MDY0OjI6IHdhcm5pbmc6IGNvbXBhcmlzb24gYmV0d2VlbiBzaWduZWQgYW5k
DQo+IAkJdW5zaWduZWQgaW50ZWdlciBleHByZXNzaW9ucyBbLVdzaWduLWNvbXBhcmVdDQo+IA0K
PiBXaHkgaXMgbWF4X3JlcXMgYWx3YXlzIHUzMiwgYnV0IG5mczRfc2xvdF90YWJsZS5tYXhfc2xv
dHMgYSBzaWduZWQNCj4gaW50ZWdlcj8NCj4gDQoNClRoZXJlIGlzIGFscmVhZHkgYSBmaXggZm9y
IHRoaXMgaW4gdGhlIG5mcy1mb3ItbmV4dCBicmFuY2guDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0
DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RA
bmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

Re: [PATCH 03/13] SUNRPC: Use KERN_DEFAULT for debugging printk's

[Myklebust, Trond]

T24gV2VkLCAyMDEyLTAyLTE1IGF0IDE2OjM1IC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
T3VyIGRwcmludGsoKSBkZWJ1Z2dpbmcgZmFjaWxpdHkgZG9lc24ndCBzcGVjaWZ5IGFueSB2ZXJi
b3NpdHkgbGV2ZWwNCj4gZm9yIGl0J3MgcHJpbnRrKCkgY2FsbHMsIGJ1dCBpdCBzaG91bGQuDQo+
IA0KPiBUaGUgZGVmYXVsdCB2ZXJib3NpdHkgZm9yIHByaW50aydzIGlzIEtFUk5fREVGQVVMVC4g
IFlvdSBtaWdodCBhcmd1ZQ0KPiB0aGF0IHRoZXNlIGFyZSBkZWJ1Z2dpbmcgcHJpbnRrJ3MgYW5k
IHRodXMgdGhlIHZlcmJvc2l0eSBzaG91bGQgYmUNCj4gS0VSTl9ERUJVRy4gIFRoYXQgd291bGQg
bWVhbiB0aGF0IHRvIHNlZSBORlMgYW5kIFNVTlJQQyBkZWJ1Z2dpbmcNCj4gb3V0cHV0IGFuIGFk
bWluIHdvdWxkIGFsc28gaGF2ZSB0byBib29zdCB0aGUgc3lzbG9nIHZlcmJvc2l0eSwgd2hpY2gN
Cj4gd291bGQgYmUgaW5zdWZmZXJhYmx5IG5vaXN5Lg0KPiANCj4gU2lnbmVkLW9mZi1ieTogQ2h1
Y2sgTGV2ZXIgPGNodWNrLmxldmVyQG9yYWNsZS5jb20+DQo+IC0tLQ0KPiANCj4gIGluY2x1ZGUv
bGludXgvc3VucnBjL2RlYnVnLmggfCAgICA2ICsrKysrLQ0KPiAgMSBmaWxlcyBjaGFuZ2VkLCA1
IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb25zKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvaW5jbHVk
ZS9saW51eC9zdW5ycGMvZGVidWcuaCBiL2luY2x1ZGUvbGludXgvc3VucnBjL2RlYnVnLmgNCj4g
aW5kZXggYzI3ODZmMi4uZTA5MjdjMiAxMDA2NDQNCj4gLS0tIGEvaW5jbHVkZS9saW51eC9zdW5y
cGMvZGVidWcuaA0KPiArKysgYi9pbmNsdWRlL2xpbnV4L3N1bnJwYy9kZWJ1Zy5oDQo+IEBAIC01
MSw3ICs1MSwxMSBAQCBleHRlcm4gdW5zaWduZWQgaW50CQlubG1fZGVidWc7DQo+ICAjdW5kZWYg
aWZkZWJ1Zw0KPiAgI2lmZGVmIFJQQ19ERUJVRwkJCQ0KPiAgIyBkZWZpbmUgaWZkZWJ1ZyhmYWMp
CQlpZiAodW5saWtlbHkocnBjX2RlYnVnICYgUlBDREJHXyMjZmFjKSkNCj4gLSMgZGVmaW5lIGRm
cHJpbnRrKGZhYywgYXJncy4uLikJZG8geyBpZmRlYnVnKGZhYykgcHJpbnRrKGFyZ3MpOyB9IHdo
aWxlKDApDQo+ICsjIGRlZmluZSBkZnByaW50ayhmYWMsIGFyZ3MuLi4pCVwNCj4gKwlkbyB7IFwN
Cj4gKwkJaWZkZWJ1ZyhmYWMpIFwNCj4gKwkJCXByaW50ayhLRVJOX0RFRkFVTFQgYXJncyk7IFwN
Cj4gKwl9IHdoaWxlICgwKQ0KPiAgIyBkZWZpbmUgUlBDX0lGREVCVUcoeCkJCXgNCj4gICNlbHNl
DQo+ICAjIGRlZmluZSBpZmRlYnVnKGZhYykJCWlmICgwKQ0KPiANCg0KTG9va3MgZ29vZC4gQXBw
bGllZC4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5l
cg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K
DQo=

Re: [PATCH 07/13] NFS: Reduce debugging noise from encode_compound_hdr

[Myklebust, Trond]

T24gV2VkLCAyMDEyLTAyLTE1IGF0IDE2OjM1IC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
V2hlbiBORlNEQkdfWERSIGRlYnVnZ2luZyBpcyBlbmFibGVkLCBJIHNlZSBhIGxvdCBvZg0KPiAN
Cj4gICBlbmNvZGVfY29tcG91bmQ6IHRhZz0NCj4gDQo+IG9uIHRoZSBjb25zb2xlLiAgVGhpcyBp
cyBub2lzZSBpZiB0aGUgdGFnIGlzIGVtcHR5LiAgU29tZSBtaWdodCBhcmd1ZQ0KPiB0aGF0IGl0
IGlzIG5vaXNlIGV2ZW4gaWYgdGhlIHRhZyBpc24ndCBlbXB0eS4uLg0KPiANCj4gU2lnbmVkLW9m
Zi1ieTogQ2h1Y2sgTGV2ZXIgPGNodWNrLmxldmVyQG9yYWNsZS5jb20+DQo+IC0tLQ0KPiANCj4g
IGZzL25mcy9uZnM0eGRyLmMgfCAgICA0ICsrKy0NCj4gIDEgZmlsZXMgY2hhbmdlZCwgMyBpbnNl
cnRpb25zKCspLCAxIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0
eGRyLmMgYi9mcy9uZnMvbmZzNHhkci5jDQo+IGluZGV4IDYwNjg1MmEuLjJmYjEyOWQgMTAwNjQ0
DQo+IC0tLSBhL2ZzL25mcy9uZnM0eGRyLmMNCj4gKysrIGIvZnMvbmZzL25mczR4ZHIuYw0KPiBA
QCAtODg5LDcgKzg4OSw5IEBAIHN0YXRpYyB2b2lkIGVuY29kZV9jb21wb3VuZF9oZHIoc3RydWN0
IHhkcl9zdHJlYW0gKnhkciwNCj4gIAkgKiBidXQgdGhpcyBpcyBub3QgcmVxdWlyZWQgYXMgYSBN
VVNUIGZvciB0aGUgc2VydmVyIHRvIGRvIHNvLiAqLw0KPiAgCWhkci0+cmVwbGVuID0gUlBDX1JF
UEhEUlNJWkUgKyBhdXRoLT5hdV9yc2xhY2sgKyAzICsgaGRyLT50YWdsZW47DQo+ICANCj4gLQlk
cHJpbnRrKCJlbmNvZGVfY29tcG91bmQ6IHRhZz0lLipzXG4iLCAoaW50KWhkci0+dGFnbGVuLCBo
ZHItPnRhZyk7DQo+ICsJaWYgKHVubGlrZWx5KGhkci0+dGFnbGVuICE9IDApKQ0KPiArCQlkcHJp
bnRrKCIlczogdGFnPSUuKnNcbiIsIF9fZnVuY19fLA0KPiArCQkJCShpbnQpaGRyLT50YWdsZW4s
IGhkci0+dGFnKTsNCj4gIAlCVUdfT04oaGRyLT50YWdsZW4gPiBORlM0X01BWFRBR0xFTik7DQo+
ICAJcCA9IHJlc2VydmVfc3BhY2UoeGRyLCA0ICsgaGRyLT50YWdsZW4gKyA4KTsNCj4gIAlwID0g
eGRyX2VuY29kZV9vcGFxdWUocCwgaGRyLT50YWcsIGhkci0+dGFnbGVuKTsNCj4gDQoNCkxldCdz
IGp1c3QgY29tbWVudCBvdXQgdGhlIHRhZyBkcHJpbnRrIGZvciBub3cuIFdlIGRvbid0IHVzZSB0
aGUgdGFnDQpmaWVsZCBhdCBhbGwuLi4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5G
UyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29t
DQp3d3cubmV0YXBwLmNvbQ0KDQo=

Re: [PATCH 08/13] SUNRPC: Add API to acquire source address

[Myklebust, Trond]

T24gV2VkLCAyMDEyLTAyLTE1IGF0IDE2OjM1IC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
TkZTdjQuMCBjbGllbnRzIG11c3Qgc2VuZCBlbmRwb2ludCBpbmZvcm1hdGlvbiBmb3IgdGhlaXIg
Y2FsbGJhY2sNCj4gc2VydmljZSB0byBORlN2NC4wIHNlcnZlcnMgZHVyaW5nIHRoZWlyIGZpcnN0
IGNvbnRhY3Qgd2l0aCBhIHNlcnZlci4NCj4gVHJhZGl0aW9uYWxseSBvbiBMaW51eCwgdXNlciBz
cGFjZSBwcm92aWRlcyB0aGUgY2FsbGJhY2sgZW5kcG9pbnQgSVANCj4gYWRkcmVzcyB2aWEgdGhl
ICJjbGllbnRhZGRyPSIgbW91bnQgb3B0aW9uLg0KPiANCj4gRHVyaW5nIGFuIE5GU3Y0IG1pZ3Jh
dGlvbiBldmVudCwgaXQgaXMgcG9zc2libGUgdGhhdCBhbiBGU0lEIG1heSBiZQ0KPiBtaWdyYXRl
ZCB0byBhIGRlc3RpbmF0aW9uIHNlcnZlciB0aGF0IGlzIGFjY2Vzc2libGUgdmlhIGEgZGlmZmVy
ZW50DQo+IHNvdXJjZSBJUCBhZGRyZXNzIHRoYW4gdGhlIHNvdXJjZSBzZXJ2ZXIgd2FzLiAgVGhl
IGNsaWVudCBtdXN0IHVwZGF0ZQ0KPiBjYWxsYmFjayBlbmRwb2ludCBpbmZvcm1hdGlvbiBvbiB0
aGUgZGVzdGluYXRpb24gc2VydmVyIHNvIHRoYXQgaXQgY2FuDQo+IG1haW50YWluIGxlYXNlcyBh
bmQgYWxsb3cgZGVsZWdhdGlvbi4NCj4gDQo+IFdpdGhvdXQgYSBuZXcgImNsaWVudGFkZHI9IiBv
cHRpb24gZnJvbSB1c2VyIHNwYWNlLCBob3dldmVyLCB0aGUNCj4ga2VybmVsIGl0c2VsZiBtdXN0
IGNvbnN0cnVjdCBhbiBhcHByb3ByaWF0ZSBJUCBhZGRyZXNzIGZvciB0aGUNCj4gY2FsbGJhY2sg
dXBkYXRlLiAgUHJvdmlkZSBhbiBBUEkgaW4gdGhlIFJQQyBjbGllbnQgZm9yIHVwcGVyIGxheWVy
DQo+IFJQQyBjb25zdW1lcnMgdG8gYWNxdWlyZSBhIHNvdXJjZSBhZGRyZXNzIGZvciBhIHJlbW90
ZS4NCj4gDQo+IFRoZSBtZWNoYW5pc20gdXNlZCBieSB0aGUgbW91bnQubmZzIGNvbW1hbmQgaXMg
Y29waWVkOiBzZXQgdXAgYQ0KPiBjb25uZWN0ZWQgVURQIHNvY2tldCB0byB0aGUgZGVzaWduYXRl
ZCByZW1vdGUsIHRoZW4gc2NyYXBlIHRoZSBzb3VyY2UNCj4gYWRkcmVzcyBvZmYgdGhlIHNvY2tl
dC4gIFdlIGFyZSBjYXJlZnVsIHRvIHNlbGVjdCB0aGUgY29ycmVjdCBuZXR3b3JrDQo+IG5hbWVz
cGFjZSB3aGVuIHNldHRpbmcgdXAgdGhlIHRlbXBvcmFyeSBVRFAgc29ja2V0Lg0KPiANCj4gU2ln
bmVkLW9mZi1ieTogQ2h1Y2sgTGV2ZXIgPGNodWNrLmxldmVyQG9yYWNsZS5jb20+DQo+IC0tLQ0K
DQpUaGlzIGRvZXNuJ3QgYXBwZWFyIHRvIGFwcGx5IChub3IgZG8gc2V2ZXJhbCBvZiB0aGUgZGVi
dWdnaW5nIHBhdGNoZXMpLg0KV2FzIGl0IHdyaXR0ZW4gYWdhaW5zdCB0aGUgbmZzLWZvci1uZXh0
IGJyYW5jaD8NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRh
aW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNv
bQ0KDQo=

Re: [PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer

[Chuck Lever]

On Feb 16, 2012, at 2:52 PM, Myklebust, Trond wrote:

> On Wed, 2012-02-15 at 16:34 -0500, Chuck Lever wrote:
>> Eliminate a number of implicit type casts in comparisons, and these
>> compiler warnings:
>> 
>> fs/nfs/dir.c: In function ‘nfs_readdir_clear_array’:
>> fs/nfs/dir.c:264:16: warning: comparison between signed and unsigned
>> 		integer expressions [-Wsign-compare]
>> fs/nfs/dir.c: In function ‘nfs_readdir_search_for_cookie’:
>> fs/nfs/dir.c:352:16: warning: comparison between signed and unsigned
>> 		integer expressions [-Wsign-compare]
>> fs/nfs/dir.c: In function ‘nfs_do_filldir’:
>> fs/nfs/dir.c:769:38: warning: comparison between signed and unsigned
>> 		integer expressions [-Wsign-compare]
>> fs/nfs/dir.c:780:9: warning: comparison between signed and unsigned
>> 		integer expressions [-Wsign-compare]
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 
>> fs/nfs/dir.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> index ac28990..8661e13 100644
>> --- a/fs/nfs/dir.c
>> +++ b/fs/nfs/dir.c
>> @@ -207,7 +207,7 @@ struct nfs_cache_array_entry {
>> };
>> 
>> struct nfs_cache_array {
>> -	unsigned int size;
>> +	int size;
>> 	int eof_index;
>> 	u64 last_cookie;
>> 	struct nfs_cache_array_entry array[0];
>> 
> 
> Nope. That's a cop-out: array sizes cannot be negative.

That's typically true, but IIRC the array size here can hold a "-1" as a special value.

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

Re: [PATCH 02/13] NFS: Fix comparison signage warnings with slot ID computations

[Chuck Lever]

On Feb 16, 2012, at 2:52 PM, Myklebust, Trond wrote:

> On Wed, 2012-02-15 at 16:35 -0500, Chuck Lever wrote:
>> To make nfs4proc.c compile cleanly, address the following compiler
>> warnings:
>> 
>> fs/nfs/nfs4proc.c: In function ‘nfs4_free_slot’:
>> fs/nfs/nfs4proc.c:371:2: warning: comparison between signed and
>> 		unsigned integer expressions [-Wsign-compare]
>> fs/nfs/nfs4proc.c: In function ‘nfs4_reset_slot_table’:
>> fs/nfs/nfs4proc.c:5000:15: warning: comparison between signed and
>> 		unsigned integer expressions [-Wsign-compare]
>> fs/nfs/nfs4proc.c: In function ‘nfs4_init_slot_table’:
>> fs/nfs/nfs4proc.c:5064:2: warning: comparison between signed and
>> 		unsigned integer expressions [-Wsign-compare]
>> 
>> Why is max_reqs always u32, but nfs4_slot_table.max_slots a signed
>> integer?
>> 
> 
> There is already a fix for this in the nfs-for-next branch.

I'll drop mine.

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

Re: [PATCH 07/13] NFS: Reduce debugging noise from encode_compound_hdr

[Chuck Lever]

On Feb 16, 2012, at 3:04 PM, Myklebust, Trond wrote:

> On Wed, 2012-02-15 at 16:35 -0500, Chuck Lever wrote:
>> When NFSDBG_XDR debugging is enabled, I see a lot of
>> 
>>  encode_compound: tag=
>> 
>> on the console.  This is noise if the tag is empty.  Some might argue
>> that it is noise even if the tag isn't empty...
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 
>> fs/nfs/nfs4xdr.c |    4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index 606852a..2fb129d 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -889,7 +889,9 @@ static void encode_compound_hdr(struct xdr_stream *xdr,
>> 	 * but this is not required as a MUST for the server to do so. */
>> 	hdr->replen = RPC_REPHDRSIZE + auth->au_rslack + 3 + hdr->taglen;
>> 
>> -	dprintk("encode_compound: tag=%.*s\n", (int)hdr->taglen, hdr->tag);
>> +	if (unlikely(hdr->taglen != 0))
>> +		dprintk("%s: tag=%.*s\n", __func__,
>> +				(int)hdr->taglen, hdr->tag);
>> 	BUG_ON(hdr->taglen > NFS4_MAXTAGLEN);
>> 	p = reserve_space(xdr, 4 + hdr->taglen + 8);
>> 	p = xdr_encode_opaque(p, hdr->tag, hdr->taglen);
>> 
> 
> Let's just comment out the tag dprintk for now. We don't use the tag
> field at all...

OK.

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

Re: [PATCH 08/13] SUNRPC: Add API to acquire source address

[Chuck Lever]

On Feb 16, 2012, at 3:07 PM, Myklebust, Trond wrote:

> On Wed, 2012-02-15 at 16:35 -0500, Chuck Lever wrote:
>> NFSv4.0 clients must send endpoint information for their callback
>> service to NFSv4.0 servers during their first contact with a server.
>> Traditionally on Linux, user space provides the callback endpoint IP
>> address via the "clientaddr=" mount option.
>> 
>> During an NFSv4 migration event, it is possible that an FSID may be
>> migrated to a destination server that is accessible via a different
>> source IP address than the source server was.  The client must update
>> callback endpoint information on the destination server so that it can
>> maintain leases and allow delegation.
>> 
>> Without a new "clientaddr=" option from user space, however, the
>> kernel itself must construct an appropriate IP address for the
>> callback update.  Provide an API in the RPC client for upper layer
>> RPC consumers to acquire a source address for a remote.
>> 
>> The mechanism used by the mount.nfs command is copied: set up a
>> connected UDP socket to the designated remote, then scrape the source
>> address off the socket.  We are careful to select the correct network
>> namespace when setting up the temporary UDP socket.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
> 
> This doesn't appear to apply (nor do several of the debugging patches).
> Was it written against the nfs-for-next branch?

No, against 3.2.  These were intended for review only, since I wasn't sure which branch to port them to.  Now I know.

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

Re: [PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDE1OjA5IC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
T24gRmViIDE2LCAyMDEyLCBhdCAyOjUyIFBNLCBNeWtsZWJ1c3QsIFRyb25kIHdyb3RlOg0KPiAN
Cj4gPiBPbiBXZWQsIDIwMTItMDItMTUgYXQgMTY6MzQgLTA1MDAsIENodWNrIExldmVyIHdyb3Rl
Og0KPiA+PiBFbGltaW5hdGUgYSBudW1iZXIgb2YgaW1wbGljaXQgdHlwZSBjYXN0cyBpbiBjb21w
YXJpc29ucywgYW5kIHRoZXNlDQo+ID4+IGNvbXBpbGVyIHdhcm5pbmdzOg0KPiA+PiANCj4gPj4g
ZnMvbmZzL2Rpci5jOiBJbiBmdW5jdGlvbiDigJhuZnNfcmVhZGRpcl9jbGVhcl9hcnJheeKAmToN
Cj4gPj4gZnMvbmZzL2Rpci5jOjI2NDoxNjogd2FybmluZzogY29tcGFyaXNvbiBiZXR3ZWVuIHNp
Z25lZCBhbmQgdW5zaWduZWQNCj4gPj4gCQlpbnRlZ2VyIGV4cHJlc3Npb25zIFstV3NpZ24tY29t
cGFyZV0NCj4gPj4gZnMvbmZzL2Rpci5jOiBJbiBmdW5jdGlvbiDigJhuZnNfcmVhZGRpcl9zZWFy
Y2hfZm9yX2Nvb2tpZeKAmToNCj4gPj4gZnMvbmZzL2Rpci5jOjM1MjoxNjogd2FybmluZzogY29t
cGFyaXNvbiBiZXR3ZWVuIHNpZ25lZCBhbmQgdW5zaWduZWQNCj4gPj4gCQlpbnRlZ2VyIGV4cHJl
c3Npb25zIFstV3NpZ24tY29tcGFyZV0NCj4gPj4gZnMvbmZzL2Rpci5jOiBJbiBmdW5jdGlvbiDi
gJhuZnNfZG9fZmlsbGRpcuKAmToNCj4gPj4gZnMvbmZzL2Rpci5jOjc2OTozODogd2FybmluZzog
Y29tcGFyaXNvbiBiZXR3ZWVuIHNpZ25lZCBhbmQgdW5zaWduZWQNCj4gPj4gCQlpbnRlZ2VyIGV4
cHJlc3Npb25zIFstV3NpZ24tY29tcGFyZV0NCj4gPj4gZnMvbmZzL2Rpci5jOjc4MDo5OiB3YXJu
aW5nOiBjb21wYXJpc29uIGJldHdlZW4gc2lnbmVkIGFuZCB1bnNpZ25lZA0KPiA+PiAJCWludGVn
ZXIgZXhwcmVzc2lvbnMgWy1Xc2lnbi1jb21wYXJlXQ0KPiA+PiANCj4gPj4gU2lnbmVkLW9mZi1i
eTogQ2h1Y2sgTGV2ZXIgPGNodWNrLmxldmVyQG9yYWNsZS5jb20+DQo+ID4+IC0tLQ0KPiA+PiAN
Cj4gPj4gZnMvbmZzL2Rpci5jIHwgICAgMiArLQ0KPiA+PiAxIGZpbGVzIGNoYW5nZWQsIDEgaW5z
ZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkNCj4gPj4gDQo+ID4+IGRpZmYgLS1naXQgYS9mcy9u
ZnMvZGlyLmMgYi9mcy9uZnMvZGlyLmMNCj4gPj4gaW5kZXggYWMyODk5MC4uODY2MWUxMyAxMDA2
NDQNCj4gPj4gLS0tIGEvZnMvbmZzL2Rpci5jDQo+ID4+ICsrKyBiL2ZzL25mcy9kaXIuYw0KPiA+
PiBAQCAtMjA3LDcgKzIwNyw3IEBAIHN0cnVjdCBuZnNfY2FjaGVfYXJyYXlfZW50cnkgew0KPiA+
PiB9Ow0KPiA+PiANCj4gPj4gc3RydWN0IG5mc19jYWNoZV9hcnJheSB7DQo+ID4+IC0JdW5zaWdu
ZWQgaW50IHNpemU7DQo+ID4+ICsJaW50IHNpemU7DQo+ID4+IAlpbnQgZW9mX2luZGV4Ow0KPiA+
PiAJdTY0IGxhc3RfY29va2llOw0KPiA+PiAJc3RydWN0IG5mc19jYWNoZV9hcnJheV9lbnRyeSBh
cnJheVswXTsNCj4gPj4gDQo+ID4gDQo+ID4gTm9wZS4gVGhhdCdzIGEgY29wLW91dDogYXJyYXkg
c2l6ZXMgY2Fubm90IGJlIG5lZ2F0aXZlLg0KPiANCj4gVGhhdCdzIHR5cGljYWxseSB0cnVlLCBi
dXQgSUlSQyB0aGUgYXJyYXkgc2l6ZSBoZXJlIGNhbiBob2xkIGEgIi0xIiBhcyBhIHNwZWNpYWwg
dmFsdWUuDQoNCg0KV2hlcmUgZG8geW91IHNlZSB0aGF0PyBJJ20gbm90IGZpbmRpbmcgdGhhdCBp
biB0aGUgdXBzdHJlYW0ga2VybmVsLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZT
IGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20N
Cnd3dy5uZXRhcHAuY29tDQoNCg==

Re: [PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer

[Chuck Lever]

On Feb 16, 2012, at 3:34 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-16 at 15:09 -0500, Chuck Lever wrote:
>> On Feb 16, 2012, at 2:52 PM, Myklebust, Trond wrote:
>> 
>>> On Wed, 2012-02-15 at 16:34 -0500, Chuck Lever wrote:
>>>> Eliminate a number of implicit type casts in comparisons, and these
>>>> compiler warnings:
>>>> 
>>>> fs/nfs/dir.c: In function ‘nfs_readdir_clear_array’:
>>>> fs/nfs/dir.c:264:16: warning: comparison between signed and unsigned
>>>> 		integer expressions [-Wsign-compare]
>>>> fs/nfs/dir.c: In function ‘nfs_readdir_search_for_cookie’:
>>>> fs/nfs/dir.c:352:16: warning: comparison between signed and unsigned
>>>> 		integer expressions [-Wsign-compare]
>>>> fs/nfs/dir.c: In function ‘nfs_do_filldir’:
>>>> fs/nfs/dir.c:769:38: warning: comparison between signed and unsigned
>>>> 		integer expressions [-Wsign-compare]
>>>> fs/nfs/dir.c:780:9: warning: comparison between signed and unsigned
>>>> 		integer expressions [-Wsign-compare]
>>>> 
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>> 
>>>> fs/nfs/dir.c |    2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>>>> index ac28990..8661e13 100644
>>>> --- a/fs/nfs/dir.c
>>>> +++ b/fs/nfs/dir.c
>>>> @@ -207,7 +207,7 @@ struct nfs_cache_array_entry {
>>>> };
>>>> 
>>>> struct nfs_cache_array {
>>>> -	unsigned int size;
>>>> +	int size;
>>>> 	int eof_index;
>>>> 	u64 last_cookie;
>>>> 	struct nfs_cache_array_entry array[0];
>>>> 
>>> 
>>> Nope. That's a cop-out: array sizes cannot be negative.
>> 
>> That's typically true, but IIRC the array size here can hold a "-1" as a special value.
> 
> 
> Where do you see that? I'm not finding that in the upstream kernel.

Paging this all back in... "eof_index" can be -1, thus it is an int.  "size" is compared with eof_index, with signed loop iterators, and with "current_index," which is a loff_t (also a signed type).  Therefore "size" should also be signed.  This kind of abuse is quite common in the network code.

The other option is to leave these warnings.  The patch is a compromise at best, I agree, but mixed sign comparisons have practical implications, while keeping "size" unsigned because it represents a natural number does not have practical implications.

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

Re: [PATCH 13/13] NFS: Request fh_expire_type attribute in "server caps" operation

[Matthew Treinish]

On Wed, Feb 15, 2012 at 04:36:34PM -0500, Chuck Lever wrote:
> The fh_expire_type file attribute is a filesystem wide attribute that
> consists of flags that indicate what characteristics file handles
> on this FSID have.
> 
> Our client won't support volatile file handles, but it should find
> out at mount time whether the server is going to play shenanighans
> with file handles during a migration.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfs/nfs4proc.c         |    1 +
>  fs/nfs/nfs4xdr.c          |   26 ++++++++++++++++++++++++++
>  include/linux/nfs_fs_sb.h |    3 +++
>  include/linux/nfs_xdr.h   |    1 +
>  4 files changed, 31 insertions(+), 0 deletions(-)
> 
<snip>
> index 6532d7b..f07d966 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -144,6 +144,9 @@ struct nfs_server {
>  	u32			acl_bitmask;	/* V4 bitmask representing the ACEs
>  						   that are supported on this
>  						   filesystem */
> +	u32			fh_expire_type;	/* V4 bitmask representing file
> +						   handle volatility type for
> +						   this filesystem */
>  	struct pnfs_layoutdriver_type  *pnfs_curr_ld; /* Active layout driver */
>  	struct rpc_wait_queue	roc_rpcwaitq;
>  	void			*pnfs_ld_data;	/* per mount point data */
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index d609631..ff74d94 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -973,6 +973,7 @@ struct nfs4_server_caps_res {
>  	u32				acl_bitmask;
>  	u32				has_links;
>  	u32				has_symlinks;
> +	u32				fh_expire_type;
>  	struct nfs4_sequence_res	seq_res;
>  };

I had a similar patch in my volatile file handle tree, 
(http://www.spinics.net/lists/linux-nfs/msg26069.html) but in my commit I used 
fs_info instead of server caps. If I remember the spec correctly, it says that
the attribute is set per file system, that's why I put it in fs_info. I'm just
curious why you used server caps, not that it really makes a difference.

-Matt Treinish

[PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer

[Chuck Lever]

Eliminate a number of implicit type casts in comparisons, and these
compiler warnings:

fs/nfs/dir.c: In function ‘nfs_readdir_clear_array’:
fs/nfs/dir.c:264:16: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]
fs/nfs/dir.c: In function ‘nfs_readdir_search_for_cookie’:
fs/nfs/dir.c:352:16: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]
fs/nfs/dir.c: In function ‘nfs_do_filldir’:
fs/nfs/dir.c:769:38: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]
fs/nfs/dir.c:780:9: warning: comparison between signed and unsigned
		integer expressions [-Wsign-compare]

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

 fs/nfs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ac28990..8661e13 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -207,7 +207,7 @@ struct nfs_cache_array_entry {
 };
 
 struct nfs_cache_array {
-	unsigned int size;
+	int size;
 	int eof_index;
 	u64 last_cookie;
 	struct nfs_cache_array_entry array[0];