2011-08-12 20:48:15

by Jim Rees

[permalink] [raw]
Subject: two callback patches

When you get a chance, could you pull these into pnfs-all-latest? You'll
find them on the head of my for-benny branch, and I assume upstream.

commit 3070296f2b3ddc094bda51286c3e9687d59d2a22
Author: Trond Myklebust <[email protected]>
Date: Tue Aug 2 14:50:40 2011 -0400

NFSv4.1: Return NFS4ERR_BADSESSION to callbacks during session resets

If the client is in the process of resetting the session when it receives
a callback, then returning NFS4ERR_DELAY may cause a deadlock with the
DESTROY_SESSION call.

Basically, if the client returns NFS4ERR_DELAY in response to the
CB_SEQUENCE call, then the server is entitled to believe that the
client is busy because it is already processing that call. In that
case, the server is perfectly entitled to respond with a
NFS4ERR_BACK_CHAN_BUSY to any DESTROY_SESSION call.

Fix this by having the client reply with a NFS4ERR_BADSESSION in
response to the callback if it is resetting the session.

Cc: [email protected] [2.6.38+]
Signed-off-by: Trond Myklebust <[email protected]>

commit a8446678ba8b850bcac2286c6ed803c339049096
Author: Trond Myklebust <[email protected]>
Date: Tue Aug 2 14:50:39 2011 -0400

NFSv4.1: Fix the callback 'highest_used_slotid' behaviour

Currently, there is no guarantee that we will call nfs4_cb_take_slot() even
though nfs4_callback_compound() will consistently call
nfs4_cb_free_slot() provided the cb_process_state has set the 'clp' field.
The result is that we can trigger the BUG_ON() upon the next call to
nfs4_cb_take_slot().

This patch fixes the above problem by using the slot id that was taken in
the CB_SEQUENCE operation as a flag for whether or not we need to call
nfs4_cb_free_slot().
It also fixes an atomicity problem: we need to set tbl->highest_used_slotid
atomically with the check for NFS4_SESSION_DRAINING, otherwise we end up
racing with the various tests in nfs4_begin_drain_session().

Cc: [email protected] [2.6.38+]
Signed-off-by: Trond Myklebust <[email protected]>


2011-08-12 21:15:30

by Boaz Harrosh

[permalink] [raw]
Subject: Re: two callback patches

On 08/12/2011 01:48 PM, Jim Rees wrote:
> When you get a chance, could you pull these into pnfs-all-latest? You'll
> find them on the head of my for-benny branch, and I assume upstream.
>

Right and some other bug fixes as well. I find that all of trond/linux-next
branch are needed for proper run with v3.1-rc1 stuff. Best is if you can
merge in trond/linux-next at the client branch side if the pnfs tree.

Thanks
Boaz

> commit 3070296f2b3ddc094bda51286c3e9687d59d2a22
> Author: Trond Myklebust <[email protected]>
> Date: Tue Aug 2 14:50:40 2011 -0400
>
> NFSv4.1: Return NFS4ERR_BADSESSION to callbacks during session resets
>
> If the client is in the process of resetting the session when it receives
> a callback, then returning NFS4ERR_DELAY may cause a deadlock with the
> DESTROY_SESSION call.
>
> Basically, if the client returns NFS4ERR_DELAY in response to the
> CB_SEQUENCE call, then the server is entitled to believe that the
> client is busy because it is already processing that call. In that
> case, the server is perfectly entitled to respond with a
> NFS4ERR_BACK_CHAN_BUSY to any DESTROY_SESSION call.
>
> Fix this by having the client reply with a NFS4ERR_BADSESSION in
> response to the callback if it is resetting the session.
>
> Cc: [email protected] [2.6.38+]
> Signed-off-by: Trond Myklebust <[email protected]>
>
> commit a8446678ba8b850bcac2286c6ed803c339049096
> Author: Trond Myklebust <[email protected]>
> Date: Tue Aug 2 14:50:39 2011 -0400
>
> NFSv4.1: Fix the callback 'highest_used_slotid' behaviour
>
> Currently, there is no guarantee that we will call nfs4_cb_take_slot() even
> though nfs4_callback_compound() will consistently call
> nfs4_cb_free_slot() provided the cb_process_state has set the 'clp' field.
> The result is that we can trigger the BUG_ON() upon the next call to
> nfs4_cb_take_slot().
>
> This patch fixes the above problem by using the slot id that was taken in
> the CB_SEQUENCE operation as a flag for whether or not we need to call
> nfs4_cb_free_slot().
> It also fixes an atomicity problem: we need to set tbl->highest_used_slotid
> atomically with the check for NFS4_SESSION_DRAINING, otherwise we end up
> racing with the various tests in nfs4_begin_drain_session().
>
> Cc: [email protected] [2.6.38+]
> Signed-off-by: Trond Myklebust <[email protected]>
> --
> 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


