[PATCH 00/13] For 3.4

[PATCH 00/13] For 3.4

[Chuck Lever]

Hi-

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 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];

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 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 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