2012-05-22 02:44:16

by Chuck Lever III

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

Hi-

Reposting UCS pre-requisites series, ported to your nfs-for-next branch
after this merge commit:

commit b3f87b98aa3dc22cc58f970140113b270015cddb
Merge: 041245c 1afeaf5
Author: Trond Myklebust <[email protected]>
Date: Mon May 21 10:12:39 2012 -0400

Merge branch 'bugfixes' into nfs-for-next

While retesting the ported purge-state patch, I noticed that
NFSv4.1's RECLAIM_NOGRACE recovery does not appear to work correctly.
At this point, I think this is an existing problem, and is not
introduced by the purge-state patch itself. I'll keep looking at this
issue and may post a bug-fix or two at some later time.

---

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 | 10 +-
fs/nfs/netns.h | 5 +
fs/nfs/nfs4_fs.h | 8 -
fs/nfs/nfs4filelayoutdev.c | 2
fs/nfs/nfs4proc.c | 76 +++++++++----
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, 220 insertions(+), 160 deletions(-)

--
Signature


2012-05-21 15:19:20

by Chuck Lever III

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


On May 21, 2012, at 11:10 AM, Adamson, Dros wrote:

> On May 18, 2012, at 6:05 PM, Chuck Lever wrote:
>
>> Clean up: prefer using the proper types in "if" expressions.
>
> Is that the preferred style?

Arguable, I suppose. It has been, lately.

> I certainly prefer it (being raised on openbsd style(9)) , but when I wrote the code below I was trying to follow the style of most of fs/nfs.

There is always a question of whether to follow the existing style in a file, or introduce the latest style preferences. Since I'm going to add a "significant" amount of new logic here, I thought I would update the style first. I usually like to have consistent style at least within a particular function.

So, perhaps the description should read "Clean up: update to use matching types in "if" expressions" ?

> -dros

Thanks for the review.

>
>>
>> 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,
>>
>> --
>> 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-22 02:45:44

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 the same
nfs_client_id4 string along 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.
(This is known as the Uniform Client String model).

If the nfs_client_id4 string is the same but the boot verifier changes
for each server IP address, SETCLIENTID and EXCHANGE_ID operations
from such a client could unintentionally result in a server wiping a
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
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 9b9df71..af9b7e4 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -184,7 +184,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];
@@ -1813,6 +1812,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 81ccdbb..9e9334a 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

@@ -3903,8 +3904,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];

@@ -3914,8 +3915,9 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
verf[0] = 0;
verf[1] = (__be32)(NSEC_PER_SEC + 1);
} 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));
}
@@ -3939,7 +3941,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();
@@ -5099,7 +5101,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 db040e9..12b9982 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

@@ -1702,6 +1704,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;

/*
@@ -1743,7 +1746,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-22 02:45: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 af9b7e4..5f19f95 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -506,6 +506,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
*/
@@ -528,8 +557,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;

@@ -538,7 +571,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 */
@@ -555,33 +589,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-22 02:44: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 39db1be..9b9df71 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;
@@ -174,7 +174,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);
@@ -252,7 +252,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);
@@ -305,7 +305,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);

@@ -323,7 +323,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);
@@ -661,7 +661,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,
@@ -715,7 +715,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)
@@ -1060,7 +1060,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);
@@ -1077,7 +1077,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))
@@ -1486,7 +1486,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;
struct nfs_client *clp;
@@ -1709,7 +1709,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 3e8edbe..2eaecf9 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 bf49b78..c610f84 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -629,7 +629,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-22 02:45:35

by Chuck Lever III

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

nfs4_reset_all_state() refreshes the boot verifier a server sees to
trigger that 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.

To facilitate server trunking discovery, we will eventually want to
move the cl_boot_time field to a more global structure. The Uniform
Client String model (and specifically, server trunking detection)
requires that all servers see the same boot verifier until the client
actually does reboot, and not a fresh verifier every time the client
unmounts and remounts the server.

Without the cl_boot_time field, however, nfs4_reset_all_state() will
have to find some other 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 force the server
to wipe NFSv4 state by updating the boot verifier as we do now, 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 | 11 +++++++++--
fs/nfs/nfs4state.c | 13 +++++++++++--
3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 1526fdf..e6da021 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 ab6b2e5..81ccdbb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3908,8 +3908,15 @@ 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)) {
+ /* An impossible timestamp guarantees this value
+ * will never match a generated boot time. */
+ verf[0] = 0;
+ verf[1] = (__be32)(NSEC_PER_SEC + 1);
+ } 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-22 02:45: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 b14bcc3..1526fdf 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-22 02:46:10

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 8a4b3c2..34b2e68 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -131,6 +131,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;
@@ -542,8 +543,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);
@@ -565,9 +565,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);
@@ -651,8 +652,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 = {
@@ -667,9 +667,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))
@@ -809,14 +809,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;

@@ -830,8 +828,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);
@@ -881,10 +878,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);
@@ -1364,15 +1362,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;
@@ -1386,8 +1382,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;

