2012-05-18 22:05:18

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 00/14] UCS pre-requisites for 3.5

Hi-

Here is a motley set of patches from the previously posted UCS series
that have been tested to some extent and are ready for mainline.

Unfortunately I was not able to get to the CLID_INUSE recovery
overhaul that we have discussed because of family matters. The
three or four patches that implement UCS and the "migration"
mount option are not included here, as they depend on proper
CLID_INUSE recovery.

---

Chuck Lever (14):
NFS: EXCHANGE_ID should save the server major and minor ID
NFS: Add nfs_client behavior flags
NFS: Refactor nfs_get_client(): initialize nfs_client
NFS: Refactor nfs_get_client(): add nfs_found_client()
NFS: Always use the same SETCLIENTID boot verifier
NFS: Force server to drop NFSv4 state
NFS: Add NFSDBG_STATE
NFS: Don't swap bytes in nfs4_construct_boot_verifier()
NFS: Remove nfs_unique_id
NFS: Clean up return code checking in nfs4_proc_exchange_id()
NFS: Use proper naming conventions for the nfs_client.net field
NFS: Use proper naming conventions for nfs_client.impl_id field
NFS: Use proper naming conventions for NFSv4.1 server scope fields
NFS: Fix comment misspelling in struct nfs_client definition


fs/nfs/blocklayout/blocklayoutdev.c | 2
fs/nfs/client.c | 204 +++++++++++++++++++----------------
fs/nfs/idmap.c | 4 -
fs/nfs/internal.h | 17 +--
fs/nfs/netns.h | 5 +
fs/nfs/nfs4_fs.h | 8 -
fs/nfs/nfs4filelayoutdev.c | 2
fs/nfs/nfs4proc.c | 74 ++++++++-----
fs/nfs/nfs4renewd.c | 2
fs/nfs/nfs4state.c | 15 ++-
fs/nfs/nfs4xdr.c | 18 ++-
fs/nfs/super.c | 4 -
include/linux/nfs_fs.h | 1
include/linux/nfs_fs_sb.h | 17 ++-
include/linux/nfs_xdr.h | 12 +-
15 files changed, 222 insertions(+), 163 deletions(-)

--
Chuck Lever


2012-05-18 22:05:44

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 03/14] NFS: Use proper naming conventions for nfs_client.impl_id field

Clean up: When naming fields and data types, follow established
conventions to facilitate accurate grep/cscope searches.

Additionally, for consistency, move the impl_id field into the NFSv4-
specific part of the nfs_client, and free that memory in the logic
that shuts down NFSv4 nfs_clients.

Introduced by commit 7d2ed9ac "NFSv4: parse and display server
implementation ids," Fri Feb 17, 2012.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 2 +-
fs/nfs/nfs4proc.c | 12 ++++++------
fs/nfs/super.c | 4 ++--
include/linux/nfs_fs_sb.h | 2 +-
4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 1285f70..ec5a276 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -236,6 +236,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)

rpc_destroy_wait_queue(&clp->cl_rpcwaitq);
kfree(clp->cl_serverscope);
+ kfree(clp->cl_implid);
}

/* idr_remove_all is not needed as all id's are removed by nfs_put_client */
@@ -304,7 +305,6 @@ static void nfs_free_client(struct nfs_client *clp)

put_net(clp->net);
kfree(clp->cl_hostname);
- kfree(clp->impl_id);
kfree(clp);

dprintk("<-- nfs_free_client()\n");
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 35585ef..386a756 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5147,8 +5147,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)

if (!status) {
/* use the most recent implementation id */
- kfree(clp->impl_id);
- clp->impl_id = res.impl_id;
+ kfree(clp->cl_implid);
+ clp->cl_implid = res.impl_id;
} else
kfree(res.impl_id);

@@ -5172,12 +5172,12 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
out_server_scope:
kfree(res.server_scope);
out:
- if (clp->impl_id)
+ if (clp->cl_implid)
dprintk("%s: Server Implementation ID: "
"domain: %s, name: %s, date: %llu,%u\n",
- __func__, clp->impl_id->domain, clp->impl_id->name,
- clp->impl_id->date.seconds,
- clp->impl_id->date.nseconds);
+ __func__, clp->cl_implid->domain, clp->cl_implid->name,
+ clp->cl_implid->date.seconds,
+ clp->cl_implid->date.nseconds);
dprintk("<-- %s status= %d\n", __func__, status);
return status;
}
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 4ac7fca..2c5390e 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -786,8 +786,8 @@ static void show_pnfs(struct seq_file *m, struct nfs_server *server)

