On Thu, Oct 22, 2009 at 9:20 AM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote:
> On Oct. 21, 2009, 20:24 +0200, andros@xxxxxxxxxx wrote:
>> From: Andy Adamson <andros@xxxxxxxxxx>
>>
>> Do not clear the NFS4CLNT_SESSION_SETUP bit until after the session has been
>> reset (a possible EXCHANGE_ID, a DESTROY_SESSION, and a CREATE_SESSION) to
>> prevent a race with nfs41_setup_sequence assigning a slot on a
>> partially reset session.
>>
>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>> ---
>> fs/nfs/nfs4proc.c | 3 +++
>> fs/nfs/nfs4state.c | 15 +++++++++------
>> 2 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index eb245a1..80764e2 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -4788,6 +4788,9 @@ int nfs4_proc_create_session(struct nfs_client *clp, int reset)
>> if (status)
>> goto out;
>>
>> + /* Signal nfs41_setup_sequence that the session is ready for use */
>> + clear_bit(NFS4CLNT_SESSION_SETUP, &clp->cl_state);
>> +
>> 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]);
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index 1394dfb..09ca30b 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1252,17 +1252,20 @@ static void nfs4_state_manager(struct nfs_client *clp)
>> continue;
>> }
>> /* Initialize or reset the session */
>> - if (test_and_clear_bit(NFS4CLNT_SESSION_SETUP, &clp->cl_state)
>> - && nfs4_has_session(clp)) {
>> + if (test_bit(NFS4CLNT_SESSION_SETUP, &clp->cl_state)
>> + && nfs4_has_session(clp)) {
>> if (clp->cl_cons_state == NFS_CS_SESSION_INITING)
>> status = nfs4_initialize_session(clp);
>> else
>> status = nfs4_reset_session(clp);
>> - if (status) {
>> - if (status == -NFS4ERR_STALE_CLIENTID)
>> - continue;
>> + if (status == -NFS4ERR_STALE_CLIENTID)
>> + continue;
>> + /* For error case. On success the bit is cleared in
>> + * nfs4_proc_create_session */
>
> Why the separation?
> Why not handle both success and error cases here?
We don't clear the bit on NFS4ERR_STALE_CLIENTID because we first need
an EXCHANGE_ID and then session reset and we don't want any rpc's
queued on the slot_tbl_waitq to run. The check for
NFS4ERR_STALE_CLIENTID is in the nfs4_state_manager. Also, all other
cases in the state manager clear the bits used - so I thought it best
to follow suit.
-->Andy
>
> Benny
>
>> + clear_bit(NFS4CLNT_SESSION_SETUP, &clp->cl_state);
>> +
>> + if (status)
>> goto out_error;
>> - }
>> }
>> /* First recover reboot state... */
>> if (test_and_clear_bit(NFS4CLNT_RECLAIM_REBOOT, &clp->cl_state)) {
> _______________________________________________
> pNFS mailing list
> pNFS@xxxxxxxxxxxxx
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
