2012-05-25 22:03:41

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 1/8] NFSv4.1: Exchange ID must use GFP_NOFS allocation mode

Exchange ID can be called in a lease reclaim situation, so it
will deadlock if it then tries to write out dirty NFS pages.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e8988c0..f8817e8 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5192,20 +5192,20 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
clp->cl_rpcclient->cl_auth->au_flavor);

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

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

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



2012-05-25 22:03:42

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 4/8] NFSv4.1: Handle NFS4ERR_SEQ_MISORDERED when confirming the lease

Apparently the patch "NFS: Always use the same SETCLIENTID boot verifier"
is tickling a Linux nfs server bug, and causing a regression: the server
can get into a situation where it keeps replying NFS4ERR_SEQ_MISORDERED
to our CREATE_SESSION request even when we are sending the correct
sequence ID.

Fix this by purging the lease and then retrying.

Reported-by: Bryan Schumaker <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4state.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 604c600..419f8c4 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1580,6 +1580,11 @@ out:
static int nfs4_handle_reclaim_lease_error(struct nfs_client *clp, int status)
{
switch (status) {
+ case -NFS4ERR_SEQ_MISORDERED:
+ if (test_and_set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state))
+ return -ESERVERFAULT;
+ /* Lease confirmation error: retry after purging the lease */
+ ssleep(1);
case -NFS4ERR_CLID_INUSE:
case -NFS4ERR_STALE_CLIENTID:
clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
--
1.7.7.6


2012-05-25 22:03:43

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 6/8] NFSv4.1: Ensure we use the correct credentials for session create/destroy

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 4 ++--
fs/nfs/nfs4proc.c | 27 +++++++++++++++++----------
fs/nfs/nfs4state.c | 10 +++++++---
3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 5fcb1ad..a5dbe62 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -241,8 +241,8 @@ extern int nfs41_setup_sequence(struct nfs4_session *session,
struct rpc_task *task);
extern void nfs4_destroy_session(struct nfs4_session *session);
extern struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp);
-extern int nfs4_proc_create_session(struct nfs_client *);
-extern int nfs4_proc_destroy_session(struct nfs4_session *);
+extern int nfs4_proc_create_session(struct nfs_client *, struct rpc_cred *);
+extern int nfs4_proc_destroy_session(struct nfs4_session *, struct rpc_cred *);
extern int nfs4_init_session(struct nfs_server *server);
extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
struct nfs_fsinfo *fsinfo);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f8817e8..8fa3a36 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5480,8 +5480,12 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
void nfs4_destroy_session(struct nfs4_session *session)
{
struct rpc_xprt *xprt;
+ struct rpc_cred *cred;

- nfs4_proc_destroy_session(session);
+ cred = nfs4_get_exchange_id_cred(session->clp);
+ nfs4_proc_destroy_session(session, cred);
+ if (cred)
+ put_rpccred(cred);

rcu_read_lock();
xprt = rcu_dereference(session->clp->cl_rpcclient->cl_xprt);
@@ -5591,7 +5595,8 @@ static int nfs4_verify_channel_attrs(struct nfs41_create_session_args *args,
return nfs4_verify_back_channel_attrs(args, session);
}

-static int _nfs4_proc_create_session(struct nfs_client *clp)
+static int _nfs4_proc_create_session(struct nfs_client *clp,
+ struct rpc_cred *cred)
{
struct nfs4_session *session = clp->cl_session;
struct nfs41_create_session_args args = {
@@ -5605,6 +5610,7 @@ static int _nfs4_proc_create_session(struct nfs_client *clp)
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_CREATE_SESSION],
.rpc_argp = &args,
.rpc_resp = &res,
+ .rpc_cred = cred,
};
int status;

@@ -5629,7 +5635,7 @@ static int _nfs4_proc_create_session(struct nfs_client *clp)
* It is the responsibility of the caller to verify the session is
* expired before calling this routine.
*/
-int nfs4_proc_create_session(struct nfs_client *clp)
+int nfs4_proc_create_session(struct nfs_client *clp, struct rpc_cred *cred)
{
int status;
unsigned *ptr;
@@ -5637,7 +5643,7 @@ int nfs4_proc_create_session(struct nfs_client *clp)

dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);

- status = _nfs4_proc_create_session(clp);
+ status = _nfs4_proc_create_session(clp, cred);
if (status)
goto out;

@@ -5659,10 +5665,15 @@ out:
* Issue the over-the-wire RPC DESTROY_SESSION.
* The caller must serialize access to this routine.
*/
-int nfs4_proc_destroy_session(struct nfs4_session *session)
+int nfs4_proc_destroy_session(struct nfs4_session *session,
+ struct rpc_cred *cred)
{
+ struct rpc_message msg = {
+ .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_DESTROY_SESSION],
+ .rpc_argp = session,
+ .rpc_cred = cred,
+ };
int status = 0;
- struct rpc_message msg;

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

@@ -5670,10 +5681,6 @@ int nfs4_proc_destroy_session(struct nfs4_session *session)
if (session->clp->cl_cons_state != NFS_CS_READY)
return status;

- msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_DESTROY_SESSION];
- msg.rpc_argp = session;
- msg.rpc_resp = NULL;
- msg.rpc_cred = NULL;
status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);

