The following two patches fix the problem where we send RPCs with
sessionID of 0 during session recovery. Failed CREATE_SESSION
recovery is now done within the state manager for NFS4ERR_STALE_CLIENTID.
I believe the same applies to NFS4ERR_DELAY, but I had no way of
testing it so I did not add the error case at this time.
[PATCH 1/2] nfs41: nfs41_setup_state_renewal
[PATCH 2/2] nfs41: Don't clear DRAINING flag on NFS4ERR_STALE_CLIENTID
- ricardo
If CREATE_SESSION fails with NFS4ERR_STALE_CLIENTID, don't clear the
NFS4CLNT_SESSION_DRAINING flag and don't wake RPCs waiting for the
session to be reestablished. We don't have a session yet, so there
is no reason to wake other RPCs.
This avoids sending spurious compounds with bogus sequenceID during
session and state recovery.
Signed-off-by: Ricardo Labiaga <[email protected]>
---
fs/nfs/nfs4state.c | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index d236257..94c238d 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1250,7 +1250,7 @@ static int nfs4_reset_session(struct nfs_client *clp)
spin_unlock(&tbl->slot_tbl_lock);
status = wait_for_completion_interruptible(&ses->complete);
if (status) /* -ERESTARTSYS */
- goto out;
+ goto out_wake;
} else {
spin_unlock(&tbl->slot_tbl_lock);
}
@@ -1259,18 +1259,26 @@ static int nfs4_reset_session(struct nfs_client *clp)
if (status && status != -NFS4ERR_BADSESSION &&
status != -NFS4ERR_DEADSESSION) {
nfs4_session_recovery_handle_error(clp, status);
- goto out;
+ goto out_wake;
}
memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIONID_LEN);
status = nfs4_proc_create_session(clp);
- if (status)
+ if (status) {
nfs4_session_recovery_handle_error(clp, status);
- /* fall through*/
-out:
- /* Wake up the next rpc task even on error */
+ if (status == -NFS4ERR_STALE_CLIENTID) {
+ /*
+ * Let the state manager reestablish state
+ * without waking other yet.
+ */
+ goto out;
+ }
+ }
+out_wake:
+ /* Wake up the next rpc task */
clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
+out:
if (status == 0)
nfs41_setup_state_renewal(clp);
return status;
@@ -1337,6 +1345,8 @@ static void nfs4_state_manager(struct nfs_client *clp)
status = nfs4_reset_session(clp);
if (test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
continue;
+ if (status == -NFS4ERR_STALE_CLIENTID)
+ continue;
if (status < 0)
goto out_error;
}
--
1.5.4.3
Move call to get the lease time and the setup of the state
renewal out of nfs4_create_session so that it can be called
after clearing the DRAINING flag. We use the getattr RPC
to obtain the lease time, which requires a sequence slot.
Signed-off-by: Ricardo Labiaga <[email protected]>
---
fs/nfs/nfs4_fs.h | 2 ++
fs/nfs/nfs4proc.c | 13 -------------
fs/nfs/nfs4state.c | 27 ++++++++++++++++++++++++++-
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 50dd550..7e57b04 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -225,6 +225,8 @@ 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_init_session(struct nfs_server *server);
+extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
+ struct nfs_fsinfo *fsinfo);
#else /* CONFIG_NFS_v4_1 */
static inline int nfs4_setup_sequence(struct nfs_client *clp,
struct nfs4_sequence_args *args, struct nfs4_sequence_res *res,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b4ef570..4be0369 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4745,7 +4745,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
{
int status;
unsigned *ptr;
- struct nfs_fsinfo fsinfo;
struct nfs4_session *session = clp->cl_session;
dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
@@ -4767,18 +4766,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
ptr = (unsigned *)&session->sess_id.data[0];
dprintk("%s client>seqid %d sessionid %u:%u:%u:%u\n", __func__,
clp->cl_seqid, ptr[0], ptr[1], ptr[2], ptr[3]);
-
- /* Get the lease time */
- status = nfs4_proc_get_lease_time(clp, &fsinfo);
- if (status == 0) {
- /* Update lease time and schedule renewal */
- spin_lock(&clp->cl_lock);
- clp->cl_lease_time = fsinfo.lease_time * HZ;
- clp->cl_last_renewal = jiffies;
- spin_unlock(&clp->cl_lock);
-
- nfs4_schedule_state_renewal(clp);
- }
out:
dprintk("<-- %s\n", __func__);
return status;
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index ef9622e..d236257 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -116,14 +116,37 @@ struct rpc_cred *nfs4_get_renew_cred_locked(struct nfs_client *clp)
#if defined(CONFIG_NFS_V4_1)
+static int nfs41_setup_state_renewal(struct nfs_client *clp)
+{
+ int status;
+ struct nfs_fsinfo fsinfo;
+
+ status = nfs4_proc_get_lease_time(clp, &fsinfo);
+ if (status == 0) {
+ /* Update lease time and schedule renewal */
+ spin_lock(&clp->cl_lock);
+ clp->cl_lease_time = fsinfo.lease_time * HZ;
+ clp->cl_last_renewal = jiffies;
+ spin_unlock(&clp->cl_lock);
+
+ nfs4_schedule_state_renewal(clp);
+ }
+
+ return status;
+}
+
int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
{
int status;
status = nfs4_proc_exchange_id(clp, cred);
if (status == 0)
- /* create session schedules state renewal upon success */
status = nfs4_proc_create_session(clp);
+ if (status == 0 && test_and_clear_bit(
+ NFS4CLNT_SESSION_DRAINING, &clp->cl_state))
+ rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
+ if (status == 0)
+ nfs41_setup_state_renewal(clp);
if (status == 0)
nfs_mark_client_ready(clp, NFS_CS_READY);
return status;
@@ -1248,6 +1271,8 @@ out:
/* Wake up the next rpc task even on error */
clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
+ if (status == 0)
+ nfs41_setup_state_renewal(clp);
return status;
}
--
1.5.4.3
On Sun, 2009-12-06 at 03:16 -0800, Ricardo Labiaga wrote:
> If CREATE_SESSION fails with NFS4ERR_STALE_CLIENTID, don't clear the
> NFS4CLNT_SESSION_DRAINING flag and don't wake RPCs waiting for the
> session to be reestablished. We don't have a session yet, so there
> is no reason to wake other RPCs.
>
> This avoids sending spurious compounds with bogus sequenceID during
> session and state recovery.
>
> Signed-off-by: Ricardo Labiaga <[email protected]>
> ---
> fs/nfs/nfs4state.c | 22 ++++++++++++++++------
> 1 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index d236257..94c238d 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1250,7 +1250,7 @@ static int nfs4_reset_session(struct nfs_client *clp)
> spin_unlock(&tbl->slot_tbl_lock);
> status = wait_for_completion_interruptible(&ses->complete);
> if (status) /* -ERESTARTSYS */
> - goto out;
> + goto out_wake;
> } else {
> spin_unlock(&tbl->slot_tbl_lock);
> }
> @@ -1259,18 +1259,26 @@ static int nfs4_reset_session(struct nfs_client *clp)
> if (status && status != -NFS4ERR_BADSESSION &&
> status != -NFS4ERR_DEADSESSION) {
> nfs4_session_recovery_handle_error(clp, status);
> - goto out;
> + goto out_wake;
> }
>
> memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIONID_LEN);
> status = nfs4_proc_create_session(clp);
> - if (status)
> + if (status) {
> nfs4_session_recovery_handle_error(clp, status);
> - /* fall through*/
> -out:
> - /* Wake up the next rpc task even on error */
> + if (status == -NFS4ERR_STALE_CLIENTID) {
Let's do this with a test bit too...
> + /*
> + * Let the state manager reestablish state
> + * without waking other yet.
> + */
> + goto out;
> + }
> + }
> +out_wake:
> + /* Wake up the next rpc task */
> clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
> rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
> +out:
-out:
> if (status == 0)
> nfs41_setup_state_renewal(clp);
+out:
> return status;
> @@ -1337,6 +1345,8 @@ static void nfs4_state_manager(struct nfs_client *clp)
> status = nfs4_reset_session(clp);
> if (test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
> continue;
> + if (status == -NFS4ERR_STALE_CLIENTID)
> + continue;
This should be redundant. The above test for LEASE_EXPIRED should catch
it first.
> if (status < 0)
> goto out_error;
> }
On Sun, 2009-12-06 at 03:16 -0800, Ricardo Labiaga wrote:
> If CREATE_SESSION fails with NFS4ERR_STALE_CLIENTID, don't clear the
> NFS4CLNT_SESSION_DRAINING flag and don't wake RPCs waiting for the
> session to be reestablished. We don't have a session yet, so there
> is no reason to wake other RPCs.
>
> This avoids sending spurious compounds with bogus sequenceID during
> session and state recovery.
How about the following instead? It ensures that we drain the session
before renewing the lease too.
Cheers
Trond
---------------------------------------------------------------------------------------
nfs41: Don't clear DRAINING flag on NFS4ERR_STALE_CLIENTID
From: Ricardo Labiaga <[email protected]>
If CREATE_SESSION fails with NFS4ERR_STALE_CLIENTID, don't clear the
NFS4CLNT_SESSION_DRAINING flag and don't wake RPCs waiting for the
session to be reestablished. We don't have a session yet, so there
is no reason to wake other RPCs.
This avoids sending spurious compounds with bogus sequenceID during
session and state recovery.
Signed-off-by: Ricardo Labiaga <[email protected]>
[[email protected]: cleaned up patch by adding the
nfs41_begin/end_drain_session() helpers]
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 2 ++
fs/nfs/nfs4state.c | 59 ++++++++++++++++++++++++++++++++++++----------------
2 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 4be0369..fbae2c9 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4586,10 +4586,12 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
init_completion(&session->complete);
tbl = &session->fc_slot_table;
+ tbl->highest_used_slotid = -1;
spin_lock_init(&tbl->slot_tbl_lock);
rpc_init_wait_queue(&tbl->slot_tbl_waitq, "ForeChannel Slot table");
tbl = &session->bc_slot_table;
+ tbl->highest_used_slotid = -1;
spin_lock_init(&tbl->slot_tbl_lock);
rpc_init_wait_queue(&tbl->slot_tbl_waitq, "BackChannel Slot table");
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 9cfe686..1b629cc 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -135,16 +135,43 @@ static int nfs41_setup_state_renewal(struct nfs_client *clp)
return status;
}
+static void nfs41_end_drain_session(struct nfs_client *clp,
+ struct nfs4_session *ses)
+{
+ if (test_and_clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state))
+ rpc_wake_up(&ses->fc_slot_table.slot_tbl_waitq);
+}
+
+static int nfs41_begin_drain_session(struct nfs_client *clp,
+ struct nfs4_session *ses)
+{
+ struct nfs4_slot_table *tbl = &ses->fc_slot_table;
+
+ spin_lock(&tbl->slot_tbl_lock);
+ set_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
+ if (tbl->highest_used_slotid != -1) {
+ INIT_COMPLETION(ses->complete);
+ spin_unlock(&tbl->slot_tbl_lock);
+ return wait_for_completion_interruptible(&ses->complete);
+ }
+ spin_unlock(&tbl->slot_tbl_lock);
+ return 0;
+}
+
int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
{
int status;
+ status = nfs41_begin_drain_session(clp, clp->cl_session);
+ if (status != 0)
+ goto out;
status = nfs4_proc_exchange_id(clp, cred);
if (status != 0)
goto out;
status = nfs4_proc_create_session(clp);
if (status != 0)
goto out;
+ nfs41_end_drain_session(clp, clp->cl_session);
nfs41_setup_state_renewal(clp);
nfs_mark_client_ready(clp, NFS_CS_READY);
out:
@@ -1239,20 +1266,11 @@ static void nfs4_session_recovery_handle_error(struct nfs_client *clp, int err)
static int nfs4_reset_session(struct nfs_client *clp)
{
struct nfs4_session *ses = clp->cl_session;
- struct nfs4_slot_table *tbl = &ses->fc_slot_table;
int status;
- spin_lock(&tbl->slot_tbl_lock);
- set_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
- if (tbl->highest_used_slotid != -1) {
- INIT_COMPLETION(ses->complete);
- spin_unlock(&tbl->slot_tbl_lock);
- status = wait_for_completion_interruptible(&ses->complete);
- if (status) /* -ERESTARTSYS */
- goto out;
- } else {
- spin_unlock(&tbl->slot_tbl_lock);
- }
+ status = nfs41_begin_drain_session(clp, ses);
+ if (status != 0)
+ return status;
status = nfs4_proc_destroy_session(clp->cl_session);
if (status && status != -NFS4ERR_BADSESSION &&
@@ -1265,13 +1283,18 @@ static int nfs4_reset_session(struct nfs_client *clp)
status = nfs4_proc_create_session(clp);
if (status)
nfs4_session_recovery_handle_error(clp, status);
- /* fall through*/
+
out:
- /* Wake up the next rpc task even on error */
- clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
- rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
- if (status == 0)
- nfs41_setup_state_renewal(clp);
+ /*
+ * Let the state manager reestablish state
+ * without waking other tasks yet.
+ */
+ if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
+ /* Wake up the next rpc task */
+ nfs41_end_drain_session(clp, ses);
+ if (status == 0)
+ nfs41_setup_state_renewal(clp);
+ }
return status;
}
On Sun, 2009-12-06 at 03:16 -0800, Ricardo Labiaga wrote:
> Move call to get the lease time and the setup of the state
> renewal out of nfs4_create_session so that it can be called
> after clearing the DRAINING flag. We use the getattr RPC
> to obtain the lease time, which requires a sequence slot.
>
> Signed-off-by: Ricardo Labiaga <[email protected]>
<snip>
> status = nfs4_proc_exchange_id(clp, cred);
> if (status == 0)
> - /* create session schedules state renewal upon success */
> status = nfs4_proc_create_session(clp);
> + if (status == 0 && test_and_clear_bit(
> + NFS4CLNT_SESSION_DRAINING, &clp->cl_state))
> + rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
This was clearly supposed to be part of PATCH 2/2...
> + if (status == 0)
> + nfs41_setup_state_renewal(clp);
Hrm... Lots of tests of 'status == 0' without status actually changing.
I've fixed this up (see below).
> if (status == 0)
> nfs_mark_client_ready(clp, NFS_CS_READY);
> return status;
-------------------------------------------------------------------------------------------------------
nfs41: nfs41_setup_state_renewal
From: Ricardo Labiaga <[email protected]>
Move call to get the lease time and the setup of the state
renewal out of nfs4_create_session so that it can be called
after clearing the DRAINING flag. We use the getattr RPC
to obtain the lease time, which requires a sequence slot.
Signed-off-by: Ricardo Labiaga <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 2 ++
fs/nfs/nfs4proc.c | 13 -------------
fs/nfs/nfs4state.c | 34 +++++++++++++++++++++++++++++-----
3 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 50dd550..7e57b04 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -225,6 +225,8 @@ 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_init_session(struct nfs_server *server);
+extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
+ struct nfs_fsinfo *fsinfo);
#else /* CONFIG_NFS_v4_1 */
static inline int nfs4_setup_sequence(struct nfs_client *clp,
struct nfs4_sequence_args *args, struct nfs4_sequence_res *res,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b4ef570..4be0369 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4745,7 +4745,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
{
int status;
unsigned *ptr;
- struct nfs_fsinfo fsinfo;
struct nfs4_session *session = clp->cl_session;
dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
@@ -4767,18 +4766,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
ptr = (unsigned *)&session->sess_id.data[0];
dprintk("%s client>seqid %d sessionid %u:%u:%u:%u\n", __func__,
clp->cl_seqid, ptr[0], ptr[1], ptr[2], ptr[3]);
-
- /* Get the lease time */
- status = nfs4_proc_get_lease_time(clp, &fsinfo);
- if (status == 0) {
- /* Update lease time and schedule renewal */
- spin_lock(&clp->cl_lock);
- clp->cl_lease_time = fsinfo.lease_time * HZ;
- clp->cl_last_renewal = jiffies;
- spin_unlock(&clp->cl_lock);
-
- nfs4_schedule_state_renewal(clp);
- }
out:
dprintk("<-- %s\n", __func__);
return status;
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index ef9622e..9cfe686 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -116,16 +116,38 @@ struct rpc_cred *nfs4_get_renew_cred_locked(struct nfs_client *clp)
#if defined(CONFIG_NFS_V4_1)
+static int nfs41_setup_state_renewal(struct nfs_client *clp)
+{
+ int status;
+ struct nfs_fsinfo fsinfo;
+
+ status = nfs4_proc_get_lease_time(clp, &fsinfo);
+ if (status == 0) {
+ /* Update lease time and schedule renewal */
+ spin_lock(&clp->cl_lock);
+ clp->cl_lease_time = fsinfo.lease_time * HZ;
+ clp->cl_last_renewal = jiffies;
+ spin_unlock(&clp->cl_lock);
+
+ nfs4_schedule_state_renewal(clp);
+ }
+
+ return status;
+}
+
int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
{
int status;
status = nfs4_proc_exchange_id(clp, cred);
- if (status == 0)
- /* create session schedules state renewal upon success */
- status = nfs4_proc_create_session(clp);
- if (status == 0)
- nfs_mark_client_ready(clp, NFS_CS_READY);
+ if (status != 0)
+ goto out;
+ status = nfs4_proc_create_session(clp);
+ if (status != 0)
+ goto out;
+ nfs41_setup_state_renewal(clp);
+ nfs_mark_client_ready(clp, NFS_CS_READY);
+out:
return status;
}
@@ -1248,6 +1270,8 @@ out:
/* Wake up the next rpc task even on error */
clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
+ if (status == 0)
+ nfs41_setup_state_renewal(clp);
return status;
}
On 12/6/09 10:02 AM, "Trond Myklebust" <[email protected]> wrote:
> On Sun, 2009-12-06 at 03:16 -0800, Ricardo Labiaga wrote:
>> Move call to get the lease time and the setup of the state
>> renewal out of nfs4_create_session so that it can be called
>> after clearing the DRAINING flag. We use the getattr RPC
>> to obtain the lease time, which requires a sequence slot.
>>
>> Signed-off-by: Ricardo Labiaga <[email protected]>
> <snip>
>> status = nfs4_proc_exchange_id(clp, cred);
>> if (status == 0)
>> - /* create session schedules state renewal upon success */
>> status = nfs4_proc_create_session(clp);
>> + if (status == 0 && test_and_clear_bit(
>> + NFS4CLNT_SESSION_DRAINING, &clp->cl_state))
>> + rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
>
> This was clearly supposed to be part of PATCH 2/2...
>
Right.
>> + if (status == 0)
>> + nfs41_setup_state_renewal(clp);
>
> Hrm... Lots of tests of 'status == 0' without status actually changing.
> I've fixed this up (see below).
Looks good, thanks.
- ricardo
>
>> if (status == 0)
>> nfs_mark_client_ready(clp, NFS_CS_READY);
>> return status;
>
> ------------------------------------------------------------------------------
> -------------------------
> nfs41: nfs41_setup_state_renewal
>
> From: Ricardo Labiaga <[email protected]>
>
> Move call to get the lease time and the setup of the state
> renewal out of nfs4_create_session so that it can be called
> after clearing the DRAINING flag. We use the getattr RPC
> to obtain the lease time, which requires a sequence slot.
>
> Signed-off-by: Ricardo Labiaga <[email protected]>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
>
> fs/nfs/nfs4_fs.h | 2 ++
> fs/nfs/nfs4proc.c | 13 -------------
> fs/nfs/nfs4state.c | 34 +++++++++++++++++++++++++++++-----
> 3 files changed, 31 insertions(+), 18 deletions(-)
>
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 50dd550..7e57b04 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -225,6 +225,8 @@ 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_init_session(struct nfs_server *server);
> +extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
> + struct nfs_fsinfo *fsinfo);
> #else /* CONFIG_NFS_v4_1 */
> static inline int nfs4_setup_sequence(struct nfs_client *clp,
> struct nfs4_sequence_args *args, struct nfs4_sequence_res *res,
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index b4ef570..4be0369 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4745,7 +4745,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
> {
> int status;
> unsigned *ptr;
> - struct nfs_fsinfo fsinfo;
> struct nfs4_session *session = clp->cl_session;
>
> dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
> @@ -4767,18 +4766,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
> ptr = (unsigned *)&session->sess_id.data[0];
> dprintk("%s client>seqid %d sessionid %u:%u:%u:%u\n", __func__,
> clp->cl_seqid, ptr[0], ptr[1], ptr[2], ptr[3]);
> -
> - /* Get the lease time */
> - status = nfs4_proc_get_lease_time(clp, &fsinfo);
> - if (status == 0) {
> - /* Update lease time and schedule renewal */
> - spin_lock(&clp->cl_lock);
> - clp->cl_lease_time = fsinfo.lease_time * HZ;
> - clp->cl_last_renewal = jiffies;
> - spin_unlock(&clp->cl_lock);
> -
> - nfs4_schedule_state_renewal(clp);
> - }
> out:
> dprintk("<-- %s\n", __func__);
> return status;
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index ef9622e..9cfe686 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -116,16 +116,38 @@ struct rpc_cred *nfs4_get_renew_cred_locked(struct
> nfs_client *clp)
>
> #if defined(CONFIG_NFS_V4_1)
>
> +static int nfs41_setup_state_renewal(struct nfs_client *clp)
> +{
> + int status;
> + struct nfs_fsinfo fsinfo;
> +
> + status = nfs4_proc_get_lease_time(clp, &fsinfo);
> + if (status == 0) {
> + /* Update lease time and schedule renewal */
> + spin_lock(&clp->cl_lock);
> + clp->cl_lease_time = fsinfo.lease_time * HZ;
> + clp->cl_last_renewal = jiffies;
> + spin_unlock(&clp->cl_lock);
> +
> + nfs4_schedule_state_renewal(clp);
> + }
> +
> + return status;
> +}
> +
> int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> {
> int status;
>
> status = nfs4_proc_exchange_id(clp, cred);
> - if (status == 0)
> - /* create session schedules state renewal upon success */
> - status = nfs4_proc_create_session(clp);
> - if (status == 0)
> - nfs_mark_client_ready(clp, NFS_CS_READY);
> + if (status != 0)
> + goto out;
> + status = nfs4_proc_create_session(clp);
> + if (status != 0)
> + goto out;
> + nfs41_setup_state_renewal(clp);
> + nfs_mark_client_ready(clp, NFS_CS_READY);
> +out:
> return status;
> }
>
> @@ -1248,6 +1270,8 @@ out:
> /* Wake up the next rpc task even on error */
> clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
> rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
> + if (status == 0)
> + nfs41_setup_state_renewal(clp);
> return status;
> }
>
>
>
On 12/6/09 10:02 AM, "Trond Myklebust" <[email protected]> wrote:
> On Sun, 2009-12-06 at 03:16 -0800, Ricardo Labiaga wrote:
>> If CREATE_SESSION fails with NFS4ERR_STALE_CLIENTID, don't clear the
>> NFS4CLNT_SESSION_DRAINING flag and don't wake RPCs waiting for the
>> session to be reestablished. We don't have a session yet, so there
>> is no reason to wake other RPCs.
>>
>> This avoids sending spurious compounds with bogus sequenceID during
>> session and state recovery.
>
> How about the following instead? It ensures that we drain the session
> before renewing the lease too.
>
Looks good, I tested it as well.
- ricardo
> Cheers
> Trond
> ------------------------------------------------------------------------------
> ---------
> nfs41: Don't clear DRAINING flag on NFS4ERR_STALE_CLIENTID
>
> From: Ricardo Labiaga <[email protected]>
>
> If CREATE_SESSION fails with NFS4ERR_STALE_CLIENTID, don't clear the
> NFS4CLNT_SESSION_DRAINING flag and don't wake RPCs waiting for the
> session to be reestablished. We don't have a session yet, so there
> is no reason to wake other RPCs.
>
> This avoids sending spurious compounds with bogus sequenceID during
> session and state recovery.
>
> Signed-off-by: Ricardo Labiaga <[email protected]>
> [[email protected]: cleaned up patch by adding the
> nfs41_begin/end_drain_session() helpers]
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
>
> fs/nfs/nfs4proc.c | 2 ++
> fs/nfs/nfs4state.c | 59
> ++++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 43 insertions(+), 18 deletions(-)
>
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 4be0369..fbae2c9 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4586,10 +4586,12 @@ struct nfs4_session *nfs4_alloc_session(struct
> nfs_client *clp)
> init_completion(&session->complete);
>
> tbl = &session->fc_slot_table;
> + tbl->highest_used_slotid = -1;
> spin_lock_init(&tbl->slot_tbl_lock);
> rpc_init_wait_queue(&tbl->slot_tbl_waitq, "ForeChannel Slot table");
>
> tbl = &session->bc_slot_table;
> + tbl->highest_used_slotid = -1;
> spin_lock_init(&tbl->slot_tbl_lock);
> rpc_init_wait_queue(&tbl->slot_tbl_waitq, "BackChannel Slot table");
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 9cfe686..1b629cc 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -135,16 +135,43 @@ static int nfs41_setup_state_renewal(struct nfs_client
> *clp)
> return status;
> }
>
> +static void nfs41_end_drain_session(struct nfs_client *clp,
> + struct nfs4_session *ses)
> +{
> + if (test_and_clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state))
> + rpc_wake_up(&ses->fc_slot_table.slot_tbl_waitq);
> +}
> +
> +static int nfs41_begin_drain_session(struct nfs_client *clp,
> + struct nfs4_session *ses)
> +{
> + struct nfs4_slot_table *tbl = &ses->fc_slot_table;
> +
> + spin_lock(&tbl->slot_tbl_lock);
> + set_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
> + if (tbl->highest_used_slotid != -1) {
> + INIT_COMPLETION(ses->complete);
> + spin_unlock(&tbl->slot_tbl_lock);
> + return wait_for_completion_interruptible(&ses->complete);
> + }
> + spin_unlock(&tbl->slot_tbl_lock);
> + return 0;
> +}
> +
> int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> {
> int status;
>
> + status = nfs41_begin_drain_session(clp, clp->cl_session);
> + if (status != 0)
> + goto out;
> status = nfs4_proc_exchange_id(clp, cred);
> if (status != 0)
> goto out;
> status = nfs4_proc_create_session(clp);
> if (status != 0)
> goto out;
> + nfs41_end_drain_session(clp, clp->cl_session);
> nfs41_setup_state_renewal(clp);
> nfs_mark_client_ready(clp, NFS_CS_READY);
> out:
> @@ -1239,20 +1266,11 @@ static void nfs4_session_recovery_handle_error(struct
> nfs_client *clp, int err)
> static int nfs4_reset_session(struct nfs_client *clp)
> {
> struct nfs4_session *ses = clp->cl_session;
> - struct nfs4_slot_table *tbl = &ses->fc_slot_table;
> int status;
>
> - spin_lock(&tbl->slot_tbl_lock);
> - set_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
> - if (tbl->highest_used_slotid != -1) {
> - INIT_COMPLETION(ses->complete);
> - spin_unlock(&tbl->slot_tbl_lock);
> - status = wait_for_completion_interruptible(&ses->complete);
> - if (status) /* -ERESTARTSYS */
> - goto out;
> - } else {
> - spin_unlock(&tbl->slot_tbl_lock);
> - }
> + status = nfs41_begin_drain_session(clp, ses);
> + if (status != 0)
> + return status;
>
> status = nfs4_proc_destroy_session(clp->cl_session);
> if (status && status != -NFS4ERR_BADSESSION &&
> @@ -1265,13 +1283,18 @@ static int nfs4_reset_session(struct nfs_client *clp)
> status = nfs4_proc_create_session(clp);
> if (status)
> nfs4_session_recovery_handle_error(clp, status);
> - /* fall through*/
> +
> out:
> - /* Wake up the next rpc task even on error */
> - clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
> - rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq);
> - if (status == 0)
> - nfs41_setup_state_renewal(clp);
> + /*
> + * Let the state manager reestablish state
> + * without waking other tasks yet.
> + */
> + if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
> + /* Wake up the next rpc task */
> + nfs41_end_drain_session(clp, ses);
> + if (status == 0)
> + nfs41_setup_state_renewal(clp);
> + }
> return status;
> }
>
>