2016-10-20 6:14 GMT-07:00 Sachin Prabhu <sprabhu@xxxxxxxxxx>:
> Commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
> long after socket reconnect") changes the behaviour of the SMB2 echo
> service and causes it to renegotiate after a socket reconnect. However
> under default settings, the echo service could take up to 120 seconds to
> be scheduled.
>
> The patch forces the echo service to be called immediately resulting a
> negotiate call being made immediately on reconnect.
Looks like a right thing to do. Comments are inlined below:
>
> Signed-off-by: Sachin Prabhu <sprabhu@xxxxxxxxxx>
> ---
> fs/cifs/connect.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index aab5227..43fd838 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -412,6 +412,9 @@ cifs_reconnect(struct TCP_Server_Info *server)
> }
> } while (server->tcpStatus == CifsNeedReconnect);
>
> + if (server->tcpStatus == CifsNeedNegotiate)
> + mod_delayed_work(cifsiod_wq, &server->echo, 0);
> +
> return rc;
> }
>
> @@ -421,17 +424,25 @@ cifs_echo_request(struct work_struct *work)
> int rc;
> struct TCP_Server_Info *server = container_of(work,
> struct TCP_Server_Info, echo.work);
> - unsigned long echo_interval = server->echo_interval;
> + unsigned long echo_interval;
> +
> + /*
> + * If we need to renegotiate, set echo interval to zero to
> + * immediately call echo service where we can renegotiate.
> + */
> + if (server->tcpStatus == CifsNeedNegotiate)
> + echo_interval = 0;
Suppose we are setting echo_interval to 0 above
> + else
> + echo_interval = server->echo_interval;
>
> /*
> - * We cannot send an echo if it is disabled or until the
> - * NEGOTIATE_PROTOCOL request is done, which is indicated by
> - * server->ops->need_neg() == true. Also, no need to ping if
> - * we got a response recently.
> + * We cannot send an echo if it is disabled.
> + * Also, no need to ping if we got a response recently.
> */
>
> if (server->tcpStatus == CifsNeedReconnect ||
> - server->tcpStatus == CifsExiting || server->tcpStatus == CifsNew ||
> + server->tcpStatus == CifsExiting ||
> + server->tcpStatus == CifsNew ||
> (server->ops->can_echo && !server->ops->can_echo(server)) ||
> time_before(jiffies, server->lstrp + echo_interval - HZ))
> goto requeue_echo;
then we are sending an echo message and re-queue the echo work with
echo_interval==0:
rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
....
queue_delayed_work(cifsiod_wq, &server->echo, echo_interval);
Shouldn't we queue the echo work with server->echo_interval (not 0) here?
--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