if (status)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 419f8c4..1587840 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -256,7 +256,7 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
goto out;
set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
do_confirm:
- status = nfs4_proc_create_session(clp);
+ status = nfs4_proc_create_session(clp, cred);
if (status != 0)
goto out;
clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
@@ -1717,10 +1717,12 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)

static int nfs4_reset_session(struct nfs_client *clp)
{
+ struct rpc_cred *cred;
int status;

nfs4_begin_drain_session(clp);
- status = nfs4_proc_destroy_session(clp->cl_session);
+ cred = nfs4_get_exchange_id_cred(clp);
+ status = nfs4_proc_destroy_session(clp->cl_session, cred);
if (status && status != -NFS4ERR_BADSESSION &&
status != -NFS4ERR_DEADSESSION) {
status = nfs4_recovery_handle_error(clp, status);
@@ -1728,7 +1730,7 @@ static int nfs4_reset_session(struct nfs_client *clp)
}

memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIONID_LEN);
- status = nfs4_proc_create_session(clp);
+ status = nfs4_proc_create_session(clp, cred);
if (status) {
status = nfs4_recovery_handle_error(clp, status);
goto out;
@@ -1742,6 +1744,8 @@ static int nfs4_reset_session(struct nfs_client *clp)
if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
nfs41_setup_state_renewal(clp);
out:
+ if (cred)
+ put_rpccred(cred);
return status;
}

--
1.7.7.6


2012-05-25 22:03:42

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 3/8] NFSv4: When purging the lease, we must clear NFS4CLNT_LEASE_CONFIRM

Otherwise we can end up not sending a new exchange-id/setclientid

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4state.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 758b9a8..604c600 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1647,6 +1647,7 @@ static void nfs4_reset_all_state(struct nfs_client *clp)
{
if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
+ clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
nfs4_state_start_reclaim_nograce(clp);
nfs4_schedule_state_manager(clp);
}
--
1.7.7.6


2012-05-29 15:57:18

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 5/8] NFSv4.1: Move NFSPROC4_CLNT_BIND_CONN_TO_SESSION to the end of the operations


On May 29, 2012, at 11:43 AM, Myklebust, Trond wrote:

> On Tue, 2012-05-29 at 15:05 +0000, Adamson, Dros wrote:
>> On May 25, 2012, at 6:03 PM, Trond Myklebust wrote:
>>
>>> For backward compatibility with nfs-utils.
>>
>> Can you expand on that a bit? I put them where they are because they are in order of operation id (bind_conn_to_session = 41, exchange_id = 42).
>
> nfs4_procedures isn't (and really can't be) ordered by operation. It is
> a set of COMPOUNDs, each of which reflects a certain functionality
> rather than a specific operation.

Oh I see.

>
>> If we really must only add to the end of the list, we really should have a comment above nfs4_procedures, etc.
>
> The whole nfsstat is currently problematic because we occasionally do
> want to be able to add to the NFSv4.0 procedures (above the #ifdef
> CONFIG_NFS_V4_1) as well as the NFSv4.1 procedures.
>
> Appending is usually the right thing to do, but that #ifdef makes it
> problematic since if we were to append NFSv4.0 routines, then they would
> move depending on whether or not we compile in NFSv4.1 or not.
>
> Adding in NFSv4.2 will give rise to even more 'interesting' situations
> should we start protecting it with yet more #ifdefs...
>
> The solution may be to just accept that we cannot keep separating
> NFSv4.0 and NFSv4.1 routines, and to just get rid of the #ifdefs so that
> we can always append to nfs4_procedures[]. However we haven't yet done
> this, and so there is no good 'official' documented policy.

Maybe a comment with a note about how fragile this is and a reminder to test nfsstat?

-dros


