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
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 <[email protected]>
---
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);
}
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 <[email protected]>
---
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;
};
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 <[email protected]>
---
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;
}
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 <[email protected]>
---
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 */
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==
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 <[email protected]>
---
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 = {
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 <[email protected]>
---
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)
{
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 <[email protected]>
---
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 \
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=
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 <[email protected]>
---
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];
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 <[email protected]>
> ---
>
> 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
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 <[email protected]>
---
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);
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 <[email protected]>
---
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
*/
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 <[email protected]>
---
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)
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 <[email protected]>
>> ---
>>
>> 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
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 <[email protected]>
---
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);
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=
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 <[email protected]>
---
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;
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 [[email protected]] 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 <[email protected]>
> ---
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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=
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 <[email protected]>
>>>> ---
>>>>
>>>> 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