2011-08-14 20:08:15

by Benny Halevy

[permalink] [raw]
Subject: Re: two callback patches

On 2011-08-13 00:15, Boaz Harrosh wrote:
> On 08/12/2011 01:48 PM, Jim Rees wrote:
>> When you get a chance, could you pull these into pnfs-all-latest? You'll
>> find them on the head of my for-benny branch, and I assume upstream.
>>
>
> Right and some other bug fixes as well. I find that all of trond/linux-next
> branch are needed for proper run with v3.1-rc1 stuff. Best is if you can
> merge in trond/linux-next at the client branch side if the pnfs tree.

Yup. I fetched nfs-2.6/bugfixes (same as linux-next as of today)
into my nfs-bugfixes branch and released under pnfs-all-3.1-rc1-2011-08-14

Benny

>
> Thanks
> Boaz
>
>> commit 3070296f2b3ddc094bda51286c3e9687d59d2a22
>> Author: Trond Myklebust <[email protected]>
>> Date: Tue Aug 2 14:50:40 2011 -0400
>>
>> NFSv4.1: Return NFS4ERR_BADSESSION to callbacks during session resets
>>
>> If the client is in the process of resetting the session when it receives
>> a callback, then returning NFS4ERR_DELAY may cause a deadlock with the
>> DESTROY_SESSION call.
>>
>> Basically, if the client returns NFS4ERR_DELAY in response to the
>> CB_SEQUENCE call, then the server is entitled to believe that the
>> client is busy because it is already processing that call. In that
>> case, the server is perfectly entitled to respond with a
>> NFS4ERR_BACK_CHAN_BUSY to any DESTROY_SESSION call.
>>
>> Fix this by having the client reply with a NFS4ERR_BADSESSION in
>> response to the callback if it is resetting the session.
>>
>> Cc: [email protected] [2.6.38+]
>> Signed-off-by: Trond Myklebust <[email protected]>
>>
>> commit a8446678ba8b850bcac2286c6ed803c339049096
>> Author: Trond Myklebust <[email protected]>
>> Date: Tue Aug 2 14:50:39 2011 -0400
>>
>> NFSv4.1: Fix the callback 'highest_used_slotid' behaviour
>>
>> Currently, there is no guarantee that we will call nfs4_cb_take_slot() even
>> though nfs4_callback_compound() will consistently call
>> nfs4_cb_free_slot() provided the cb_process_state has set the 'clp' field.
>> The result is that we can trigger the BUG_ON() upon the next call to
>> nfs4_cb_take_slot().
>>
>> This patch fixes the above problem by using the slot id that was taken in
>> the CB_SEQUENCE operation as a flag for whether or not we need to call
>> nfs4_cb_free_slot().
>> It also fixes an atomicity problem: we need to set tbl->highest_used_slotid
>> atomically with the check for NFS4_SESSION_DRAINING, otherwise we end up
>> racing with the various tests in nfs4_begin_drain_session().
>>
>> Cc: [email protected] [2.6.38+]
>> Signed-off-by: Trond Myklebust <[email protected]>
>> --
>> 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
>
> --
> 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