Attachments:
smime.p7s (1.34 kB)

2012-05-29 15:43:41

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 5/8] NFSv4.1: Move NFSPROC4_CLNT_BIND_CONN_TO_SESSION to the end of the operations

T24gVHVlLCAyMDEyLTA1LTI5IGF0IDE1OjA1ICswMDAwLCBBZGFtc29uLCBEcm9zIHdyb3RlOg0K
PiBPbiBNYXkgMjUsIDIwMTIsIGF0IDY6MDMgUE0sIFRyb25kIE15a2xlYnVzdCB3cm90ZToNCj4g
DQo+ID4gRm9yIGJhY2t3YXJkIGNvbXBhdGliaWxpdHkgd2l0aCBuZnMtdXRpbHMuDQo+IA0KPiBD
YW4geW91IGV4cGFuZCBvbiB0aGF0IGEgYml0PyBJIHB1dCB0aGVtIHdoZXJlIHRoZXkgYXJlIGJl
Y2F1c2UgdGhleSBhcmUgaW4gb3JkZXIgb2Ygb3BlcmF0aW9uIGlkIChiaW5kX2Nvbm5fdG9fc2Vz
c2lvbiA9IDQxLCBleGNoYW5nZV9pZCA9IDQyKS4NCg0KbmZzNF9wcm9jZWR1cmVzIGlzbid0IChh
bmQgcmVhbGx5IGNhbid0IGJlKSBvcmRlcmVkIGJ5IG9wZXJhdGlvbi4gSXQgaXMNCmEgc2V0IG9m
IENPTVBPVU5EcywgZWFjaCBvZiB3aGljaCByZWZsZWN0cyBhIGNlcnRhaW4gZnVuY3Rpb25hbGl0
eQ0KcmF0aGVyIHRoYW4gYSBzcGVjaWZpYyBvcGVyYXRpb24uDQoNCj4gSWYgd2UgcmVhbGx5IG11
c3Qgb25seSBhZGQgdG8gdGhlIGVuZCBvZiB0aGUgbGlzdCwgd2UgcmVhbGx5IHNob3VsZCBoYXZl
IGEgY29tbWVudCBhYm92ZSBuZnM0X3Byb2NlZHVyZXMsIGV0Yy4NCg0KVGhlIHdob2xlIG5mc3N0
YXQgaXMgY3VycmVudGx5IHByb2JsZW1hdGljIGJlY2F1c2Ugd2Ugb2NjYXNpb25hbGx5IGRvDQp3
YW50IHRvIGJlIGFibGUgdG8gYWRkIHRvIHRoZSBORlN2NC4wIHByb2NlZHVyZXMgKGFib3ZlIHRo
ZSAjaWZkZWYNCkNPTkZJR19ORlNfVjRfMSkgYXMgd2VsbCBhcyB0aGUgTkZTdjQuMSBwcm9jZWR1
cmVzLg0KDQpBcHBlbmRpbmcgaXMgdXN1YWxseSB0aGUgcmlnaHQgdGhpbmcgdG8gZG8sIGJ1dCB0
aGF0ICNpZmRlZiBtYWtlcyBpdA0KcHJvYmxlbWF0aWMgc2luY2UgaWYgd2Ugd2VyZSB0byBhcHBl
bmQgTkZTdjQuMCByb3V0aW5lcywgdGhlbiB0aGV5IHdvdWxkDQptb3ZlIGRlcGVuZGluZyBvbiB3
aGV0aGVyIG9yIG5vdCB3ZSBjb21waWxlIGluIE5GU3Y0LjEgb3Igbm90Lg0KDQpBZGRpbmcgaW4g
TkZTdjQuMiB3aWxsIGdpdmUgcmlzZSB0byBldmVuIG1vcmUgJ2ludGVyZXN0aW5nJyBzaXR1YXRp
b25zDQpzaG91bGQgd2Ugc3RhcnQgcHJvdGVjdGluZyBpdCB3aXRoIHlldCBtb3JlICNpZmRlZnMu
Li4NCg0KVGhlIHNvbHV0aW9uIG1heSBiZSB0byBqdXN0IGFjY2VwdCB0aGF0IHdlIGNhbm5vdCBr
ZWVwIHNlcGFyYXRpbmcNCk5GU3Y0LjAgYW5kIE5GU3Y0LjEgcm91dGluZXMsIGFuZCB0byBqdXN0
IGdldCByaWQgb2YgdGhlICNpZmRlZnMgc28gdGhhdA0Kd2UgY2FuIGFsd2F5cyBhcHBlbmQgdG8g
bmZzNF9wcm9jZWR1cmVzW10uIEhvd2V2ZXIgd2UgaGF2ZW4ndCB5ZXQgZG9uZQ0KdGhpcywgYW5k
IHNvIHRoZXJlIGlzIG5vIGdvb2QgJ29mZmljaWFsJyBkb2N1bWVudGVkIHBvbGljeS4NCg0KLS0g
DQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHAN
ClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=

