Re: [PATCH 1/4] CIFS: queue 'reconnect' thread with a delay

[Germano Percossi <germano.percossi-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>]

Some servers (Microsoft for example) reply to reconnection
requests with errors during the failover phase.
Asking repeatedly to reconnect (hundreds times in few
seconds) is not gonna solve the problem.

This is would happen (reconnection bugs aside, fix later)
without self-rescheduling and delay:
* one node fails over during I/O
* reconnect thread is immediately started
* server is not ready and send an error
* reconnect fails and exit
* echo will notice connection is down and schedule reconnect
* reconnect (hopefully succeeds)

The problem here is that the echo timeout will make the difference:
60 seconds for some applications are not tolerable and
making echo more frequent just to solve a once-off problem is
sub-optimal.

The reason for rescheduling by itself is to avoid relying
on the echo thread and make possible to tune parameters
separately: echo can still be used with its default (60s)
without impacting the reconnect that usually happens when
there is pending I/O.

My initial idea was to make it configurable like
echo_interval.
Happy to do it if the whole idea of a delay and self rescheduling
is accepted.

Cheers,
Germano

P.S: there is a coding style error in this patch, I am
sending the correct version.

On 04/05/2017 07:07 PM, Sachin Prabhu wrote:
> On Wed, 2017-03-29 at 00:26 +0100, Germano Percossi wrote:
>> All the other threads are queue with a delay, no reason
>> why this one need to be so aggressive.
> 
> Is there any reason to queue with a delay? Is there a problem with
> reconnecting immediately?
> 
> Sachin Prabhu
> 
>>
>> Signed-off-by: Germano Percossi <germano.percossi-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
>> ---
>>  fs/cifs/smb2pdu.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
>> index 7446496..efe167c 100644
>> --- a/fs/cifs/smb2pdu.c
>> +++ b/fs/cifs/smb2pdu.c
>> @@ -258,7 +258,7 @@ smb2_reconnect(__le16 smb2_command, struct
>> cifs_tcon *tcon)
>>  		goto out;
>>  
>>  	if (smb2_command != SMB2_INTERNAL_CMD)
>> -		queue_delayed_work(cifsiod_wq, &server->reconnect,
>> 0);
>> +		queue_delayed_work(cifsiod_wq, &server->reconnect, 2
>> * HZ);
>>  
>>  	atomic_inc(&tconInfoReconnectCount);
>>  out:
>> @@ -2231,7 +2231,7 @@ SMB2_echo(struct TCP_Server_Info *server)
>>  
>>  	if (server->tcpStatus == CifsNeedNegotiate) {
>>  		/* No need to send echo on newly established
>> connections */
>> -		queue_delayed_work(cifsiod_wq, &server->reconnect,
>> 0);
>> +		queue_delayed_work(cifsiod_wq, &server->reconnect, 2
>> * HZ);
>>  		return rc;
>>  	}
>>  
>