@@ -1455,9 +1451,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;
@@ -1512,7 +1510,7 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
*/
nfs_init_timeout_values(&ds_timeout, ds_proto, ds_timeo, ds_retrans);
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 3a9e80c..547f24f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -240,8 +240,7 @@ extern int nfs4_init_ds_session(struct nfs_client *clp);
void nfs_close_context(struct nfs_open_context *ctx, int is_sync);
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);
+ const char *ip_addr, rpc_authflavor_t authflavour);

/* dir.c */
extern int nfs_access_cache_shrinker(struct shrinker *shrink,
@@ -376,8 +375,7 @@ extern void __nfs4_read_done_cb(struct nfs_read_data *);
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 int _nfs4_call_sync(struct rpc_clnt *clnt,
struct nfs_server *server,
struct rpc_message *msg,
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 0c521cd..07048c0 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1399,7 +1399,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);
};

/*


2012-05-21 16:50:02

by Myklebust, Trond

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

T24gTW9uLCAyMDEyLTA1LTIxIGF0IDEyOjI0IC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
T24gTWF5IDIxLCAyMDEyLCBhdCAxMjoxNiBQTSwgSi4gQnJ1Y2UgRmllbGRzIHdyb3RlOg0KPiAN
Cj4gPiBPbiBNb24sIE1heSAyMSwgMjAxMiBhdCAxMjoxMTo0OFBNIC0wNDAwLCBDaHVjayBMZXZl
ciB3cm90ZToNCj4gPj4gDQo+ID4+IE9uIE1heSAyMSwgMjAxMiwgYXQgMTI6MDggUE0sIEouIEJy
dWNlIEZpZWxkcyB3cm90ZToNCj4gPj4gDQo+ID4+PiBPbiBGcmksIE1heSAxOCwgMjAxMiBhdCAw
NjowNjozM1BNIC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4gPj4+PiBAQCAtMzkzNyw4ICsz
OTM3LDEzIEBAIHN0YXRpYyB2b2lkIG5mczRfY29uc3RydWN0X2Jvb3RfdmVyaWZpZXIoc3RydWN0
IG5mc19jbGllbnQgKmNscCwNCj4gPj4+PiB7DQo+ID4+Pj4gCV9fYmUzMiB2ZXJmWzJdOw0KPiA+
Pj4+IA0KPiA+Pj4+IC0JdmVyZlswXSA9IChfX2JlMzIpY2xwLT5jbF9ib290X3RpbWUudHZfc2Vj
Ow0KPiA+Pj4+IC0JdmVyZlsxXSA9IChfX2JlMzIpY2xwLT5jbF9ib290X3RpbWUudHZfbnNlYzsN
Cj4gPj4+PiArCWlmICh0ZXN0X2JpdChORlM0Q0xOVF9QVVJHRV9TVEFURSwgJmNscC0+Y2xfc3Rh
dGUpKSB7DQo+ID4+Pj4gKwkJdmVyZlswXSA9IChfX2JlMzIpQ1VSUkVOVF9USU1FLnR2X3NlYzsN
Cj4gPj4+PiArCQl2ZXJmWzFdID0gKF9fYmUzMilDVVJSRU5UX1RJTUUudHZfbnNlYzsNCj4gPj4+
IA0KPiA+Pj4gSSBzdXBwb3NlIGl0J3MgcHJldHR5IHVubGlrZWx5IHRoaXMgY291bGQgaGFwcGVu
IHdpdGhpbiBhIGppZmZ5IG9mDQo+ID4+PiBzZXR0aW5nIGNsX2Jvb3RfdGltZT8NCj4gPj4gDQo+
ID4+IEJvb3QgdGltZSBoYXMgbmFub3NlY29uZCByZXNvbHV0aW9uLiAgSSdtIHByZXR0eSBzdXJl
IHdlIGFyZSBzYWZlIGhlcmUuDQo+ID4gDQo+ID4gQ1VSUkVOVF9USU1FIGlzIG9ubHkgdXBkYXRl
ZCBldmVyeSBqaWZmeS4gIEl0IHdvdWxkIHN0aWxsIGhhdmUgdG8gaGFwcGVuDQo+ID4gcmlkaWN1
bG91c2x5IGZhc3QsIGJ1dCBJIHRoaW5rIHRoYXQncyBtaWxsaXNlY29uZHMgcmF0aGVyIHRoYW4N
Cj4gPiBuYW5vc2Vjb25kcy4NCj4gPiANCj4gPj4+IFdvdWxkIGl0IGJlIHNpbXBsZXIgdG8gdXNl
IHNvbWUgc3BlY2lhbCB2YWx1ZSAoYWxsLXplcm9zPykgaW5zdGVhZD8NCj4gPj4gDQo+ID4+IFdl
J2QgaGF2ZSB0byBwaWNrIGEgdmFsdWUgdGhhdCB3YXMgZ3VhcmFudGVlZCBuZXZlciB0byBiZSBh
IHZhbGlkIHRpbWUgc3RhbXAuDQo+ID4gDQo+ID4gQSBjbGllbnQgdGhhdCB0aGlua3MgaXQncyBj
dXJyZW50bHkgSmFuLiAxIDE5NzAgcHJvYmFibHkgaGFzIGJpZ2dlcg0KPiA+IHByb2JsZW1zLg0K
PiA+IA0KPiA+IEkgZHVubm8sIHlvdXIgY2FsbC4NCj4gDQo+IFVzaW5nIGFuIG91dC1vZi1kYXRl
IHRpbWVzdGFtcCAoc3VjaCBhcyBhbGwgemVyb2VzKSBjb3VsZCB3b3JrLiAgV2UnZCBqdXN0IGhh
dmUgdG8gYnVpbGQgYW4gYXBwcm9wcmlhdGUgY29uc3RhbnQuDQoNCkJldHRlciB5ZXQ6IHVzZSBh
biBpbnZhbGlkIHRpbWVzdGFtcC4gQW55dGhpbmcgd2l0aCBhIHR2X25zZWMgPj0NCjEwMDAwMDAw
MDBMIHdvdWxkIHdvcmsuDQoNCj4gVHJvbmQsIGRvIHlvdSByZW1lbWJlciB3aHkgeW91IGxpa2Vk
IENVUlJFTlRfVElNRT8NCg0KSSBkb24ndCB0aGluayB0aGF0IEkgd2FzIHRoZSBvbmUgd2hvIGNo
b3NlIENVUlJFTlRfVElNRSBmb3IgdGhlDQpwdXJnZV9zdGF0ZS4gSSdtIGZpbmUgd2l0aCBhbnl0
aGluZyB0aGF0IGlzIGd1YXJhbnRlZWQgdG8gYWx3YXlzIGRpZmZlcg0KZnJvbSBjbHAtPmNsX2Jv
b3RfdGltZS4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRh
aW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNv
bQ0KDQo=

2012-05-21 16:24:54

by Chuck Lever III

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


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

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

Using an out-of-date timestamp (such as all zeroes) could work. We'd just have to build an appropriate constant.

Trond, do you remember why you liked CURRENT_TIME?

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





2012-05-22 02:44:25

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-22 02:45:01

by Chuck Lever III

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

Clean up: update to use matching 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 e182134..0b9b55c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5102,30 +5102,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",
@@ -5135,7 +5135,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;
}
@@ -5144,7 +5144,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-22 02:44:43

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 471fc9b..39db1be 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -238,6 +238,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 */
@@ -306,7 +307,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 2c1e57e..e182134 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5119,8 +5119,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);