2012-05-29 15:04:39

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 7/8] NFSv4.1: Ensure we use the correct credentials for bind_conn_to_session

This looks good.

-dros

On May 25, 2012, at 6:03 PM, Trond Myklebust wrote:

> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4_fs.h | 2 +-
> fs/nfs/nfs4proc.c | 3 ++-
> fs/nfs/nfs4state.c | 9 ++++++++-
> 3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index a5dbe62..f730730 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -212,7 +212,7 @@ struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
> extern int nfs4_proc_setclientid(struct nfs_client *, u32, unsigned short, struct rpc_cred *, struct nfs4_setclientid_res *);
> extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setclientid_res *arg, struct rpc_cred *);
> extern int nfs4_proc_get_rootfh(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
> -extern int nfs4_proc_bind_conn_to_session(struct nfs_client *);
> +extern int nfs4_proc_bind_conn_to_session(struct nfs_client *, struct rpc_cred *cred);
> extern int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred);
> extern int nfs4_init_clientid(struct nfs_client *, struct rpc_cred *);
> extern int nfs41_init_clientid(struct nfs_client *, struct rpc_cred *);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 8fa3a36..3fdff0c 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5105,7 +5105,7 @@ nfs41_same_server_scope(struct nfs41_server_scope *a,
> * The 4.1 client currently uses the same TCP connection for the
> * fore and backchannel.
> */
> -int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
> +int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred)
> {
> int status;
> struct nfs41_bind_conn_to_session_res res;
> @@ -5114,6 +5114,7 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
> &nfs4_procedures[NFSPROC4_CLNT_BIND_CONN_TO_SESSION],
> .rpc_argp = clp,
> .rpc_resp = &res,
> + .rpc_cred = cred,
> };
>
> dprintk("--> %s\n", __func__);
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 1587840..7dbca66 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1779,7 +1779,14 @@ static int nfs4_recall_slot(struct nfs_client *clp)
>
> static int nfs4_bind_conn_to_session(struct nfs_client *clp)
> {
> - return nfs4_proc_bind_conn_to_session(clp);
> + struct rpc_cred *cred;
> + int ret;
> +
> + cred = nfs4_get_exchange_id_cred(clp);
> + ret = nfs4_proc_bind_conn_to_session(clp, cred);
> + if (cred)
> + put_rpccred(cred);
> + return ret;
> }
> #else /* CONFIG_NFS_V4_1 */
> static int nfs4_reset_session(struct nfs_client *clp) { return 0; }
> --
> 1.7.7.6
>


Attachments:
smime.p7s (1.34 kB)

2012-05-25 22:03:43

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 5/8] NFSv4.1: Move NFSPROC4_CLNT_BIND_CONN_TO_SESSION to the end of the operations

For backward compatibility with nfs-utils.

