From: Andy Adamson <[email protected]>
Pre-allocating session slots for RPC tasks waking up from a wait queue provides
a shorter slot allocation code path and can help performance.
Durring multiple server reboots (interface moves), the pre-allocated
slot can be held and not freed as a task is transferred to various slot table
wait queues and the state manager is draining these queues. Not freeing the
slots results in a state manager hang waiting for completion on the slot
table draining. In this case, performance is not a consideration as the
client is in recovery of a session or a clientid.
Prevent this state manager hang by not pre-allocating session slots, but
just wake up the tasks and let them grab a sessions slot in the
nfs4_alloc_slot call in nfs41-setup_sequence.
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index e5be725..6ddd96e 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -219,7 +219,7 @@ static void nfs4_end_drain_slot_table(struct nfs4_slot_table *tbl)
{
if (test_and_clear_bit(NFS4_SLOT_TBL_DRAINING, &tbl->slot_tbl_state)) {
spin_lock(&tbl->slot_tbl_lock);
- nfs41_wake_slot_table(tbl);
+ rpc_wake_up(&tbl->slot_tbl_waitq);
spin_unlock(&tbl->slot_tbl_lock);
}
}
--
1.8.3.1
On Jan 27, 2014, at 12:25, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Pre-allocating session slots for RPC tasks waking up from a wait queue provides
> a shorter slot allocation code path and can help performance.
>
> Durring multiple server reboots (interface moves), the pre-allocated
> slot can be held and not freed as a task is transferred to various slot table
> wait queues and the state manager is draining these queues. Not freeing the
> slots results in a state manager hang waiting for completion on the slot
> table draining. In this case, performance is not a consideration as the
> client is in recovery of a session or a clientid.
>
I?m not understanding this reasoning. If the slot table is drained, then the slots must be returned to the session manager by all outstanding RPC calls so that the state manager thread can go about its business. By what mechanism are these slots being held and not freed here?
> Prevent this state manager hang by not pre-allocating session slots, but
> just wake up the tasks and let them grab a sessions slot in the
> nfs4_alloc_slot call in nfs41-setup_sequence.
If the slots are already held, then how does this help?
--
Trond Myklebust
Linux NFS client maintainer
On Jan 27, 2014, at 12:40 PM, Trond Myklebust <[email protected]>
wrote:
>
> On Jan 27, 2014, at 12:25, [email protected] wrote:
>
>> From: Andy Adamson <[email protected]>
>>
>> Pre-allocating session slots for RPC tasks waking up from a wait queue provides
>> a shorter slot allocation code path and can help performance.
>>
>> Durring multiple server reboots (interface moves), the pre-allocated
>> slot can be held and not freed as a task is transferred to various slot table
>> wait queues and the state manager is draining these queues. Not freeing the
>> slots results in a state manager hang waiting for completion on the slot
>> table draining. In this case, performance is not a consideration as the
>> client is in recovery of a session or a clientid.
>>
>
> I?m not understanding this reasoning. If the slot table is drained, then the slots must be returned to the session manager by all outstanding RPC calls so that the state manager thread can go about its business. By what mechanism are these slots being held and not freed here?
AFAICS this is the scenario.
The slot table is being drained multiple times, once for each interface move. The first time draining "ends" with pre-allocated slot tasks placed on the forechannel slot table waitq - not yet run. Then the second draining occurs prior to them being run, and the state manager sets the draining flag again, and then "waits for completion" e.g. waits for nfs4_free_slot to call nfs4_slot_table_drain_complete. The tasks with the pre-allocated slots that are left on the slot table waitq, still haven't run and can't until the draining flag is unset, nfs4_free_slot has not been called on their allocated slots and the state manager hangs.
>
>> Prevent this state manager hang by not pre-allocating session slots, but
>> just wake up the tasks and let them grab a sessions slot in the
>> nfs4_alloc_slot call in nfs41-setup_sequence.
>
> If the slots are already held, then how does this help?
The slots are pre-allocated in the nfs4_wake_slot_table call in the first draining, which sets them up to be orphaned if a second draing occurs with the right timing.
The call to rpc_wake_up fixes the hang as it move the allocation of the slots to the time when the tasks are actually ready to run - they can hang out on a waitq without preventing a draining completion call.
-->Andy
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
On Jan 27, 2014, at 13:03, Adamson, Andy <[email protected]> wrote:
>
> On Jan 27, 2014, at 12:40 PM, Trond Myklebust <[email protected]>
> wrote:
>
>>
>> On Jan 27, 2014, at 12:25, [email protected] wrote:
>>
>>> From: Andy Adamson <[email protected]>
>>>
>>> Pre-allocating session slots for RPC tasks waking up from a wait queue provides
>>> a shorter slot allocation code path and can help performance.
>>>
>>> Durring multiple server reboots (interface moves), the pre-allocated
>>> slot can be held and not freed as a task is transferred to various slot table
>>> wait queues and the state manager is draining these queues. Not freeing the
>>> slots results in a state manager hang waiting for completion on the slot
>>> table draining. In this case, performance is not a consideration as the
>>> client is in recovery of a session or a clientid.
>>>
>>
>> I?m not understanding this reasoning. If the slot table is drained, then the slots must be returned to the session manager by all outstanding RPC calls so that the state manager thread can go about its business. By what mechanism are these slots being held and not freed here?
>
> AFAICS this is the scenario.
>
> The slot table is being drained multiple times, once for each interface move. The first time draining "ends" with pre-allocated slot tasks placed on the forechannel slot table waitq - not yet run. Then the second draining occurs prior to them being run, and the state manager sets the draining flag again, and then "waits for completion" e.g. waits for nfs4_free_slot to call nfs4_slot_table_drain_complete. The tasks with the pre-allocated slots that are left on the slot table waitq, still haven't run and can't until the draining flag is unset, nfs4_free_slot has not been called on their allocated slots and the state manager hangs.
That isn?t supposed to be possible, though. Nothing should be sleeping on slot_tbl_waitq while holding a slot, and any subsequent slot assignment is supposed to be done as part of the RPC task wake-up (see __nfs41_wake_and_assign_slot).
Question: is it possible for nfs41_setup_sequence() to be called while the rpc_task is still sleeping on slot_tbl_waitq? That could cause a race, since the test for res->sr_slot is done before we grab the slot_tbl_lock.
>>
>>> Prevent this state manager hang by not pre-allocating session slots, but
>>> just wake up the tasks and let them grab a sessions slot in the
>>> nfs4_alloc_slot call in nfs41-setup_sequence.
>>
>> If the slots are already held, then how does this help?
>
>
> The slots are pre-allocated in the nfs4_wake_slot_table call in the first draining, which sets them up to be orphaned if a second draing occurs with the right timing.
>
> The call to rpc_wake_up fixes the hang as it move the allocation of the slots to the time when the tasks are actually ready to run - they can hang out on a waitq without preventing a draining completion call.
See above: nothing is allowed to sleep on slot_tbl_waitq while holding a slot.
--
Trond Myklebust
Linux NFS client maintainer