@@ -5144,12 +5144,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 a973eb1..ff656c0 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -796,8 +796,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-22 02:45: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 0b9b55c..ab6b2e5 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3908,8 +3908,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:10:44

by Adamson, Dros

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

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

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

Is that the preferred style? I certainly prefer it (being raised on openbsd style(9)) , but when I wrote the code below I was trying to follow the style of most of fs/nfs.

-dros

>
> 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,
>
> --
> 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-22 02:44:34

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 b4e2199..471fc9b 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -237,6 +237,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 */
@@ -305,7 +306,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 edeef71..b14bcc3 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -338,7 +338,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 78784e5..2c1e57e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5052,7 +5052,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)
@@ -5099,7 +5100,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;
@@ -5123,18 +5125,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 2e53a3f..c420b8d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1104,7 +1104,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];
};
@@ -1118,7 +1118,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-22 02:46:20

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 34b2e68..3c14468 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -237,6 +237,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 9e9334a..0d46fe4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5109,11 +5109,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);
@@ -5127,6 +5134,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;
@@ -5150,6 +5163,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 12b9982..5ad2b2c 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -5144,24 +5144,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 07048c0..0872f32 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1098,7 +1098,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];
@@ -1118,6 +1118,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-22 02:45: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 6cc7dba..80a9385 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -634,6 +634,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-22 02:46: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 | 4 +-
include/linux/nfs_xdr.h | 3 +-
3 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 5f19f95..8a4b3c2 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -546,7 +546,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",
@@ -563,8 +562,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);

@@ -574,21 +578,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;
}

/*
@@ -813,10 +802,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)
{
@@ -825,7 +823,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;
}

/*
@@ -837,12 +835,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);
}

/*
@@ -1358,14 +1357,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;
@@ -1373,7 +1380,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 */
@@ -1413,12 +1420,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 989959a..3a9e80c 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -238,7 +238,7 @@ 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,
+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);
@@ -373,7 +373,7 @@ void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,

/* nfs4proc.c */
extern void __nfs4_read_done_cb(struct nfs_read_data *);
-extern int nfs4_init_client(struct nfs_client *clp,
+extern struct nfs_client *nfs4_init_client(struct nfs_client *clp,
const struct rpc_timeout *timeparms,
const char *ip_addr,
rpc_authflavor_t authflavour,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index c420b8d..0c521cd 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1397,7 +1397,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);
};