Signed-off-by: Trond Myklebust <[email protected]>
Cc: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4xdr.c | 4 ++--
include/linux/nfs4.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index a6b95b7..1d4d259 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -7221,8 +7221,6 @@ struct rpc_procinfo nfs4_procedures[] = {
PROC(RELEASE_LOCKOWNER, enc_release_lockowner, dec_release_lockowner),
PROC(SECINFO, enc_secinfo, dec_secinfo),
#if defined(CONFIG_NFS_V4_1)
- PROC(BIND_CONN_TO_SESSION,
- enc_bind_conn_to_session, dec_bind_conn_to_session),
PROC(EXCHANGE_ID, enc_exchange_id, dec_exchange_id),
PROC(CREATE_SESSION, enc_create_session, dec_create_session),
PROC(DESTROY_SESSION, enc_destroy_session, dec_destroy_session),
@@ -7237,6 +7235,8 @@ struct rpc_procinfo nfs4_procedures[] = {
PROC(TEST_STATEID, enc_test_stateid, dec_test_stateid),
PROC(FREE_STATEID, enc_free_stateid, dec_free_stateid),
PROC(GETDEVICELIST, enc_getdevicelist, dec_getdevicelist),
+ PROC(BIND_CONN_TO_SESSION,
+ enc_bind_conn_to_session, dec_bind_conn_to_session),
#endif /* CONFIG_NFS_V4_1 */
};

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index a2b71cb..54006a9 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -593,7 +593,6 @@ enum {
NFSPROC4_CLNT_SECINFO,

/* nfs41 */
- NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
NFSPROC4_CLNT_EXCHANGE_ID,
NFSPROC4_CLNT_CREATE_SESSION,
NFSPROC4_CLNT_DESTROY_SESSION,
@@ -608,6 +607,7 @@ enum {
NFSPROC4_CLNT_TEST_STATEID,
NFSPROC4_CLNT_FREE_STATEID,
NFSPROC4_CLNT_GETDEVICELIST,
+ NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
};

/* nfs41 types */
--
1.7.7.6


2012-05-25 22:03:44

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 8/8] NFSv4.1: Add DESTROY_CLIENTID

Ensure that we destroy our lease on last unmount

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/client.c | 1 +
fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4proc.c | 32 ++++++++++++++++++++++++++++++
fs/nfs/nfs4xdr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/nfs4.h | 1 +
5 files changed, 87 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index a50bdfb..7d10875 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -209,6 +209,7 @@ static void nfs4_shutdown_session(struct nfs_client *clp)
if (nfs4_has_session(clp)) {
nfs4_deviceid_purge_client(clp);
nfs4_destroy_session(clp->cl_session);
+ nfs4_destroy_clientid(clp);
}

}
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index f730730..b20b516 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -214,6 +214,7 @@ extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setcli
extern int nfs4_proc_get_rootfh(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
extern int nfs4_proc_bind_conn_to_session(struct nfs_client *, struct rpc_cred *cred);
extern int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred);
+extern int nfs4_destroy_clientid(struct nfs_client *clp);
extern int nfs4_init_clientid(struct nfs_client *, struct rpc_cred *);
extern int nfs41_init_clientid(struct nfs_client *, struct rpc_cred *);
extern int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait, bool roc);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3fdff0c..0415825 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5261,6 +5261,38 @@ out:
return status;
}

+static int nfs4_proc_destroy_clientid(struct nfs_client *clp,
+ struct rpc_cred *cred)
+{
+ struct rpc_message msg = {
+ .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_DESTROY_CLIENTID],
+ .rpc_argp = clp,
+ .rpc_cred = cred,
+ };
+ int status;
+
+ status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+ if (status)
+ pr_warn("NFS: Got error %d from the server %s on "
+ "DESTROY_CLIENTID.", status, clp->cl_hostname);
+ return status;
+}
+
+int nfs4_destroy_clientid(struct nfs_client *clp)
+{
+ struct rpc_cred *cred;
+ int ret = 0;
+
+ if (clp->cl_mvops->minor_version < 1)
+ goto out;
+ cred = nfs4_get_exchange_id_cred(clp);
+ ret = nfs4_proc_destroy_clientid(clp, cred);
+ if (cred)
+ put_rpccred(cred);
+out:
+ return ret;
+}
+
struct nfs4_get_lease_time_data {
struct nfs4_get_lease_time_args *args;
struct nfs4_get_lease_time_res *res;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 1d4d259..b9ce3fd 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -338,6 +338,8 @@ static int nfs4_stat_to_errno(int);
1 /* bctsr_use_conn_in_rdma_mode */)
#define encode_destroy_session_maxsz (op_encode_hdr_maxsz + 4)
#define decode_destroy_session_maxsz (op_decode_hdr_maxsz)
+#define encode_destroy_clientid_maxsz (op_encode_hdr_maxsz + 2)
+#define decode_destroy_clientid_maxsz (op_decode_hdr_maxsz)
#define encode_sequence_maxsz (op_encode_hdr_maxsz + \
XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + 4)
#define decode_sequence_maxsz (op_decode_hdr_maxsz + \
@@ -751,6 +753,10 @@ static int nfs4_stat_to_errno(int);
encode_destroy_session_maxsz)
#define NFS4_dec_destroy_session_sz (compound_decode_hdr_maxsz + \
decode_destroy_session_maxsz)
+#define NFS4_enc_destroy_clientid_sz (compound_encode_hdr_maxsz + \
+ encode_destroy_clientid_maxsz)
+#define NFS4_dec_destroy_clientid_sz (compound_decode_hdr_maxsz + \
+ decode_destroy_clientid_maxsz)
#define NFS4_enc_sequence_sz \
(compound_decode_hdr_maxsz + \
encode_sequence_maxsz)
@@ -1804,6 +1810,14 @@ static void encode_destroy_session(struct xdr_stream *xdr,
encode_opaque_fixed(xdr, session->sess_id.data, NFS4_MAX_SESSIONID_LEN);
}