static void show_implementation_id(struct seq_file *m, struct nfs_server *nfss)
{
- if (nfss->nfs_client && nfss->nfs_client->impl_id) {
- struct nfs41_impl_id *impl_id = nfss->nfs_client->impl_id;
+ if (nfss->nfs_client && nfss->nfs_client->cl_implid) {
+ struct nfs41_impl_id *impl_id = nfss->nfs_client->cl_implid;
seq_printf(m, "\n\timpl_id:\tname='%s',domain='%s',"
"date='%llu,%u'",
impl_id->name, impl_id->domain,
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 900d733..773e021 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -81,13 +81,13 @@ struct nfs_client {
u32 cl_exchange_flags;
struct nfs4_session *cl_session; /* shared session */
struct nfs41_server_scope *cl_serverscope;
+ struct nfs41_impl_id *cl_implid;
#endif /* CONFIG_NFS_V4 */

#ifdef CONFIG_NFS_FSCACHE
struct fscache_cookie *fscache; /* client index cache cookie */
#endif

- struct nfs41_impl_id *impl_id; /* from exchange_id */
struct net *net;
};



2012-05-21 15:51:13

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 07/14] NFS: Don't swap bytes in nfs4_construct_boot_verifier()

On Mon, May 21, 2012 at 11:47:38AM -0400, Chuck Lever wrote:
> Hi-
>
> On May 21, 2012, at 11:40 AM, J. Bruce Fields wrote:
>
> > On Fri, May 18, 2012 at 06:06:16PM -0400, Chuck Lever wrote:
> >> The SETCLIENTID boot verifier is opaque to NFSv4 servers, thus there
> >> is no requirement for byte swapping before the client puts the
> >> verifier on the wire.
> >>
> >> This treatment is similar to other timestamp-based verifiers.
> >
> > The fact that it's opaque means we don't *have* to be consistent about
> > byte-order. But we can still choose to do so.
>
> Agree, and that's consistent with "no requirement for byte swapping".
>
> > It may help debugging a
> > little (by making it just a little easier to decode the on-the-wire
> > verifier).
>
> In this case, I doubt it. We're talking about raw timestamps here, there's nothing we can really recognize in those. If this were a structured piece of data, I'd feel more agreement about debugging ease.
>
> > And changing the encoding of the verifier means it won't
> > work over boots that cross kernel versions.
>
> The boot verifier is *supposed* to change over a reboot, so I think this is not consequential. The server is only looking for a mismatch between verifiers, and this changes preserves that semantic.

Whoops, you're right (ignoring amusing but unlikely cases where
subsequent boot times happen to be byte-swaps of each other).

> > (I'll admit that's *not* a
> > big deal--most clients probably take longer than most servers' lease
> > times anyway--but why do this without a reason?)
>
> It reduces code clutter for the subsequent patch which adds the NFS4CLNT_PURGE_STATE bit.

Huh, I don't see that it makes much difference.

Whatever, I don't care much.

--b.

>
> >
> > --b.
> >
> >>
> >> Signed-off-by: Chuck Lever <[email protected]>
> >> ---
> >>
> >> fs/nfs/nfs4proc.c | 4 ++--
> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index db7b76a..73cfd9e 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -3937,8 +3937,8 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> >> {
> >> __be32 verf[2];
> >>
> >> - verf[0] = htonl((u32)clp->cl_boot_time.tv_sec);
> >> - verf[1] = htonl((u32)clp->cl_boot_time.tv_nsec);
> >> + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> >> + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> >> memcpy(bootverf->data, verf, sizeof(bootverf->data));
> >> }
> >>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
>
> --
> 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

2012-05-18 22:05:52

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 04/14] NFS: Use proper naming conventions for the nfs_client.net field

Clean up: When naming fields and data types, follow established
conventions to facilitate accurate grep/cscope searches.

Introduced by commit e50a7a1a "NFS: make NFS client allocated per
network namespace context," Tue Jan 10, 2012.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/blocklayout/blocklayoutdev.c | 2 +-
fs/nfs/client.c | 22 +++++++++++-----------
fs/nfs/idmap.c | 4 ++--
fs/nfs/nfs4filelayoutdev.c | 2 +-
include/linux/nfs_fs_sb.h | 2 +-
5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayoutdev.c b/fs/nfs/blocklayout/blocklayoutdev.c
index a5c88a5..c965542 100644
--- a/fs/nfs/blocklayout/blocklayoutdev.c
+++ b/fs/nfs/blocklayout/blocklayoutdev.c
@@ -123,7 +123,7 @@ nfs4_blk_decode_device(struct nfs_server *server,
uint8_t *dataptr;
DECLARE_WAITQUEUE(wq, current);
int offset, len, i, rc;
- struct net *net = server->nfs_client->net;
+ struct net *net = server->nfs_client->cl_net;
struct nfs_net *nn = net_generic(net, nfs_net_id);
struct bl_dev_msg *reply = &nn->bl_mount_reply;

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index ec5a276..c430acd 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -65,7 +65,7 @@ static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion)
{
int ret = 0;
- struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
+ struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);

if (clp->rpc_ops->version != 4 || minorversion != 0)
return ret;
@@ -172,7 +172,7 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
clp->cl_rpcclient = ERR_PTR(-EINVAL);

clp->cl_proto = cl_init->proto;
- clp->net = get_net(cl_init->net);
+ clp->cl_net = get_net(cl_init->net);

#ifdef CONFIG_NFS_V4
err = nfs_get_cb_ident_idr(clp, cl_init->minorversion);
@@ -250,7 +250,7 @@ void nfs_cleanup_cb_ident_idr(struct net *net)
/* nfs_client_lock held */
static void nfs_cb_idr_remove_locked(struct nfs_client *clp)
{
- struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
+ struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);

if (clp->cl_cb_ident)
idr_remove(&nn->cb_ident_idr, clp->cl_cb_ident);
@@ -303,7 +303,7 @@ static void nfs_free_client(struct nfs_client *clp)
if (clp->cl_machine_cred != NULL)
put_rpccred(clp->cl_machine_cred);

- put_net(clp->net);
+ put_net(clp->cl_net);
kfree(clp->cl_hostname);
kfree(clp);

@@ -321,7 +321,7 @@ void nfs_put_client(struct nfs_client *clp)
return;

dprintk("--> nfs_put_client({%d})\n", atomic_read(&clp->cl_count));
- nn = net_generic(clp->net, nfs_net_id);
+ nn = net_generic(clp->cl_net, nfs_net_id);

if (atomic_dec_and_lock(&clp->cl_count, &nn->nfs_client_lock)) {
list_del(&clp->cl_share_link);
@@ -659,7 +659,7 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
{
struct rpc_clnt *clnt = NULL;
struct rpc_create_args args = {
- .net = clp->net,
+ .net = clp->cl_net,
.protocol = clp->cl_proto,
.address = (struct sockaddr *)&clp->cl_addr,
.addrsize = clp->cl_addrlen,
@@ -713,7 +713,7 @@ static int nfs_start_lockd(struct nfs_server *server)
.nfs_version = clp->rpc_ops->version,
.noresvport = server->flags & NFS_MOUNT_NORESVPORT ?
1 : 0,
- .net = clp->net,
+ .net = clp->cl_net,
};

if (nlm_init.nfs_version > 3)
@@ -1048,7 +1048,7 @@ static void nfs_server_copy_userdata(struct nfs_server *target, struct nfs_serve
static void nfs_server_insert_lists(struct nfs_server *server)
{
struct nfs_client *clp = server->nfs_client;
- struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
+ struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);

spin_lock(&nn->nfs_client_lock);
list_add_tail_rcu(&server->client_link, &clp->cl_superblocks);
@@ -1065,7 +1065,7 @@ static void nfs_server_remove_lists(struct nfs_server *server)

if (clp == NULL)
return;
- nn = net_generic(clp->net, nfs_net_id);
+ nn = net_generic(clp->cl_net, nfs_net_id);
spin_lock(&nn->nfs_client_lock);
list_del_rcu(&server->client_link);
if (list_empty(&clp->cl_superblocks))
@@ -1474,7 +1474,7 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
.rpc_ops = &nfs_v4_clientops,
.proto = ds_proto,
.minorversion = mds_clp->cl_minorversion,
- .net = mds_clp->net,
+ .net = mds_clp->cl_net,
};
struct rpc_timeout ds_timeout = {
.to_initval = 15 * HZ,
@@ -1701,7 +1701,7 @@ struct nfs_server *nfs4_create_referral_server(struct nfs_clone_mount *data,
rpc_protocol(parent_server->client),
parent_server->client->cl_timeout,
parent_client->cl_mvops->minor_version,
- parent_client->net);
+ parent_client->cl_net);
if (error < 0)
goto error;

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index ba3019f..dded556 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -415,7 +415,7 @@ static int __nfs_idmap_register(struct dentry *dir,
static void nfs_idmap_unregister(struct nfs_client *clp,
struct rpc_pipe *pipe)
{
- struct net *net = clp->net;
+ struct net *net = clp->cl_net;
struct super_block *pipefs_sb;

pipefs_sb = rpc_get_sb_net(net);
@@ -429,7 +429,7 @@ static int nfs_idmap_register(struct nfs_client *clp,
struct idmap *idmap,
struct rpc_pipe *pipe)
{
- struct net *net = clp->net;
+ struct net *net = clp->cl_net;
struct super_block *pipefs_sb;
int err = 0;

diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index c9cff9a..22ab5ca 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -602,7 +602,7 @@ decode_device(struct inode *ino, struct pnfs_device *pdev, gfp_t gfp_flags)

mp_count = be32_to_cpup(p); /* multipath count */
for (j = 0; j < mp_count; j++) {
- da = decode_ds_addr(NFS_SERVER(ino)->nfs_client->net,
+ da = decode_ds_addr(NFS_SERVER(ino)->nfs_client->cl_net,
&stream, gfp_flags);
if (da)
list_add_tail(&da->da_node, &dsaddrs);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 773e021..59410b3 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -88,7 +88,7 @@ struct nfs_client {
struct fscache_cookie *fscache; /* client index cache cookie */
#endif

- struct net *net;
+ struct net *cl_net;
};

/*


2012-05-18 22:06:35

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 09/14] NFS: Force server to drop NFSv4 state

A SETCLIENTID boot verifier is nothing more than the client's boot
timestamp. An NFSv4 server is obligated to wipe all NFSv4 state for
an NFS client when the client presents an updated SETCLIENTID boot
verifier. This is how servers detect client reboots.

nfs4_reset_all_state() refreshes our client's boot verifier to trigger
a server to wipe this client's state. This function is invoked when
an NFSv4.1 server reports that it has revoked some or all of a
client's NFSv4 state.

Soon we want to get rid of the per-nfs_client cl_boot_time field,
however. Without cl_boot_time, our NFS client will need to find a
different way to force the server to purge the client's NFSv4 state.

Because these verifiers are opaque (ie, the server doesn't know or
care that they happen to be timestamps), we can do force the server
to wipe NFSv4 state by using the same trick we use now, but then
immediately afterwards establish a fresh client ID using the old boot
verifier again.

Hopefully there are no extra paranoid server implementations that keep
track of the client's boot verifiers and prevent clients from reusing
a previous one.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4proc.c | 9 +++++++--
fs/nfs/nfs4state.c | 13 +++++++++++--
3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 03d3ff0..f164c73 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -24,6 +24,7 @@ enum nfs4_client_state {
NFS4CLNT_RECALL_SLOT,
NFS4CLNT_LEASE_CONFIRM,
NFS4CLNT_SERVER_SCOPE_MISMATCH,
+ NFS4CLNT_PURGE_STATE,
};

enum nfs4_session_state {
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 73cfd9e..c56d547 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
{
__be32 verf[2];

- verf[0] = (__be32)clp->cl_boot_time.tv_sec;
- verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
+ if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
+ verf[0] = (__be32)CURRENT_TIME.tv_sec;
+ verf[1] = (__be32)CURRENT_TIME.tv_nsec;
+ } else {
+ verf[0] = (__be32)clp->cl_boot_time.tv_sec;
+ verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
+ }
memcpy(bootverf->data, verf, sizeof(bootverf->data));
}

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index f8c06de..32cce4a 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1615,7 +1615,7 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
static void nfs4_reset_all_state(struct nfs_client *clp)
{
if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
- clp->cl_boot_time = CURRENT_TIME;
+ set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
nfs4_state_start_reclaim_nograce(clp);
nfs4_schedule_state_manager(clp);
}
@@ -1631,7 +1631,6 @@ static void nfs41_handle_server_reboot(struct nfs_client *clp)

static void nfs41_handle_state_revoked(struct nfs_client *clp)
{
- /* Temporary */
nfs4_reset_all_state(clp);
}

@@ -1652,6 +1651,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
{
if (!flags)
return;
+
+ dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
+ __func__, clp->cl_hostname, clp->cl_clientid, flags);
+
if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
nfs41_handle_server_reboot(clp);
if (flags & (SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED |
@@ -1762,6 +1765,12 @@ static void nfs4_state_manager(struct nfs_client *clp)

/* Ensure exclusive access to NFSv4 state */
do {
+ if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
+ nfs4_reclaim_lease(clp);
+ clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
+ set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
+ }
+
if (test_and_clear_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
/* We're going to have to re-establish a clientid */
status = nfs4_reclaim_lease(clp);


2012-05-21 16:08:47

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 09/14] NFS: Force server to drop NFSv4 state

On Fri, May 18, 2012 at 06:06:33PM -0400, Chuck Lever wrote:
> A SETCLIENTID boot verifier is nothing more than the client's boot
> timestamp. An NFSv4 server is obligated to wipe all NFSv4 state for
> an NFS client when the client presents an updated SETCLIENTID boot
> verifier. This is how servers detect client reboots.
>
> nfs4_reset_all_state() refreshes our client's boot verifier to trigger
> a server to wipe this client's state. This function is invoked when
> an NFSv4.1 server reports that it has revoked some or all of a
> client's NFSv4 state.
>
> Soon we want to get rid of the per-nfs_client cl_boot_time field,

OK, I guess you don't need it if you never intend to send a callback
update or anything?

> however. Without cl_boot_time, our NFS client will need to find a
> different way to force the server to purge the client's NFSv4 state.
>
> Because these verifiers are opaque (ie, the server doesn't know or
> care that they happen to be timestamps), we can do force the server
> to wipe NFSv4 state by using the same trick we use now, but then
> immediately afterwards establish a fresh client ID using the old boot
> verifier again.
>
> Hopefully there are no extra paranoid server implementations that keep
> track of the client's boot verifiers and prevent clients from reusing
> a previous one.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/nfs/nfs4_fs.h | 1 +
> fs/nfs/nfs4proc.c | 9 +++++++--
> fs/nfs/nfs4state.c | 13 +++++++++++--
> 3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 03d3ff0..f164c73 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -24,6 +24,7 @@ enum nfs4_client_state {
> NFS4CLNT_RECALL_SLOT,
> NFS4CLNT_LEASE_CONFIRM,
> NFS4CLNT_SERVER_SCOPE_MISMATCH,
> + NFS4CLNT_PURGE_STATE,
> };
>
> enum nfs4_session_state {
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 73cfd9e..c56d547 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> {
> __be32 verf[2];
>
> - verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> - verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> + verf[0] = (__be32)CURRENT_TIME.tv_sec;
> + verf[1] = (__be32)CURRENT_TIME.tv_nsec;

I suppose it's pretty unlikely this could happen within a jiffy of
setting cl_boot_time?

Would it be simpler to use some special value (all-zeros?) instead?

--b.

> + } else {
> + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> + }
> memcpy(bootverf->data, verf, sizeof(bootverf->data));
> }
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index f8c06de..32cce4a 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1615,7 +1615,7 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
> static void nfs4_reset_all_state(struct nfs_client *clp)
> {
> if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
> - clp->cl_boot_time = CURRENT_TIME;
> + set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
> nfs4_state_start_reclaim_nograce(clp);
> nfs4_schedule_state_manager(clp);
> }
> @@ -1631,7 +1631,6 @@ static void nfs41_handle_server_reboot(struct nfs_client *clp)
>
> static void nfs41_handle_state_revoked(struct nfs_client *clp)
> {
> - /* Temporary */
> nfs4_reset_all_state(clp);
> }
>
> @@ -1652,6 +1651,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
> {
> if (!flags)
> return;
> +
> + dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
> + __func__, clp->cl_hostname, clp->cl_clientid, flags);
> +
> if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
> nfs41_handle_server_reboot(clp);
> if (flags & (SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED |
> @@ -1762,6 +1765,12 @@ static void nfs4_state_manager(struct nfs_client *clp)
>
> /* Ensure exclusive access to NFSv4 state */
> do {
> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> + nfs4_reclaim_lease(clp);
> + clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
> + set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
> + }
> +
> if (test_and_clear_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
> /* We're going to have to re-establish a clientid */
> status = nfs4_reclaim_lease(clp);
>
> --
> 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

2012-05-21 16:11:17

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 09/14] NFS: Force server to drop NFSv4 state

On Mon, May 21, 2012 at 12:08:44PM -0400, bfields wrote:
> On Fri, May 18, 2012 at 06:06:33PM -0400, Chuck Lever wrote:
> > A SETCLIENTID boot verifier is nothing more than the client's boot
> > timestamp. An NFSv4 server is obligated to wipe all NFSv4 state for
> > an NFS client when the client presents an updated SETCLIENTID boot
> > verifier. This is how servers detect client reboots.
> >
> > nfs4_reset_all_state() refreshes our client's boot verifier to trigger
> > a server to wipe this client's state. This function is invoked when
> > an NFSv4.1 server reports that it has revoked some or all of a
> > client's NFSv4 state.
> >
> > Soon we want to get rid of the per-nfs_client cl_boot_time field,
>
> OK, I guess you don't need it if you never intend to send a callback
> update or anything?

(Never mind, I see in the next patch you're still remembering the
verifier, just giving it a broader scope?)

--b.

>
> > however. Without cl_boot_time, our NFS client will need to find a
> > different way to force the server to purge the client's NFSv4 state.
> >
> > Because these verifiers are opaque (ie, the server doesn't know or
> > care that they happen to be timestamps), we can do force the server
> > to wipe NFSv4 state by using the same trick we use now, but then
> > immediately afterwards establish a fresh client ID using the old boot
> > verifier again.
> >
> > Hopefully there are no extra paranoid server implementations that keep
> > track of the client's boot verifiers and prevent clients from reusing
> > a previous one.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> >
> > fs/nfs/nfs4_fs.h | 1 +
> > fs/nfs/nfs4proc.c | 9 +++++++--
> > fs/nfs/nfs4state.c | 13 +++++++++++--
> > 3 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> > index 03d3ff0..f164c73 100644
> > --- a/fs/nfs/nfs4_fs.h
> > +++ b/fs/nfs/nfs4_fs.h
> > @@ -24,6 +24,7 @@ enum nfs4_client_state {
> > NFS4CLNT_RECALL_SLOT,
> > NFS4CLNT_LEASE_CONFIRM,
> > NFS4CLNT_SERVER_SCOPE_MISMATCH,
> > + NFS4CLNT_PURGE_STATE,
> > };
> >
> > enum nfs4_session_state {
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 73cfd9e..c56d547 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> > {
> > __be32 verf[2];
> >
> > - verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> > - verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> > + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> > + verf[0] = (__be32)CURRENT_TIME.tv_sec;
> > + verf[1] = (__be32)CURRENT_TIME.tv_nsec;
>
> I suppose it's pretty unlikely this could happen within a jiffy of
> setting cl_boot_time?
>
> Would it be simpler to use some special value (all-zeros?) instead?
>
> --b.
>
> > + } else {
> > + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> > + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> > + }
> > memcpy(bootverf->data, verf, sizeof(bootverf->data));
> > }
> >
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index f8c06de..32cce4a 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -1615,7 +1615,7 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
> > static void nfs4_reset_all_state(struct nfs_client *clp)
> > {
> > if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
> > - clp->cl_boot_time = CURRENT_TIME;
> > + set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
> > nfs4_state_start_reclaim_nograce(clp);
> > nfs4_schedule_state_manager(clp);
> > }
> > @@ -1631,7 +1631,6 @@ static void nfs41_handle_server_reboot(struct nfs_client *clp)
> >
> > static void nfs41_handle_state_revoked(struct nfs_client *clp)
> > {
> > - /* Temporary */
> > nfs4_reset_all_state(clp);
> > }
> >
> > @@ -1652,6 +1651,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
> > {
> > if (!flags)
> > return;
> > +
> > + dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
> > + __func__, clp->cl_hostname, clp->cl_clientid, flags);
> > +
> > if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
> > nfs41_handle_server_reboot(clp);
> > if (flags & (SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED |
> > @@ -1762,6 +1765,12 @@ static void nfs4_state_manager(struct nfs_client *clp)
> >
> > /* Ensure exclusive access to NFSv4 state */
> > do {
> > + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> > + nfs4_reclaim_lease(clp);
> > + clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
> > + set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
> > + }
> > +
> > if (test_and_clear_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
> > /* We're going to have to re-establish a clientid */
> > status = nfs4_reclaim_lease(clp);
> >
> > --
> > 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

2012-05-18 22:07:01

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 12/14] NFS: Refactor nfs_get_client(): initialize nfs_client

Clean up: Continue to rationalize the locking in nfs_get_client() by
moving the logic that handles the case where a matching server IP
address is not found.

When we support server trunking detection, client initialization may
return a different nfs_client struct than was passed to it. Change
the synopsis of the init_client methods to return an nfs_client.

The client initialization logic in nfs_get_client() is not much more
than a wrapper around ->init_client. It's simpler to keep the little
bits of error handling in the version-specific init_client methods.

No behavior change is expected.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 76 ++++++++++++++++++++++++++---------------------
fs/nfs/internal.h | 19 ++++++------
include/linux/nfs_xdr.h | 3 +-
3 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index e3bf0bd..c19af91 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -544,7 +544,6 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
int noresvport)
{
struct nfs_client *clp, *new = NULL;
- int error;
struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id);

dprintk("--> nfs_get_client(%s,v%u)\n",
@@ -561,8 +560,13 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
nfs_free_client(new);
return nfs_found_client(cl_init, clp);
}
- if (new)
- goto install_client;
+ if (new) {
+ list_add(&new->cl_share_link, &nn->nfs_client_list);
+ spin_unlock(&nn->nfs_client_lock);
+ return cl_init->rpc_ops->init_client(new,
+ timeparms, ip_addr,
+ authflavour, noresvport);
+ }

spin_unlock(&nn->nfs_client_lock);

@@ -572,21 +576,6 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
dprintk("<-- nfs_get_client() Failed to find %s (%ld)\n",
cl_init->hostname ?: "", PTR_ERR(new));
return new;
-
- /* install a new client and return with it unready */
-install_client:
- clp = new;
- list_add(&clp->cl_share_link, &nn->nfs_client_list);
- spin_unlock(&nn->nfs_client_lock);
-
- error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr,
- authflavour, noresvport);
- if (error < 0) {
- nfs_put_client(clp);
- return ERR_PTR(error);
- }
- dprintk("--> nfs_get_client() = %p [new]\n", clp);
- return clp;
}

/*
@@ -811,10 +800,19 @@ static int nfs_init_server_rpcclient(struct nfs_server *server,
return 0;
}

-/*
- * Initialise an NFS2 or NFS3 client
+/**
+ * nfs_init_client - Initialise an NFS2 or NFS3 client
+ *
+ * @clp: nfs_client to initialise
+ * @timeparms: timeout parameters for underlying RPC transport
+ * @ip_addr: IP presentation address (not used)
+ * @authflavor: authentication flavor for underlying RPC transport
+ * @noresvport: set if RPC transport can use an ephemeral source port
+ *
+ * Returns pointer to an NFS client, or an ERR_PTR value.
*/
-int nfs_init_client(struct nfs_client *clp, const struct rpc_timeout *timeparms,
+struct nfs_client *nfs_init_client(struct nfs_client *clp,
+ const struct rpc_timeout *timeparms,
const char *ip_addr, rpc_authflavor_t authflavour,
int noresvport)
{
@@ -823,7 +821,7 @@ int nfs_init_client(struct nfs_client *clp, const struct rpc_timeout *timeparms,
if (clp->cl_cons_state == NFS_CS_READY) {
/* the client is already initialised */
dprintk("<-- nfs_init_client() = 0 [already %p]\n", clp);
- return 0;
+ return clp;
}

/*
@@ -835,12 +833,13 @@ int nfs_init_client(struct nfs_client *clp, const struct rpc_timeout *timeparms,
if (error < 0)
goto error;
nfs_mark_client_ready(clp, NFS_CS_READY);
- return 0;
+ return clp;

error:
nfs_mark_client_ready(clp, error);
+ nfs_put_client(clp);
dprintk("<-- nfs_init_client() = xerror %d\n", error);
- return error;
+ return ERR_PTR(error);
}

/*
@@ -1346,14 +1345,22 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
return nfs4_init_callback(clp);
}

-/*
- * Initialise an NFS4 client record
+/**
+ * nfs4_init_client - Initialise an NFS4 client record
+ *
+ * @clp: nfs_client to initialise
+ * @timeparms: timeout parameters for underlying RPC transport
+ * @ip_addr: callback IP address in presentation format
+ * @authflavor: authentication flavor for underlying RPC transport
+ * @noresvport: set if RPC transport can use an ephemeral source port
+ *
+ * Returns pointer to an NFS client, or an ERR_PTR value.
*/
-int nfs4_init_client(struct nfs_client *clp,
- const struct rpc_timeout *timeparms,
- const char *ip_addr,
- rpc_authflavor_t authflavour,
- int noresvport)
+struct nfs_client *nfs4_init_client(struct nfs_client *clp,
+ const struct rpc_timeout *timeparms,
+ const char *ip_addr,
+ rpc_authflavor_t authflavour,
+ int noresvport)
{
char buf[INET6_ADDRSTRLEN + 1];
int error;
@@ -1361,7 +1368,7 @@ int nfs4_init_client(struct nfs_client *clp,
if (clp->cl_cons_state == NFS_CS_READY) {
/* the client is initialised already */
dprintk("<-- nfs4_init_client() = 0 [already %p]\n", clp);
- return 0;
+ return clp;
}

/* Check NFS protocol revision and initialize RPC op vector */
@@ -1401,12 +1408,13 @@ int nfs4_init_client(struct nfs_client *clp,

if (!nfs4_has_session(clp))
nfs_mark_client_ready(clp, NFS_CS_READY);
- return 0;
+ return clp;

error:
nfs_mark_client_ready(clp, error);
+ nfs_put_client(clp);
dprintk("<-- nfs4_init_client() = xerror %d\n", error);
- return error;
+ return ERR_PTR(error);
}

/*
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 93224ac..687dab2 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -154,6 +154,16 @@ extern struct nfs_client *nfs4_find_client_ident(struct net *, int);
extern struct nfs_client *
nfs4_find_client_sessionid(struct net *, const struct sockaddr *,
struct nfs4_sessionid *);
+extern struct nfs_client *nfs_init_client(struct nfs_client *clp,
+ const struct rpc_timeout *timeparms,
+ const char *ip_addr,
+ rpc_authflavor_t authflavour,
+ int noresvport);
+extern struct nfs_client *nfs4_init_client(struct nfs_client *clp,
+ const struct rpc_timeout *timeparms,
+ const char *ip_addr,
+ rpc_authflavor_t authflavour,
+ int noresvport);
extern struct nfs_server *nfs_create_server(
const struct nfs_parsed_mount_data *,
struct nfs_fh *);
@@ -241,10 +251,6 @@ extern int nfs4_init_ds_session(struct nfs_client *clp);

/* proc.c */
void nfs_close_context(struct nfs_open_context *ctx, int is_sync);
-extern int nfs_init_client(struct nfs_client *clp,
- const struct rpc_timeout *timeparms,
- const char *ip_addr, rpc_authflavor_t authflavour,
- int noresvport);

/* dir.c */
extern int nfs_access_cache_shrinker(struct shrinker *shrink,
@@ -345,11 +351,6 @@ extern int nfs_migrate_page(struct address_space *,
/* nfs4proc.c */
extern void __nfs4_read_done_cb(struct nfs_read_data *);
extern void nfs4_reset_read(struct rpc_task *task, struct nfs_read_data *data);
-extern int nfs4_init_client(struct nfs_client *clp,
- const struct rpc_timeout *timeparms,
- const char *ip_addr,
- rpc_authflavor_t authflavour,
- int noresvport);
extern void nfs4_reset_write(struct rpc_task *task, struct nfs_write_data *data);
extern int _nfs4_call_sync(struct rpc_clnt *clnt,
struct nfs_server *server,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index b23029d..a789c1e 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1287,7 +1287,8 @@ struct nfs_rpc_ops {
struct nfs_open_context *ctx,
int open_flags,
struct iattr *iattr);
- int (*init_client) (struct nfs_client *, const struct rpc_timeout *,
+ struct nfs_client *
+ (*init_client) (struct nfs_client *, const struct rpc_timeout *,
const char *, rpc_authflavor_t, int);
int (*secinfo)(struct inode *, const struct qstr *, struct nfs4_secinfo_flavors *);
};


2012-05-21 16:11:59

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 09/14] NFS: Force server to drop NFSv4 state


On May 21, 2012, at 12:08 PM, J. Bruce Fields wrote:

> On Fri, May 18, 2012 at 06:06:33PM -0400, Chuck Lever wrote:
>> A SETCLIENTID boot verifier is nothing more than the client's boot
>> timestamp. An NFSv4 server is obligated to wipe all NFSv4 state for
>> an NFS client when the client presents an updated SETCLIENTID boot
>> verifier. This is how servers detect client reboots.
>>
>> nfs4_reset_all_state() refreshes our client's boot verifier to trigger
>> a server to wipe this client's state. This function is invoked when
>> an NFSv4.1 server reports that it has revoked some or all of a
>> client's NFSv4 state.
>>
>> Soon we want to get rid of the per-nfs_client cl_boot_time field,
>
> OK, I guess you don't need it if you never intend to send a callback
> update or anything?

"Get rid of" is perhaps too strong. I'm moving it to nfs_net.

>> however. Without cl_boot_time, our NFS client will need to find a
>> different way to force the server to purge the client's NFSv4 state.
>>
>> Because these verifiers are opaque (ie, the server doesn't know or
>> care that they happen to be timestamps), we can do force the server
>> to wipe NFSv4 state by using the same trick we use now, but then
>> immediately afterwards establish a fresh client ID using the old boot
>> verifier again.
>>
>> Hopefully there are no extra paranoid server implementations that keep
>> track of the client's boot verifiers and prevent clients from reusing
>> a previous one.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> fs/nfs/nfs4_fs.h | 1 +
>> fs/nfs/nfs4proc.c | 9 +++++++--
>> fs/nfs/nfs4state.c | 13 +++++++++++--
>> 3 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index 03d3ff0..f164c73 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -24,6 +24,7 @@ enum nfs4_client_state {
>> NFS4CLNT_RECALL_SLOT,
>> NFS4CLNT_LEASE_CONFIRM,
>> NFS4CLNT_SERVER_SCOPE_MISMATCH,
>> + NFS4CLNT_PURGE_STATE,
>> };
>>
>> enum nfs4_session_state {
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 73cfd9e..c56d547 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
>> {
>> __be32 verf[2];
>>
>> - verf[0] = (__be32)clp->cl_boot_time.tv_sec;
>> - verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
>> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
>> + verf[0] = (__be32)CURRENT_TIME.tv_sec;
>> + verf[1] = (__be32)CURRENT_TIME.tv_nsec;
>
> I suppose it's pretty unlikely this could happen within a jiffy of
> setting cl_boot_time?

Boot time has nanosecond resolution. I'm pretty sure we are safe here.

> Would it be simpler to use some special value (all-zeros?) instead?

We'd have to pick a value that was guaranteed never to be a valid time stamp.

>
> --b.
>
>> + } else {
>> + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
>> + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
>> + }
>> memcpy(bootverf->data, verf, sizeof(bootverf->data));
>> }
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index f8c06de..32cce4a 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1615,7 +1615,7 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
>> static void nfs4_reset_all_state(struct nfs_client *clp)
>> {
>> if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
>> - clp->cl_boot_time = CURRENT_TIME;
>> + set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
>> nfs4_state_start_reclaim_nograce(clp);
>> nfs4_schedule_state_manager(clp);
>> }
>> @@ -1631,7 +1631,6 @@ static void nfs41_handle_server_reboot(struct nfs_client *clp)
>>
>> static void nfs41_handle_state_revoked(struct nfs_client *clp)
>> {
>> - /* Temporary */
>> nfs4_reset_all_state(clp);
>> }
>>
>> @@ -1652,6 +1651,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
>> {
>> if (!flags)
>> return;
>> +
>> + dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
>> + __func__, clp->cl_hostname, clp->cl_clientid, flags);
>> +
>> if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
>> nfs41_handle_server_reboot(clp);
>> if (flags & (SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED |
>> @@ -1762,6 +1765,12 @@ static void nfs4_state_manager(struct nfs_client *clp)
>>
>> /* Ensure exclusive access to NFSv4 state */
>> do {
>> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
>> + nfs4_reclaim_lease(clp);
>> + clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
>> + set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
>> + }
>> +
>> if (test_and_clear_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
>> /* We're going to have to re-establish a clientid */
>> status = nfs4_reclaim_lease(clp);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

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





2012-05-18 22:07:18

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 14/14] NFS: EXCHANGE_ID should save the server major and minor ID

Save the server major and minor ID results from EXCHANGE_ID, as they
are needed for detecting server trunking.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 1 +
fs/nfs/nfs4proc.c | 17 ++++++++++++++++-
fs/nfs/nfs4xdr.c | 13 ++++++++-----
include/linux/nfs_fs_sb.h | 1 +
include/linux/nfs_xdr.h | 3 ++-
5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d5613d3..8319116 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -235,6 +235,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
nfs_idmap_delete(clp);

rpc_destroy_wait_queue(&clp->cl_rpcwaitq);
+ kfree(clp->cl_serverowner);
kfree(clp->cl_serverscope);
kfree(clp->cl_implid);
}
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index aaf6b28..fa08302 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5135,11 +5135,18 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
clp->cl_rpcclient->cl_nodename,
clp->cl_rpcclient->cl_auth->au_flavor);

+ res.server_owner = kzalloc(sizeof(struct nfs41_server_owner),
+ GFP_KERNEL);
+ if (unlikely(res.server_owner == NULL)) {
+ status = -ENOMEM;
+ goto out;
+ }
+
res.server_scope = kzalloc(sizeof(struct nfs41_server_scope),
GFP_KERNEL);
if (unlikely(res.server_scope == NULL)) {
status = -ENOMEM;
- goto out;
+ goto out_server_owner;
}

res.impl_id = kzalloc(sizeof(struct nfs41_impl_id), GFP_KERNEL);
@@ -5153,6 +5160,12 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
status = nfs4_check_cl_exchange_flags(clp->cl_exchange_flags);

if (status == 0) {
+ kfree(clp->cl_serverowner);
+ clp->cl_serverowner = res.server_owner;
+ res.server_owner = NULL;
+ }
+
+ if (status == 0) {
/* use the most recent implementation id */
kfree(clp->cl_implid);
clp->cl_implid = res.impl_id;
@@ -5176,6 +5189,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
}
}

+out_server_owner:
+ kfree(res.server_owner);
out_server_scope:
kfree(res.server_scope);
out:
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 4538df1..647be1b 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -5181,24 +5181,27 @@ static int decode_exchange_id(struct xdr_stream *xdr,
if (dummy != SP4_NONE)
return -EIO;

- /* Throw away minor_id */
+ /* server_owner4.so_minor_id */
p = xdr_inline_decode(xdr, 8);
if (unlikely(!p))
goto out_overflow;
+ p = xdr_decode_hyper(p, &res->server_owner->minor_id);

- /* Throw away Major id */
+ /* server_owner4.so_major_id */
status = decode_opaque_inline(xdr, &dummy, &dummy_str);
if (unlikely(status))
return status;
+ if (unlikely(dummy > NFS4_OPAQUE_LIMIT))
+ return -EIO;
+ memcpy(res->server_owner->major_id, dummy_str, dummy);
+ res->server_owner->major_id_sz = dummy;

- /* Save server_scope */
+ /* server_scope4 */
status = decode_opaque_inline(xdr, &dummy, &dummy_str);
if (unlikely(status))
return status;
-
if (unlikely(dummy > NFS4_OPAQUE_LIMIT))
return -EIO;
-
memcpy(res->server_scope->server_scope, dummy_str, dummy);
res->server_scope->server_scope_sz = dummy;

diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 3a99f52..fbb78fb 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -80,6 +80,7 @@ struct nfs_client {
/* The flags used for obtaining the clientid during EXCHANGE_ID */
u32 cl_exchange_flags;
struct nfs4_session *cl_session; /* shared session */
+ struct nfs41_server_owner *cl_serverowner;
struct nfs41_server_scope *cl_serverscope;
struct nfs41_impl_id *cl_implid;
#endif /* CONFIG_NFS_V4 */
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index a1f368c..4eb4ac7 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1070,7 +1070,7 @@ struct nfs41_exchange_id_args {
u32 flags;
};

-struct server_owner {
+struct nfs41_server_owner {
uint64_t minor_id;
uint32_t major_id_sz;
char major_id[NFS4_OPAQUE_LIMIT];
@@ -1090,6 +1090,7 @@ struct nfs41_impl_id {
struct nfs41_exchange_id_res {
struct nfs_client *client;
u32 flags;
+ struct nfs41_server_owner *server_owner;
struct nfs41_server_scope *server_scope;
struct nfs41_impl_id *impl_id;
};


2012-05-21 15:06:56

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 03/14] NFS: Use proper naming conventions for nfs_client.impl_id field

This looks good to me.

-dros

On May 18, 2012, at 6:05 PM, Chuck Lever wrote:

> Clean up: When naming fields and data types, follow established
> conventions to facilitate accurate grep/cscope searches.
>
> Additionally, for consistency, move the impl_id field into the NFSv4-
> specific part of the nfs_client, and free that memory in the logic
> that shuts down NFSv4 nfs_clients.
>
> Introduced by commit 7d2ed9ac "NFSv4: parse and display server
> implementation ids," Fri Feb 17, 2012.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/nfs/client.c | 2 +-
> fs/nfs/nfs4proc.c | 12 ++++++------
> fs/nfs/super.c | 4 ++--
> include/linux/nfs_fs_sb.h | 2 +-
> 4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1285f70..ec5a276 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -236,6 +236,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
>
> rpc_destroy_wait_queue(&clp->cl_rpcwaitq);
> kfree(clp->cl_serverscope);
> + kfree(clp->cl_implid);
> }
>
> /* idr_remove_all is not needed as all id's are removed by nfs_put_client */
> @@ -304,7 +305,6 @@ static void nfs_free_client(struct nfs_client *clp)
>
> put_net(clp->net);
> kfree(clp->cl_hostname);
> - kfree(clp->impl_id);
> kfree(clp);
>
> dprintk("<-- nfs_free_client()\n");
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 35585ef..386a756 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5147,8 +5147,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>
> if (!status) {
> /* use the most recent implementation id */
> - kfree(clp->impl_id);
> - clp->impl_id = res.impl_id;
> + kfree(clp->cl_implid);
> + clp->cl_implid = res.impl_id;
> } else
> kfree(res.impl_id);
>
> @@ -5172,12 +5172,12 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
> out_server_scope:
> kfree(res.server_scope);
> out:
> - if (clp->impl_id)
> + if (clp->cl_implid)
> dprintk("%s: Server Implementation ID: "
> "domain: %s, name: %s, date: %llu,%u\n",
> - __func__, clp->impl_id->domain, clp->impl_id->name,
> - clp->impl_id->date.seconds,
> - clp->impl_id->date.nseconds);
> + __func__, clp->cl_implid->domain, clp->cl_implid->name,
> + clp->cl_implid->date.seconds,
> + clp->cl_implid->date.nseconds);
> dprintk("<-- %s status= %d\n", __func__, status);
> return status;
> }
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 4ac7fca..2c5390e 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -786,8 +786,8 @@ static void show_pnfs(struct seq_file *m, struct nfs_server *server)
>
> static void show_implementation_id(struct seq_file *m, struct nfs_server *nfss)
> {
> - if (nfss->nfs_client && nfss->nfs_client->impl_id) {
> - struct nfs41_impl_id *impl_id = nfss->nfs_client->impl_id;
> + if (nfss->nfs_client && nfss->nfs_client->cl_implid) {
> + struct nfs41_impl_id *impl_id = nfss->nfs_client->cl_implid;
> seq_printf(m, "\n\timpl_id:\tname='%s',domain='%s',"
> "date='%llu,%u'",
> impl_id->name, impl_id->domain,
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 900d733..773e021 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -81,13 +81,13 @@ struct nfs_client {
> u32 cl_exchange_flags;
> struct nfs4_session *cl_session; /* shared session */
> struct nfs41_server_scope *cl_serverscope;
> + struct nfs41_impl_id *cl_implid;
> #endif /* CONFIG_NFS_V4 */
>
> #ifdef CONFIG_NFS_FSCACHE
> struct fscache_cookie *fscache; /* client index cache cookie */
> #endif
>
> - struct nfs41_impl_id *impl_id; /* from exchange_id */
> struct net *net;
> };
>
>
> --
> 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


Attachments:
smime.p7s (1.34 kB)

2012-05-18 22:06:43

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 10/14] NFS: Always use the same SETCLIENTID boot verifier

Currently our NFS client assigns a unique SETCLIENTID boot verifier
for each server IP address it knows about. It's set to CURRENT_TIME
when the struct nfs_client for that server IP is created.

During the SETCLIENTID operation, our client also presents an
nfs_client_id4 string to servers, as an identifier on which the server
can hang all of this client's NFSv4 state. Our client's
nfs_client_id4 string is unique for each server IP address.

An NFSv4 server is obligated to wipe all NFSv4 state associated with
an nfs_client_id4 string when the client presents this string with a
changed SETCLIENTID boot verifier.

When our client unmounts the last of a server's shares, it destroys
that server's struct nfs_client. The next time the client mounts that
NFS server, it creates a fresh struct nfs_client with a fresh boot
verifier. On seeing the fresh verifer, the server wipes any previous
NFSv4 state associated with that nfs_client_id4.

However, NFSv4.1 clients are supposed to present the same
nfs_client_id4 string to all servers. And, to support Transparent
State Migration, the same nfs_client_id4 string should be presented
to all NFSv4.0 servers so they recognize that migrated state for this
client belongs with state a server may already have for this client.

If the nfs_client_id4 string is the same but the boot verifier changes
for each server IP address, SETCLIENTIDs from such a client could
unintentionally result in a server wiping the client's previously
obtained lease.

Thus, if our NFS client is going to use a fixed nfs_client_id4 string,
either for NFSv4.0 or NFSv4.1 mounts, our NFS client should use a
SETCLIENTID boot verifier that does not change depending on server IP
address.

Replace our current per-nfs_client boot verifier with a per-nfs_net
boot verifier.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 2 +-
fs/nfs/netns.h | 5 +++++
fs/nfs/nfs4proc.c | 14 ++++++++------
fs/nfs/nfs4xdr.c | 5 ++++-
include/linux/nfs_fs_sb.h | 3 ---
5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index c430acd..84bca37 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -182,7 +182,6 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
spin_lock_init(&clp->cl_lock);
INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
- clp->cl_boot_time = CURRENT_TIME;
clp->cl_state = 1 << NFS4CLNT_LEASE_EXPIRED;
clp->cl_minorversion = cl_init->minorversion;
clp->cl_mvops = nfs_v4_minor_ops[cl_init->minorversion];
@@ -1805,6 +1804,7 @@ void nfs_clients_init(struct net *net)
idr_init(&nn->cb_ident_idr);
#endif
spin_lock_init(&nn->nfs_client_lock);
+ nn->boot_time = CURRENT_TIME;
}

#ifdef CONFIG_PROC_FS
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index aa14ec3..8a6394e 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -1,3 +1,7 @@
+/*
+ * NFS-private data for each "struct net". Accessed with net_generic().
+ */
+
#ifndef __NFS_NETNS_H__
#define __NFS_NETNS_H__

@@ -20,6 +24,7 @@ struct nfs_net {
struct idr cb_ident_idr; /* Protected by nfs_client_lock */
#endif
spinlock_t nfs_client_lock;
+ struct timespec boot_time;
};

extern int nfs_net_id;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c56d547..aaf6b28 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -64,6 +64,7 @@
#include "iostat.h"
#include "callback.h"
#include "pnfs.h"
+#include "netns.h"

#define NFSDBG_FACILITY NFSDBG_PROC

@@ -3932,8 +3933,8 @@ wait_on_recovery:
return -EAGAIN;
}

-static void nfs4_construct_boot_verifier(struct nfs_client *clp,
- nfs4_verifier *bootverf)
+static void nfs4_init_boot_verifier(const struct nfs_client *clp,
+ nfs4_verifier *bootverf)
{
__be32 verf[2];

@@ -3941,8 +3942,9 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
verf[0] = (__be32)CURRENT_TIME.tv_sec;
verf[1] = (__be32)CURRENT_TIME.tv_nsec;
} else {
- verf[0] = (__be32)clp->cl_boot_time.tv_sec;
- verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
+ struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
+ verf[0] = (__be32)nn->boot_time.tv_sec;
+ verf[1] = (__be32)nn->boot_time.tv_nsec;
}
memcpy(bootverf->data, verf, sizeof(bootverf->data));
}
@@ -3966,7 +3968,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
int loop = 0;
int status;

- nfs4_construct_boot_verifier(clp, &sc_verifier);
+ nfs4_init_boot_verifier(clp, &sc_verifier);

for(;;) {
rcu_read_lock();
@@ -5125,7 +5127,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
dprintk("--> %s\n", __func__);
BUG_ON(clp == NULL);

- nfs4_construct_boot_verifier(clp, &verifier);
+ nfs4_init_boot_verifier(clp, &verifier);

args.id_len = scnprintf(args.id, sizeof(args.id),
"%s/%s/%u",
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index c54aae3..4538df1 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -53,9 +53,11 @@
#include <linux/nfs4.h>
#include <linux/nfs_fs.h>
#include <linux/nfs_idmap.h>
+
#include "nfs4_fs.h"
#include "internal.h"
#include "pnfs.h"
+#include "netns.h"

#define NFSDBG_FACILITY NFSDBG_XDR

@@ -1726,6 +1728,7 @@ static void encode_create_session(struct xdr_stream *xdr,
char machine_name[NFS4_MAX_MACHINE_NAME_LEN];
uint32_t len;
struct nfs_client *clp = args->client;
+ struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
u32 max_resp_sz_cached;

/*
@@ -1767,7 +1770,7 @@ static void encode_create_session(struct xdr_stream *xdr,
*p++ = cpu_to_be32(RPC_AUTH_UNIX); /* auth_sys */

/* authsys_parms rfc1831 */
- *p++ = cpu_to_be32((u32)clp->cl_boot_time.tv_nsec); /* stamp */
+ *p++ = (__be32)nn->boot_time.tv_nsec; /* stamp */
p = xdr_encode_opaque(p, machine_name, len);
*p++ = cpu_to_be32(0); /* UID */
*p++ = cpu_to_be32(0); /* GID */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 59410b3..fbec57d 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -61,9 +61,6 @@ struct nfs_client {

struct rpc_wait_queue cl_rpcwaitq;

- /* used for the setclientid verifier */
- struct timespec cl_boot_time;
-
/* idmapper */
struct idmap * cl_idmap;



2012-05-18 22:06:09

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 06/14] NFS: Remove nfs_unique_id

Clean up: this structure is unused.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/nfs4_fs.h | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index d05719e..03d3ff0 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -52,11 +52,6 @@ struct nfs4_minor_version_ops {
const struct nfs4_state_maintenance_ops *state_renewal_ops;
};

-struct nfs_unique_id {
- struct rb_node rb_node;
- __u64 id;
-};
-
#define NFS_SEQID_CONFIRMED 1
struct nfs_seqid_counter {
ktime_t create_time;


2012-05-21 15:40:08

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 07/14] NFS: Don't swap bytes in nfs4_construct_boot_verifier()

On Fri, May 18, 2012 at 06:06:16PM -0400, Chuck Lever wrote:
> The SETCLIENTID boot verifier is opaque to NFSv4 servers, thus there
> is no requirement for byte swapping before the client puts the
> verifier on the wire.
>
> This treatment is similar to other timestamp-based verifiers.

The fact that it's opaque means we don't *have* to be consistent about
byte-order. But we can still choose to do so. It may help debugging a
little (by making it just a little easier to decode the on-the-wire
verifier). And changing the encoding of the verifier means it won't
work over boots that cross kernel versions. (I'll admit that's *not* a
big deal--most clients probably take longer than most servers' lease
times anyway--but why do this without a reason?)

--b.

>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/nfs/nfs4proc.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index db7b76a..73cfd9e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3937,8 +3937,8 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> {
> __be32 verf[2];
>
> - verf[0] = htonl((u32)clp->cl_boot_time.tv_sec);
> - verf[1] = htonl((u32)clp->cl_boot_time.tv_nsec);
> + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> memcpy(bootverf->data, verf, sizeof(bootverf->data));
> }
>
>
> --
> 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

2012-05-18 22:05:26

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 01/14] NFS: Fix comment misspelling in struct nfs_client definition

Signed-off-by: Chuck Lever <[email protected]>
---

include/linux/nfs_fs_sb.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 7073fc7..5498e9d 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -79,7 +79,7 @@ struct nfs_client {
u32 cl_seqid;
/* The flags used for obtaining the clientid during EXCHANGE_ID */
u32 cl_exchange_flags;
- struct nfs4_session *cl_session; /* sharred session */
+ struct nfs4_session *cl_session; /* shared session */
#endif /* CONFIG_NFS_V4 */

#ifdef CONFIG_NFS_FSCACHE


2012-05-18 22:05:35

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 02/14] NFS: Use proper naming conventions for NFSv4.1 server scope fields

Clean up: When naming fields and data types, follow established
conventions to facilitate accurate grep/cscope searches.

Additionally, for consistency, move the scope field into the NFSv4-
specific part of the nfs_client, and free that memory in the logic
that shuts down NFSv4 nfs_clients.

Introduced by commit 99fe60d0 "nfs41: exchange_id operation", April
1 2009.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 2 +-
fs/nfs/nfs4_fs.h | 2 +-
fs/nfs/nfs4proc.c | 18 ++++++++++--------
include/linux/nfs_fs_sb.h | 4 ++--
include/linux/nfs_xdr.h | 4 ++--
5 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 60f7e4e..1285f70 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -235,6 +235,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
nfs_idmap_delete(clp);

rpc_destroy_wait_queue(&clp->cl_rpcwaitq);
+ kfree(clp->cl_serverscope);
}

/* idr_remove_all is not needed as all id's are removed by nfs_put_client */
@@ -303,7 +304,6 @@ static void nfs_free_client(struct nfs_client *clp)

put_net(clp->net);
kfree(clp->cl_hostname);
- kfree(clp->server_scope);
kfree(clp->impl_id);
kfree(clp);

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 8d75021..d05719e 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -334,7 +334,7 @@ extern void nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs
extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags);
extern void nfs41_handle_recall_slot(struct nfs_client *clp);
extern void nfs41_handle_server_scope(struct nfs_client *,
- struct server_scope **);
+ struct nfs41_server_scope **);
extern void nfs4_put_lock_state(struct nfs4_lock_state *lsp);
extern int nfs4_set_lock_state(struct nfs4_state *state, struct file_lock *fl);
extern void nfs4_select_rw_stateid(nfs4_stateid *, struct nfs4_state *,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 99650aa..35585ef 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5080,7 +5080,8 @@ out_inval:
}

static bool
-nfs41_same_server_scope(struct server_scope *a, struct server_scope *b)
+nfs41_same_server_scope(struct nfs41_server_scope *a,
+ struct nfs41_server_scope *b)
{
if (a->server_scope_sz == b->server_scope_sz &&
memcmp(a->server_scope, b->server_scope, a->server_scope_sz) == 0)
@@ -5127,7 +5128,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
clp->cl_rpcclient->cl_nodename,
clp->cl_rpcclient->cl_auth->au_flavor);

- res.server_scope = kzalloc(sizeof(struct server_scope), GFP_KERNEL);
+ res.server_scope = kzalloc(sizeof(struct nfs41_server_scope),
+ GFP_KERNEL);
if (unlikely(!res.server_scope)) {
status = -ENOMEM;
goto out;
@@ -5151,18 +5153,18 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
kfree(res.impl_id);

if (!status) {
- if (clp->server_scope &&
- !nfs41_same_server_scope(clp->server_scope,
+ if (clp->cl_serverscope &&
+ !nfs41_same_server_scope(clp->cl_serverscope,
res.server_scope)) {
dprintk("%s: server_scope mismatch detected\n",
__func__);
set_bit(NFS4CLNT_SERVER_SCOPE_MISMATCH, &clp->cl_state);
- kfree(clp->server_scope);
- clp->server_scope = NULL;
+ kfree(clp->cl_serverscope);
+ clp->cl_serverscope = NULL;
}

- if (!clp->server_scope) {
- clp->server_scope = res.server_scope;
+ if (!clp->cl_serverscope) {
+ clp->cl_serverscope = res.server_scope;
goto out;
}
}
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 5498e9d..900d733 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -17,7 +17,7 @@ struct nfs4_sequence_args;
struct nfs4_sequence_res;
struct nfs_server;
struct nfs4_minor_version_ops;
-struct server_scope;
+struct nfs41_server_scope;
struct nfs41_impl_id;

/*
@@ -80,13 +80,13 @@ struct nfs_client {
/* The flags used for obtaining the clientid during EXCHANGE_ID */
u32 cl_exchange_flags;
struct nfs4_session *cl_session; /* shared session */
+ struct nfs41_server_scope *cl_serverscope;
#endif /* CONFIG_NFS_V4 */

#ifdef CONFIG_NFS_FSCACHE
struct fscache_cookie *fscache; /* client index cache cookie */
#endif

- struct server_scope *server_scope; /* from exchange_id */
struct nfs41_impl_id *impl_id; /* from exchange_id */
struct net *net;
};
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 7ba3551..b23029d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1076,7 +1076,7 @@ struct server_owner {
char major_id[NFS4_OPAQUE_LIMIT];
};

-struct server_scope {
+struct nfs41_server_scope {
uint32_t server_scope_sz;
char server_scope[NFS4_OPAQUE_LIMIT];
};
@@ -1090,7 +1090,7 @@ struct nfs41_impl_id {
struct nfs41_exchange_id_res {
struct nfs_client *client;
u32 flags;
- struct server_scope *server_scope;
+ struct nfs41_server_scope *server_scope;
struct nfs41_impl_id *impl_id;
};



2012-05-18 22:06:18

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 07/14] NFS: Don't swap bytes in nfs4_construct_boot_verifier()

The SETCLIENTID boot verifier is opaque to NFSv4 servers, thus there
is no requirement for byte swapping before the client puts the
verifier on the wire.

This treatment is similar to other timestamp-based verifiers.

Signed-off-by: Chuck Lever <[email protected]>
---

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index db7b76a..73cfd9e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3937,8 +3937,8 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
{
__be32 verf[2];

- verf[0] = htonl((u32)clp->cl_boot_time.tv_sec);
- verf[1] = htonl((u32)clp->cl_boot_time.tv_nsec);
+ verf[0] = (__be32)clp->cl_boot_time.tv_sec;
+ verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
memcpy(bootverf->data, verf, sizeof(bootverf->data));
}



2012-05-21 15:47:52

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 07/14] NFS: Don't swap bytes in nfs4_construct_boot_verifier()

Hi-

On May 21, 2012, at 11:40 AM, J. Bruce Fields wrote:

> On Fri, May 18, 2012 at 06:06:16PM -0400, Chuck Lever wrote:
>> The SETCLIENTID boot verifier is opaque to NFSv4 servers, thus there
>> is no requirement for byte swapping before the client puts the
>> verifier on the wire.
>>
>> This treatment is similar to other timestamp-based verifiers.
>
> The fact that it's opaque means we don't *have* to be consistent about
> byte-order. But we can still choose to do so.

Agree, and that's consistent with "no requirement for byte swapping".

> It may help debugging a
> little (by making it just a little easier to decode the on-the-wire
> verifier).

In this case, I doubt it. We're talking about raw timestamps here, there's nothing we can really recognize in those. If this were a structured piece of data, I'd feel more agreement about debugging ease.

> And changing the encoding of the verifier means it won't
> work over boots that cross kernel versions.

The boot verifier is *supposed* to change over a reboot, so I think this is not consequential. The server is only looking for a mismatch between verifiers, and this changes preserves that semantic.

> (I'll admit that's *not* a
> big deal--most clients probably take longer than most servers' lease
> times anyway--but why do this without a reason?)

It reduces code clutter for the subsequent patch which adds the NFS4CLNT_PURGE_STATE bit.

>
> --b.
>
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> fs/nfs/nfs4proc.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index db7b76a..73cfd9e 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -3937,8 +3937,8 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
>> {
>> __be32 verf[2];
>>
>> - verf[0] = htonl((u32)clp->cl_boot_time.tv_sec);
>> - verf[1] = htonl((u32)clp->cl_boot_time.tv_nsec);
>> + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
>> + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
>> memcpy(bootverf->data, verf, sizeof(bootverf->data));
>> }
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

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





2012-05-18 22:07:09

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 13/14] NFS: Add nfs_client behavior flags

"noresvport" and "discrtry" can be passed to nfs_create_rpc_client()
by setting flags in the passed-in nfs_client. This change makes it
easy to add new flags.

Note that these settings are now "sticky" over the lifetime of a
struct nfs_client, and may even be copied when an nfs_client is
cloned.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 42 ++++++++++++++++++++----------------------
fs/nfs/internal.h | 6 ++----
include/linux/nfs_fs_sb.h | 3 +++
include/linux/nfs_xdr.h | 2 +-
4 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index c19af91..d5613d3 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -129,6 +129,7 @@ const struct rpc_program nfsacl_program = {
#endif /* CONFIG_NFS_V3_ACL */

struct nfs_client_initdata {
+ unsigned long init_flags;
const char *hostname;
const struct sockaddr *addr;
size_t addrlen;
@@ -540,8 +541,7 @@ static struct nfs_client *
nfs_get_client(const struct nfs_client_initdata *cl_init,
const struct rpc_timeout *timeparms,
const char *ip_addr,
- rpc_authflavor_t authflavour,
- int noresvport)
+ rpc_authflavor_t authflavour)
{
struct nfs_client *clp, *new = NULL;
struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id);
@@ -563,9 +563,10 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
if (new) {
list_add(&new->cl_share_link, &nn->nfs_client_list);
spin_unlock(&nn->nfs_client_lock);
+ new->cl_flags = cl_init->init_flags;
return cl_init->rpc_ops->init_client(new,
timeparms, ip_addr,
- authflavour, noresvport);
+ authflavour);
}

spin_unlock(&nn->nfs_client_lock);
@@ -649,8 +650,7 @@ static void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
*/
static int nfs_create_rpc_client(struct nfs_client *clp,
const struct rpc_timeout *timeparms,
- rpc_authflavor_t flavor,
- int discrtry, int noresvport)
+ rpc_authflavor_t flavor)
{
struct rpc_clnt *clnt = NULL;
struct rpc_create_args args = {
@@ -665,9 +665,9 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
.authflavor = flavor,
};

- if (discrtry)
+ if (test_bit(NFS_CS_DISCRTRY, &clp->cl_flags))
args.flags |= RPC_CLNT_CREATE_DISCRTRY;
- if (noresvport)
+ if (test_bit(NFS_CS_NORESVPORT, &clp->cl_flags))
args.flags |= RPC_CLNT_CREATE_NONPRIVPORT;

if (!IS_ERR(clp->cl_rpcclient))
@@ -807,14 +807,12 @@ static int nfs_init_server_rpcclient(struct nfs_server *server,
* @timeparms: timeout parameters for underlying RPC transport
* @ip_addr: IP presentation address (not used)
* @authflavor: authentication flavor for underlying RPC transport
- * @noresvport: set if RPC transport can use an ephemeral source port
*
* Returns pointer to an NFS client, or an ERR_PTR value.
*/
struct nfs_client *nfs_init_client(struct nfs_client *clp,
const struct rpc_timeout *timeparms,
- const char *ip_addr, rpc_authflavor_t authflavour,
- int noresvport)
+ const char *ip_addr, rpc_authflavor_t authflavour)
{
int error;

@@ -828,8 +826,7 @@ struct nfs_client *nfs_init_client(struct nfs_client *clp,
* Create a client RPC handle for doing FSSTAT with UNIX auth only
* - RFC 2623, sec 2.3.2
*/
- error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_UNIX,
- 0, noresvport);
+ error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_UNIX);
if (error < 0)
goto error;
nfs_mark_client_ready(clp, NFS_CS_READY);
@@ -869,10 +866,11 @@ static int nfs_init_server(struct nfs_server *server,

nfs_init_timeout_values(&timeparms, data->nfs_server.protocol,
data->timeo, data->retrans);
+ if (data->flags & NFS_MOUNT_NORESVPORT)
+ set_bit(NFS_CS_NORESVPORT, &cl_init.init_flags);

/* Allocate or find a client reference we can use */
- clp = nfs_get_client(&cl_init, &timeparms, NULL, RPC_AUTH_UNIX,
- data->flags & NFS_MOUNT_NORESVPORT);
+ clp = nfs_get_client(&cl_init, &timeparms, NULL, RPC_AUTH_UNIX);
if (IS_ERR(clp)) {
dprintk("<-- nfs_init_server() = error %ld\n", PTR_ERR(clp));
return PTR_ERR(clp);
@@ -1352,15 +1350,13 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
* @timeparms: timeout parameters for underlying RPC transport
* @ip_addr: callback IP address in presentation format
* @authflavor: authentication flavor for underlying RPC transport
- * @noresvport: set if RPC transport can use an ephemeral source port
*
* Returns pointer to an NFS client, or an ERR_PTR value.
*/
struct nfs_client *nfs4_init_client(struct nfs_client *clp,
const struct rpc_timeout *timeparms,
const char *ip_addr,
- rpc_authflavor_t authflavour,
- int noresvport)
+ rpc_authflavor_t authflavour)
{
char buf[INET6_ADDRSTRLEN + 1];
int error;
@@ -1374,8 +1370,8 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
/* Check NFS protocol revision and initialize RPC op vector */
clp->rpc_ops = &nfs_v4_clientops;

- error = nfs_create_rpc_client(clp, timeparms, authflavour,
- 1, noresvport);
+ __set_bit(NFS_CS_DISCRTRY, &clp->cl_flags);
+ error = nfs_create_rpc_client(clp, timeparms, authflavour);
if (error < 0)
goto error;

@@ -1443,9 +1439,11 @@ static int nfs4_set_client(struct nfs_server *server,

dprintk("--> nfs4_set_client()\n");

+ if (server->flags & NFS_MOUNT_NORESVPORT)
+ set_bit(NFS_CS_NORESVPORT, &cl_init.init_flags);
+
/* Allocate or find a client reference we can use */
- clp = nfs_get_client(&cl_init, timeparms, ip_addr, authflavour,
- server->flags & NFS_MOUNT_NORESVPORT);
+ clp = nfs_get_client(&cl_init, timeparms, ip_addr, authflavour);
if (IS_ERR(clp)) {
error = PTR_ERR(clp);
goto error;
@@ -1504,7 +1502,7 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
* (section 13.1 RFC 5661).
*/
clp = nfs_get_client(&cl_init, &ds_timeout, mds_clp->cl_ipaddr,
- mds_clp->cl_rpcclient->cl_auth->au_flavor, 0);
+ mds_clp->cl_rpcclient->cl_auth->au_flavor);

dprintk("<-- %s %p\n", __func__, clp);
return clp;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 687dab2..b9c6f1f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -157,13 +157,11 @@ nfs4_find_client_sessionid(struct net *, const struct sockaddr *,
extern struct nfs_client *nfs_init_client(struct nfs_client *clp,
const struct rpc_timeout *timeparms,
const char *ip_addr,
- rpc_authflavor_t authflavour,
- int noresvport);
+ rpc_authflavor_t authflavour);
extern struct nfs_client *nfs4_init_client(struct nfs_client *clp,
const struct rpc_timeout *timeparms,
const char *ip_addr,
- rpc_authflavor_t authflavour,
- int noresvport);
+ rpc_authflavor_t authflavour);
extern struct nfs_server *nfs_create_server(
const struct nfs_parsed_mount_data *,
struct nfs_fh *);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index fbec57d..3a99f52 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -35,6 +35,9 @@ struct nfs_client {
#define NFS_CS_RENEWD 3 /* - renewd started */
#define NFS_CS_STOP_RENEW 4 /* no more state to renew */
#define NFS_CS_CHECK_LEASE_TIME 5 /* need to check lease time */
+ unsigned long cl_flags; /* behavior switches */
+#define NFS_CS_NORESVPORT 0 /* - use ephemeral src port */
+#define NFS_CS_DISCRTRY 1 /* - disconnect on RPC retry */
struct sockaddr_storage cl_addr; /* server identifier */
size_t cl_addrlen;
char * cl_hostname; /* hostname of server */
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index a789c1e..a1f368c 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1289,7 +1289,7 @@ struct nfs_rpc_ops {
struct iattr *iattr);
struct nfs_client *
(*init_client) (struct nfs_client *, const struct rpc_timeout *,
- const char *, rpc_authflavor_t, int);
+ const char *, rpc_authflavor_t);
int (*secinfo)(struct inode *, const struct qstr *, struct nfs4_secinfo_flavors *);
};



2012-05-21 16:16:48

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 09/14] NFS: Force server to drop NFSv4 state

On Mon, May 21, 2012 at 12:11:48PM -0400, Chuck Lever wrote:
>
> On May 21, 2012, at 12:08 PM, J. Bruce Fields wrote:
>
> > On Fri, May 18, 2012 at 06:06:33PM -0400, Chuck Lever wrote:
> >> @@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> >> {
> >> __be32 verf[2];
> >>
> >> - verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> >> - verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> >> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> >> + verf[0] = (__be32)CURRENT_TIME.tv_sec;
> >> + verf[1] = (__be32)CURRENT_TIME.tv_nsec;
> >
> > I suppose it's pretty unlikely this could happen within a jiffy of
> > setting cl_boot_time?
>
> Boot time has nanosecond resolution. I'm pretty sure we are safe here.

CURRENT_TIME is only updated every jiffy. It would still have to happen
ridiculously fast, but I think that's milliseconds rather than
nanoseconds.

> > Would it be simpler to use some special value (all-zeros?) instead?
>
> We'd have to pick a value that was guaranteed never to be a valid time stamp.

A client that thinks it's currently Jan. 1 1970 probably has bigger
problems.

I dunno, your call.

--b.

2012-05-18 22:06:00

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 05/14] NFS: Clean up return code checking in nfs4_proc_exchange_id()

Clean up: prefer using the proper types in "if" expressions.

Signed-off-by: Chuck Lever <[email protected]>
---

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 386a756..db7b76a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5130,30 +5130,30 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)

res.server_scope = kzalloc(sizeof(struct nfs41_server_scope),
GFP_KERNEL);
- if (unlikely(!res.server_scope)) {
+ if (unlikely(res.server_scope == NULL)) {
status = -ENOMEM;
goto out;
}

res.impl_id = kzalloc(sizeof(struct nfs41_impl_id), GFP_KERNEL);
- if (unlikely(!res.impl_id)) {
+ if (unlikely(res.impl_id == NULL)) {
status = -ENOMEM;
goto out_server_scope;
}

status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
- if (!status)
+ if (status == 0)
status = nfs4_check_cl_exchange_flags(clp->cl_exchange_flags);

- if (!status) {
+ if (status == 0) {
/* use the most recent implementation id */
kfree(clp->cl_implid);
clp->cl_implid = res.impl_id;
} else
kfree(res.impl_id);

- if (!status) {
- if (clp->cl_serverscope &&
+ if (status == 0) {
+ if (clp->cl_serverscope != NULL &&
!nfs41_same_server_scope(clp->cl_serverscope,
res.server_scope)) {
dprintk("%s: server_scope mismatch detected\n",
@@ -5163,7 +5163,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
clp->cl_serverscope = NULL;
}

- if (!clp->cl_serverscope) {
+ if (clp->cl_serverscope == NULL) {
clp->cl_serverscope = res.server_scope;
goto out;
}
@@ -5172,7 +5172,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
out_server_scope:
kfree(res.server_scope);
out:
- if (clp->cl_implid)
+ if (clp->cl_implid != NULL)
dprintk("%s: Server Implementation ID: "
"domain: %s, name: %s, date: %llu,%u\n",
__func__, clp->cl_implid->domain, clp->cl_implid->name,


2012-05-18 22:06:52

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 11/14] NFS: Refactor nfs_get_client(): add nfs_found_client()

Clean up: Code that takes and releases nfs_client_lock remains in
nfs_get_client(). Logic that handles a pre-existing nfs_client is
moved to a separate function.

No behavior change is expected.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/client.c | 67 ++++++++++++++++++++++++++++++-------------------------
1 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 84bca37..e3bf0bd 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -504,6 +504,35 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
}

/*
+ * Found an existing client. Make sure it's ready before returning.
+ */
+static struct nfs_client *
+nfs_found_client(const struct nfs_client_initdata *cl_init,
+ struct nfs_client *clp)
+{
+ int error;
+
+ error = wait_event_killable(nfs_client_active_wq,
+ clp->cl_cons_state < NFS_CS_INITING);
+ if (error < 0) {
+ nfs_put_client(clp);
+ return ERR_PTR(-ERESTARTSYS);
+ }
+
+ if (clp->cl_cons_state < NFS_CS_READY) {
+ error = clp->cl_cons_state;
+ nfs_put_client(clp);
+ return ERR_PTR(error);
+ }
+
+ BUG_ON(clp->cl_cons_state != NFS_CS_READY);
+
+ dprintk("<-- %s found nfs_client %p for %s\n",
+ __func__, clp, cl_init->hostname ?: "");
+ return clp;
+}
+
+/*
* Look up a client by IP address and protocol version
* - creates a new record if one doesn't yet exist
*/
@@ -526,8 +555,12 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
spin_lock(&nn->nfs_client_lock);

clp = nfs_match_client(cl_init);
- if (clp)
- goto found_client;
+ if (clp) {
+ spin_unlock(&nn->nfs_client_lock);
+ if (new)
+ nfs_free_client(new);
+ return nfs_found_client(cl_init, clp);
+ }
if (new)
goto install_client;

@@ -536,7 +569,8 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
new = nfs_alloc_client(cl_init);
} while (!IS_ERR(new));

- dprintk("--> nfs_get_client() = %ld [failed]\n", PTR_ERR(new));
+ dprintk("<-- nfs_get_client() Failed to find %s (%ld)\n",
+ cl_init->hostname ?: "", PTR_ERR(new));
return new;

/* install a new client and return with it unready */
@@ -553,33 +587,6 @@ install_client:
}
dprintk("--> nfs_get_client() = %p [new]\n", clp);
return clp;
-
- /* found an existing client
- * - make sure it's ready before returning
- */
-found_client:
- spin_unlock(&nn->nfs_client_lock);
-
- if (new)
- nfs_free_client(new);
-
- error = wait_event_killable(nfs_client_active_wq,
- clp->cl_cons_state < NFS_CS_INITING);
- if (error < 0) {
- nfs_put_client(clp);
- return ERR_PTR(-ERESTARTSYS);
- }
-
- if (clp->cl_cons_state < NFS_CS_READY) {
- error = clp->cl_cons_state;
- nfs_put_client(clp);
- return ERR_PTR(error);
- }
-
- BUG_ON(clp->cl_cons_state != NFS_CS_READY);
-
- dprintk("--> nfs_get_client() = %p [share]\n", clp);
- return clp;
}

/*


2012-05-18 22:06:26

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH 08/14] NFS: Add NFSDBG_STATE

fs/nfs/nfs4state.c does not yet have any dprintk() call sites, and I'm
about to introduce some. We will need a new flag for enabling them.

Signed-off-by: Chuck Lever <[email protected]>
---

fs/nfs/nfs4renewd.c | 2 +-
fs/nfs/nfs4state.c | 2 ++
include/linux/nfs_fs.h | 1 +
3 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4renewd.c b/fs/nfs/nfs4renewd.c
index dc484c0..6930bec 100644
--- a/fs/nfs/nfs4renewd.c
+++ b/fs/nfs/nfs4renewd.c
@@ -49,7 +49,7 @@
#include "nfs4_fs.h"
#include "delegation.h"

-#define NFSDBG_FACILITY NFSDBG_PROC
+#define NFSDBG_FACILITY NFSDBG_STATE

void
nfs4_renew_state(struct work_struct *work)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 7f0fcfc..f8c06de 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -57,6 +57,8 @@
#include "internal.h"
#include "pnfs.h"

+#define NFSDBG_FACILITY NFSDBG_STATE
+
#define OPENOWNER_POOL_SIZE 8

const nfs4_stateid zero_stateid;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 52a1bdb..812a945 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -654,6 +654,7 @@ nfs_fileid_to_ino_t(u64 fileid)
#define NFSDBG_FSCACHE 0x0800
#define NFSDBG_PNFS 0x1000
#define NFSDBG_PNFS_LD 0x2000
+#define NFSDBG_STATE 0x4000
#define NFSDBG_ALL 0xFFFF

#ifdef __KERNEL__


2012-05-19 21:08:50

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 02/14] NFS: Use proper naming conventions for NFSv4.1 server scope fields

T24gRnJpLCAyMDEyLTA1LTE4IGF0IDE4OjA1IC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
Q2xlYW4gdXA6ICBXaGVuIG5hbWluZyBmaWVsZHMgYW5kIGRhdGEgdHlwZXMsIGZvbGxvdyBlc3Rh
Ymxpc2hlZA0KPiBjb252ZW50aW9ucyB0byBmYWNpbGl0YXRlIGFjY3VyYXRlIGdyZXAvY3Njb3Bl
IHNlYXJjaGVzLg0KPiANCj4gQWRkaXRpb25hbGx5LCBmb3IgY29uc2lzdGVuY3ksIG1vdmUgdGhl
IHNjb3BlIGZpZWxkIGludG8gdGhlIE5GU3Y0LQ0KPiBzcGVjaWZpYyBwYXJ0IG9mIHRoZSBuZnNf
Y2xpZW50LCBhbmQgZnJlZSB0aGF0IG1lbW9yeSBpbiB0aGUgbG9naWMNCj4gdGhhdCBzaHV0cyBk
b3duIE5GU3Y0IG5mc19jbGllbnRzLg0KPiANCj4gSW50cm9kdWNlZCBieSBjb21taXQgOTlmZTYw
ZDAgIm5mczQxOiBleGNoYW5nZV9pZCBvcGVyYXRpb24iLCBBcHJpbA0KPiAxIDIwMDkuDQo+IA0K
PiBTaWduZWQtb2ZmLWJ5OiBDaHVjayBMZXZlciA8Y2h1Y2subGV2ZXJAb3JhY2xlLmNvbT4NCj4g
LS0tDQoNCkRvZXMgbm90IGFwcGx5IHRvIHRoZSBuZnMtZm9yLW5leHQgYnJhbmNoLg0KDQotLSAN
ClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0K
VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==