+static void encode_destroy_clientid(struct xdr_stream *xdr,
+ uint64_t clientid,
+ struct compound_hdr *hdr)
+{
+ encode_op_hdr(xdr, OP_DESTROY_CLIENTID, decode_destroy_clientid_maxsz, hdr);
+ encode_uint64(xdr, clientid);
+}
+
static void encode_reclaim_complete(struct xdr_stream *xdr,
struct nfs41_reclaim_complete_args *args,
struct compound_hdr *hdr)
@@ -2724,6 +2738,22 @@ static void nfs4_xdr_enc_destroy_session(struct rpc_rqst *req,
}

/*
+ * a DESTROY_CLIENTID request
+ */
+static void nfs4_xdr_enc_destroy_clientid(struct rpc_rqst *req,
+ struct xdr_stream *xdr,
+ struct nfs_client *clp)
+{
+ struct compound_hdr hdr = {
+ .minorversion = clp->cl_mvops->minor_version,
+ };
+
+ encode_compound_hdr(xdr, req, &hdr);
+ encode_destroy_clientid(xdr, clp->cl_clientid, &hdr);
+ encode_nops(&hdr);
+}
+
+/*
* a SEQUENCE request
*/
static void nfs4_xdr_enc_sequence(struct rpc_rqst *req, struct xdr_stream *xdr,
@@ -5479,6 +5509,11 @@ static int decode_destroy_session(struct xdr_stream *xdr, void *dummy)
return decode_op_hdr(xdr, OP_DESTROY_SESSION);
}

+static int decode_destroy_clientid(struct xdr_stream *xdr, void *dummy)
+{
+ return decode_op_hdr(xdr, OP_DESTROY_CLIENTID);
+}
+
static int decode_reclaim_complete(struct xdr_stream *xdr, void *dummy)
{
return decode_op_hdr(xdr, OP_RECLAIM_COMPLETE);
@@ -6789,6 +6824,22 @@ static int nfs4_xdr_dec_destroy_session(struct rpc_rqst *rqstp,
}

/*
+ * Decode DESTROY_CLIENTID response
+ */
+static int nfs4_xdr_dec_destroy_clientid(struct rpc_rqst *rqstp,
+ struct xdr_stream *xdr,
+ void *res)
+{
+ struct compound_hdr hdr;
+ int status;
+
+ status = decode_compound_hdr(xdr, &hdr);
+ if (!status)
+ status = decode_destroy_clientid(xdr, res);
+ return status;
+}
+
+/*
* Decode SEQUENCE response
*/
static int nfs4_xdr_dec_sequence(struct rpc_rqst *rqstp,
@@ -7237,6 +7288,7 @@ struct rpc_procinfo nfs4_procedures[] = {
PROC(GETDEVICELIST, enc_getdevicelist, dec_getdevicelist),
PROC(BIND_CONN_TO_SESSION,
enc_bind_conn_to_session, dec_bind_conn_to_session),
+ PROC(DESTROY_CLIENTID, enc_destroy_clientid, dec_destroy_clientid),
#endif /* CONFIG_NFS_V4_1 */
};

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 54006a9..af2d2fa 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -608,6 +608,7 @@ enum {
NFSPROC4_CLNT_FREE_STATEID,
NFSPROC4_CLNT_GETDEVICELIST,
NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
+ NFSPROC4_CLNT_DESTROY_CLIENTID,
};

/* nfs41 types */
--
1.7.7.6


2012-05-29 15:05:55

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH 5/8] NFSv4.1: Move NFSPROC4_CLNT_BIND_CONN_TO_SESSION to the end of the operations

On May 25, 2012, at 6:03 PM, Trond Myklebust wrote:

> For backward compatibility with nfs-utils.

Can you expand on that a bit? I put them where they are because they are in order of operation id (bind_conn_to_session = 41, exchange_id = 42).

If we really must only add to the end of the list, we really should have a comment above nfs4_procedures, etc.

-dros

>
> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4xdr.c | 4 ++--
> include/linux/nfs4.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index a6b95b7..1d4d259 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -7221,8 +7221,6 @@ struct rpc_procinfo nfs4_procedures[] = {
> PROC(RELEASE_LOCKOWNER, enc_release_lockowner, dec_release_lockowner),
> PROC(SECINFO, enc_secinfo, dec_secinfo),
> #if defined(CONFIG_NFS_V4_1)
> - PROC(BIND_CONN_TO_SESSION,
> - enc_bind_conn_to_session, dec_bind_conn_to_session),
> PROC(EXCHANGE_ID, enc_exchange_id, dec_exchange_id),
> PROC(CREATE_SESSION, enc_create_session, dec_create_session),
> PROC(DESTROY_SESSION, enc_destroy_session, dec_destroy_session),
> @@ -7237,6 +7235,8 @@ struct rpc_procinfo nfs4_procedures[] = {
> PROC(TEST_STATEID, enc_test_stateid, dec_test_stateid),
> PROC(FREE_STATEID, enc_free_stateid, dec_free_stateid),
> PROC(GETDEVICELIST, enc_getdevicelist, dec_getdevicelist),
> + PROC(BIND_CONN_TO_SESSION,
> + enc_bind_conn_to_session, dec_bind_conn_to_session),
> #endif /* CONFIG_NFS_V4_1 */
> };
>
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index a2b71cb..54006a9 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -593,7 +593,6 @@ enum {
> NFSPROC4_CLNT_SECINFO,
>
> /* nfs41 */
> - NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
> NFSPROC4_CLNT_EXCHANGE_ID,
> NFSPROC4_CLNT_CREATE_SESSION,
> NFSPROC4_CLNT_DESTROY_SESSION,
> @@ -608,6 +607,7 @@ enum {
> NFSPROC4_CLNT_TEST_STATEID,
> NFSPROC4_CLNT_FREE_STATEID,
> NFSPROC4_CLNT_GETDEVICELIST,
> + NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
> };
>
> /* nfs41 types */
> --
> 1.7.7.6
>


Attachments:
smime.p7s (1.34 kB)

2012-05-25 22:03:42

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 2/8] NFSv4: Clean up the error handling for nfs4_reclaim_lease

Try to consolidate the error handling for nfs4_reclaim_lease into
a single function instead of doing a bit here, and a bit there...

Also ensure that NFS4CLNT_PURGE_STATE handles errors correctly.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4state.c | 99 ++++++++++++++++++++++++++-------------------------
1 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 03fa802..758b9a8 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1574,26 +1574,57 @@ out:
return nfs4_recovery_handle_error(clp, status);
}

+/* Set NFS4CLNT_LEASE_EXPIRED for all v4.0 errors and for recoverable errors
+ * on EXCHANGE_ID for v4.1
+ */
+static int nfs4_handle_reclaim_lease_error(struct nfs_client *clp, int status)
+{
+ switch (status) {
+ case -NFS4ERR_CLID_INUSE:
+ case -NFS4ERR_STALE_CLIENTID:
+ clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
+ break;
+ case -EACCES:
+ if (clp->cl_machine_cred == NULL)
+ return -EACCES;
+ /* Handle case where the user hasn't set up machine creds */
+ nfs4_clear_machine_cred(clp);
+ case -NFS4ERR_DELAY:
+ case -ETIMEDOUT:
+ case -EAGAIN:
+ ssleep(1);
+ break;
+
+ case -NFS4ERR_MINOR_VERS_MISMATCH:
+ if (clp->cl_cons_state == NFS_CS_SESSION_INITING)
+ nfs_mark_client_ready(clp, -EPROTONOSUPPORT);
+ return -EPROTONOSUPPORT;
+ case -EKEYEXPIRED:
+ nfs4_warn_keyexpired(clp->cl_hostname);
+ case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
+ * in nfs4_exchange_id */
+ default:
+ return status;
+ }
+ set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
+ return 0;
+}
+
static int nfs4_reclaim_lease(struct nfs_client *clp)
{
struct rpc_cred *cred;
const struct nfs4_state_recovery_ops *ops =
clp->cl_mvops->reboot_recovery_ops;
- int status = -ENOENT;
+ int status;

cred = ops->get_clid_cred(clp);
- if (cred != NULL) {
- status = ops->establish_clid(clp, cred);
- put_rpccred(cred);
- /* Handle case where the user hasn't set up machine creds */
- if (status == -EACCES && cred == clp->cl_machine_cred) {
- nfs4_clear_machine_cred(clp);
- status = -EAGAIN;
- }
- if (status == -NFS4ERR_MINOR_VERS_MISMATCH)
- status = -EPROTONOSUPPORT;
- }
- return status;
+ if (cred == NULL)
+ return -ENOENT;
+ status = ops->establish_clid(clp, cred);
+ put_rpccred(cred);
+ if (status != 0)
+ return nfs4_handle_reclaim_lease_error(clp, status);
+ return 0;
}

#ifdef CONFIG_NFS_V4_1
@@ -1751,32 +1782,6 @@ static int nfs4_bind_conn_to_session(struct nfs_client *clp)
}
#endif /* CONFIG_NFS_V4_1 */

-/* Set NFS4CLNT_LEASE_EXPIRED for all v4.0 errors and for recoverable errors
- * on EXCHANGE_ID for v4.1
- */
-static void nfs4_set_lease_expired(struct nfs_client *clp, int status)
-{
- switch (status) {
- case -NFS4ERR_CLID_INUSE:
- case -NFS4ERR_STALE_CLIENTID:
- clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
- break;
- case -NFS4ERR_DELAY:
- case -ETIMEDOUT:
- case -EAGAIN:
- ssleep(1);
- break;
-
- case -EKEYEXPIRED:
- nfs4_warn_keyexpired(clp->cl_hostname);
- case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
- * in nfs4_exchange_id */
- default:
- return;
- }
- set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
-}
-
static void nfs4_state_manager(struct nfs_client *clp)
{
int status = 0;
@@ -1784,7 +1789,9 @@ 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);
+ status = nfs4_reclaim_lease(clp);
+ if (status < 0)
+ goto out_error;
clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
}
@@ -1792,16 +1799,10 @@ static void nfs4_state_manager(struct nfs_client *clp)
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);
- if (status) {
- nfs4_set_lease_expired(clp, status);
- if (test_bit(NFS4CLNT_LEASE_EXPIRED,
- &clp->cl_state))
- continue;
- if (clp->cl_cons_state ==
- NFS_CS_SESSION_INITING)
- nfs_mark_client_ready(clp, status);
+ if (status < 0)
goto out_error;
- }
+ if (test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
+ continue;
clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state);

if (test_and_clear_bit(NFS4CLNT_SERVER_SCOPE_MISMATCH,
--
1.7.7.6


2012-05-25 22:03:43

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 7/8] NFSv4.1: Ensure we use the correct credentials for bind_conn_to_session

Signed-off-by: Trond Myklebust <[email protected]>
Cc: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4_fs.h | 2 +-
fs/nfs/nfs4proc.c | 3 ++-
fs/nfs/nfs4state.c | 9 ++++++++-
3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index a5dbe62..f730730 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -212,7 +212,7 @@ struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
extern int nfs4_proc_setclientid(struct nfs_client *, u32, unsigned short, struct rpc_cred *, struct nfs4_setclientid_res *);
extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setclientid_res *arg, struct rpc_cred *);
extern int nfs4_proc_get_rootfh(struct nfs_server *, struct nfs_fh *, struct nfs_fsinfo *);
-extern int nfs4_proc_bind_conn_to_session(struct nfs_client *);
+extern int nfs4_proc_bind_conn_to_session(struct nfs_client *, struct rpc_cred *cred);
extern int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred);
extern int nfs4_init_clientid(struct nfs_client *, struct rpc_cred *);
extern int nfs41_init_clientid(struct nfs_client *, struct rpc_cred *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 8fa3a36..3fdff0c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5105,7 +5105,7 @@ nfs41_same_server_scope(struct nfs41_server_scope *a,
* The 4.1 client currently uses the same TCP connection for the
* fore and backchannel.
*/
-int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
+int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred)
{
int status;
struct nfs41_bind_conn_to_session_res res;
@@ -5114,6 +5114,7 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp)
&nfs4_procedures[NFSPROC4_CLNT_BIND_CONN_TO_SESSION],
.rpc_argp = clp,
.rpc_resp = &res,
+ .rpc_cred = cred,
};

dprintk("--> %s\n", __func__);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 1587840..7dbca66 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1779,7 +1779,14 @@ static int nfs4_recall_slot(struct nfs_client *clp)

static int nfs4_bind_conn_to_session(struct nfs_client *clp)
{
- return nfs4_proc_bind_conn_to_session(clp);
+ struct rpc_cred *cred;
+ int ret;
+
+ cred = nfs4_get_exchange_id_cred(clp);
+ ret = nfs4_proc_bind_conn_to_session(clp, cred);
+ if (cred)
+ put_rpccred(cred);
+ return ret;
}
#else /* CONFIG_NFS_V4_1 */
static int nfs4_reset_session(struct nfs_client *clp) { return 0; }
--
1.7.7.6