cifs: Make echo interval tunable.
diff mbox

Message ID 1450097226-20815-1-git-send-email-sprabhu@redhat.com
State New
Headers show

Commit Message

Sachin Prabhu Dec. 14, 2015, 12:47 p.m. UTC
Currently the echo interval is set to 60 seconds using a macro. This
setting determines the interval at which echo requests are sent to the
server on an idling connection. This setting also affects the time
required for a connection to an unresponsive server to timeout.

Making this setting a tunable allows users to control the echo interval
times as well as control the time after which the connecting to an
unresponsive server times out.

Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
---
 fs/cifs/cifsfs.c   |  2 ++
 fs/cifs/cifsglob.h |  8 ++++++--
 fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
 3 files changed, 35 insertions(+), 8 deletions(-)

Comments

Sachin Prabhu Dec. 14, 2015, 12:54 p.m. UTC | #1
Another approach as pointed to by Jeff Layton is to make it a module
parameter which is then set for all shares mounted on the client.

Any comments on the patch are welcome.

Sachin Prabhu

On Mon, 2015-12-14 at 18:17 +0530, Sachin Prabhu wrote:
> Currently the echo interval is set to 60 seconds using a macro. This
> setting determines the interval at which echo requests are sent to
> the
> server on an idling connection. This setting also affects the time
> required for a connection to an unresponsive server to timeout.
> 
> Making this setting a tunable allows users to control the echo
> interval
> times as well as control the time after which the connecting to an
> unresponsive server times out.
> 
> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> ---
>  fs/cifs/cifsfs.c   |  2 ++
>  fs/cifs/cifsglob.h |  8 ++++++--
>  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
>  3 files changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index cbc0f4b..eab2de6 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct
> dentry *root)
>  
>  	seq_printf(s, ",rsize=%u", cifs_sb->rsize);
>  	seq_printf(s, ",wsize=%u", cifs_sb->wsize);
> +	seq_printf(s, ",echo_interval=%lu",
> +			tcon->ses->server->echo_interval / HZ);
>  	/* convert actimeo and display it in seconds */
>  	seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
>  
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 2b510c5..56d3698 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -70,8 +70,10 @@
>  #define SERVER_NAME_LENGTH 40
>  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
>  
> -/* SMB echo "timeout" -- FIXME: tunable? */
> -#define SMB_ECHO_INTERVAL (60 * HZ)
> +/* echo interval in seconds */
> +#define SMB_ECHO_INTERVAL_MIN 10
> +#define SMB_ECHO_INTERVAL_MAX 120
> +#define SMB_ECHO_INTERVAL_DEFAULT 60
>  
>  #include "cifspdu.h"
>  
> @@ -507,6 +509,7 @@ struct smb_vol {
>  	struct sockaddr_storage dstaddr; /* destination address */
>  	struct sockaddr_storage srcaddr; /* allow binding to a local
> IP */
>  	struct nls_table *local_nls;
> +	unsigned int echo_interval; /* echo interval in secs */
>  };
>  
>  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID | \
> @@ -628,6 +631,7 @@ struct TCP_Server_Info {
>  	unsigned int	max_read;
>  	unsigned int	max_write;
>  #endif /* CONFIG_CIFS_SMB2 */
> +	unsigned long echo_interval;
>  };
>  
>  static inline unsigned int
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ecb0803..3b44e9e 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -95,6 +95,7 @@ enum {
>  	Opt_cruid, Opt_gid, Opt_file_mode,
>  	Opt_dirmode, Opt_port,
>  	Opt_rsize, Opt_wsize, Opt_actimeo,
> +	Opt_echo_interval,
>  
>  	/* Mount options which take string value */
>  	Opt_user, Opt_pass, Opt_ip,
> @@ -188,6 +189,7 @@ static const match_table_t
> cifs_mount_option_tokens = {
>  	{ Opt_rsize, "rsize=%s" },
>  	{ Opt_wsize, "wsize=%s" },
>  	{ Opt_actimeo, "actimeo=%s" },
> +	{ Opt_echo_interval, "echo_interval=%s" },
>  
>  	{ Opt_blank_user, "user=" },
>  	{ Opt_blank_user, "username=" },
> @@ -418,6 +420,7 @@ 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;
>  
>  	/*
>  	 * We cannot send an echo if it is disabled or until the
> @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
>  	 */
>  	if (!server->ops->need_neg || server->ops->need_neg(server)
> ||
>  	    (server->ops->can_echo && !server->ops
> ->can_echo(server)) ||
> -	    time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL -
> HZ))
> +	    time_before(jiffies, server->lstrp + echo_interval -
> HZ))
>  		goto requeue_echo;
>  
>  	rc = server->ops->echo ? server->ops->echo(server) : 
> -ENOSYS;
> @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
>  			 server->hostname);
>  
>  requeue_echo:
> -	queue_delayed_work(cifsiod_wq, &server->echo,
> SMB_ECHO_INTERVAL);
> +	queue_delayed_work(cifsiod_wq, &server->echo,
> echo_interval);
>  }
>  
>  static bool
> @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info
> *server)
>  	 *     a response in >60s.
>  	 */
>  	if (server->tcpStatus == CifsGood &&
> -	    time_after(jiffies, server->lstrp + 2 *
> SMB_ECHO_INTERVAL)) {
> -		cifs_dbg(VFS, "Server %s has not responded in %d
> seconds. Reconnecting...\n",
> -			 server->hostname, (2 * SMB_ECHO_INTERVAL) /
> HZ);
> +	    time_after(jiffies, server->lstrp + 2 * server
> ->echo_interval)) {
> +		cifs_dbg(VFS, "Server %s has not responded in %lu
> seconds. Reconnecting...\n",
> +			 server->hostname, (2 * server
> ->echo_interval) / HZ);
>  		cifs_reconnect(server);
>  		wake_up(&server->response_q);
>  		return true;
> @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char
> *mountdata, const char *devname,
>  				goto cifs_parse_mount_err;
>  			}
>  			break;
> +		case Opt_echo_interval:
> +			if (get_option_ul(args, &option)) {
> +				cifs_dbg(VFS, "%s: Invalid echo
> interval value\n",
> +					 __func__);
> +				goto cifs_parse_mount_err;
> +			}
> +			vol->echo_interval = option;
> +			break;
>  
>  		/* String Arguments */
>  
> @@ -2089,6 +2100,9 @@ static int match_server(struct TCP_Server_Info
> *server, struct smb_vol *vol)
>  	if (!match_security(server, vol))
>  		return 0;
>  
> +	if (server->echo_interval != vol->echo_interval)
> +		return 0;
> +
>  	return 1;
>  }
>  
> @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol
> *volume_info)
>  	tcp_ses->tcpStatus = CifsNew;
>  	++tcp_ses->srv_count;
>  
> +	/* echo interval should be between 10 and 120 secs */
> +	if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
> +		volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
> +		tcp_ses->echo_interval = volume_info->echo_interval
> * HZ;
> +	else
> +		tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT *
> HZ;
> +
>  	rc = ip_connect(tcp_ses);
>  	if (rc < 0) {
>  		cifs_dbg(VFS, "Error connecting to socket. Aborting
> operation.\n");
> @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol
> *volume_info)
>  	cifs_fscache_get_client_cookie(tcp_ses);
>  
>  	/* queue echo request delayed work */
> -	queue_delayed_work(cifsiod_wq, &tcp_ses->echo,
> SMB_ECHO_INTERVAL);
> +	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses
> ->echo_interval);
>  
>  	return tcp_ses;
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shirish Pargaonkar Dec. 15, 2015, 3:58 a.m. UTC | #2
Looks correct. Only comment would be to keep one definition of echo_interval,
preferably unsigned int (instead of unsigned long also).

On Mon, Dec 14, 2015 at 6:54 AM, Sachin Prabhu <sprabhu@redhat.com> wrote:
> Another approach as pointed to by Jeff Layton is to make it a module
> parameter which is then set for all shares mounted on the client.
>
> Any comments on the patch are welcome.
>
> Sachin Prabhu
>
> On Mon, 2015-12-14 at 18:17 +0530, Sachin Prabhu wrote:
>> Currently the echo interval is set to 60 seconds using a macro. This
>> setting determines the interval at which echo requests are sent to
>> the
>> server on an idling connection. This setting also affects the time
>> required for a connection to an unresponsive server to timeout.
>>
>> Making this setting a tunable allows users to control the echo
>> interval
>> times as well as control the time after which the connecting to an
>> unresponsive server times out.
>>
>> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
>> ---
>>  fs/cifs/cifsfs.c   |  2 ++
>>  fs/cifs/cifsglob.h |  8 ++++++--
>>  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
>>  3 files changed, 35 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index cbc0f4b..eab2de6 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct
>> dentry *root)
>>
>>       seq_printf(s, ",rsize=%u", cifs_sb->rsize);
>>       seq_printf(s, ",wsize=%u", cifs_sb->wsize);
>> +     seq_printf(s, ",echo_interval=%lu",
>> +                     tcon->ses->server->echo_interval / HZ);
>>       /* convert actimeo and display it in seconds */
>>       seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 2b510c5..56d3698 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -70,8 +70,10 @@
>>  #define SERVER_NAME_LENGTH 40
>>  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
>>
>> -/* SMB echo "timeout" -- FIXME: tunable? */
>> -#define SMB_ECHO_INTERVAL (60 * HZ)
>> +/* echo interval in seconds */
>> +#define SMB_ECHO_INTERVAL_MIN 10
>> +#define SMB_ECHO_INTERVAL_MAX 120
>> +#define SMB_ECHO_INTERVAL_DEFAULT 60
>>
>>  #include "cifspdu.h"
>>
>> @@ -507,6 +509,7 @@ struct smb_vol {
>>       struct sockaddr_storage dstaddr; /* destination address */
>>       struct sockaddr_storage srcaddr; /* allow binding to a local
>> IP */
>>       struct nls_table *local_nls;
>> +     unsigned int echo_interval; /* echo interval in secs */
>>  };
>>
>>  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID | \
>> @@ -628,6 +631,7 @@ struct TCP_Server_Info {
>>       unsigned int    max_read;
>>       unsigned int    max_write;
>>  #endif /* CONFIG_CIFS_SMB2 */
>> +     unsigned long echo_interval;
>>  };
>>
>>  static inline unsigned int
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index ecb0803..3b44e9e 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -95,6 +95,7 @@ enum {
>>       Opt_cruid, Opt_gid, Opt_file_mode,
>>       Opt_dirmode, Opt_port,
>>       Opt_rsize, Opt_wsize, Opt_actimeo,
>> +     Opt_echo_interval,
>>
>>       /* Mount options which take string value */
>>       Opt_user, Opt_pass, Opt_ip,
>> @@ -188,6 +189,7 @@ static const match_table_t
>> cifs_mount_option_tokens = {
>>       { Opt_rsize, "rsize=%s" },
>>       { Opt_wsize, "wsize=%s" },
>>       { Opt_actimeo, "actimeo=%s" },
>> +     { Opt_echo_interval, "echo_interval=%s" },
>>
>>       { Opt_blank_user, "user=" },
>>       { Opt_blank_user, "username=" },
>> @@ -418,6 +420,7 @@ 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;
>>
>>       /*
>>        * We cannot send an echo if it is disabled or until the
>> @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
>>        */
>>       if (!server->ops->need_neg || server->ops->need_neg(server)
>> ||
>>           (server->ops->can_echo && !server->ops
>> ->can_echo(server)) ||
>> -         time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL -
>> HZ))
>> +         time_before(jiffies, server->lstrp + echo_interval -
>> HZ))
>>               goto requeue_echo;
>>
>>       rc = server->ops->echo ? server->ops->echo(server) :
>> -ENOSYS;
>> @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
>>                        server->hostname);
>>
>>  requeue_echo:
>> -     queue_delayed_work(cifsiod_wq, &server->echo,
>> SMB_ECHO_INTERVAL);
>> +     queue_delayed_work(cifsiod_wq, &server->echo,
>> echo_interval);
>>  }
>>
>>  static bool
>> @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info
>> *server)
>>        *     a response in >60s.
>>        */
>>       if (server->tcpStatus == CifsGood &&
>> -         time_after(jiffies, server->lstrp + 2 *
>> SMB_ECHO_INTERVAL)) {
>> -             cifs_dbg(VFS, "Server %s has not responded in %d
>> seconds. Reconnecting...\n",
>> -                      server->hostname, (2 * SMB_ECHO_INTERVAL) /
>> HZ);
>> +         time_after(jiffies, server->lstrp + 2 * server
>> ->echo_interval)) {
>> +             cifs_dbg(VFS, "Server %s has not responded in %lu
>> seconds. Reconnecting...\n",
>> +                      server->hostname, (2 * server
>> ->echo_interval) / HZ);
>>               cifs_reconnect(server);
>>               wake_up(&server->response_q);
>>               return true;
>> @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char
>> *mountdata, const char *devname,
>>                               goto cifs_parse_mount_err;
>>                       }
>>                       break;
>> +             case Opt_echo_interval:
>> +                     if (get_option_ul(args, &option)) {
>> +                             cifs_dbg(VFS, "%s: Invalid echo
>> interval value\n",
>> +                                      __func__);
>> +                             goto cifs_parse_mount_err;
>> +                     }
>> +                     vol->echo_interval = option;
>> +                     break;
>>
>>               /* String Arguments */
>>
>> @@ -2089,6 +2100,9 @@ static int match_server(struct TCP_Server_Info
>> *server, struct smb_vol *vol)
>>       if (!match_security(server, vol))
>>               return 0;
>>
>> +     if (server->echo_interval != vol->echo_interval)
>> +             return 0;
>> +
>>       return 1;
>>  }
>>
>> @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol
>> *volume_info)
>>       tcp_ses->tcpStatus = CifsNew;
>>       ++tcp_ses->srv_count;
>>
>> +     /* echo interval should be between 10 and 120 secs */
>> +     if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
>> +             volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
>> +             tcp_ses->echo_interval = volume_info->echo_interval
>> * HZ;
>> +     else
>> +             tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT *
>> HZ;
>> +
>>       rc = ip_connect(tcp_ses);
>>       if (rc < 0) {
>>               cifs_dbg(VFS, "Error connecting to socket. Aborting
>> operation.\n");
>> @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol
>> *volume_info)
>>       cifs_fscache_get_client_cookie(tcp_ses);
>>
>>       /* queue echo request delayed work */
>> -     queue_delayed_work(cifsiod_wq, &tcp_ses->echo,
>> SMB_ECHO_INTERVAL);
>> +     queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses
>> ->echo_interval);
>>
>>       return tcp_ses;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sachin Prabhu Dec. 15, 2015, 10:21 a.m. UTC | #3
On Mon, 2015-12-14 at 21:58 -0600, Shirish Pargaonkar wrote:
> Looks correct. Only comment would be to keep one definition of
> echo_interval,
> preferably unsigned int (instead of unsigned long also).

Sirish,

The event_timeout defined in struct smb_vol contains the timeout value
in seconds while the one defined in struct TCP_Server_Info multiplies
the value from the smb_vol by HZ to get the time in ticks.

Sachin Prabhu

> 
> On Mon, Dec 14, 2015 at 6:54 AM, Sachin Prabhu <sprabhu@redhat.com>
> wrote:
> > Another approach as pointed to by Jeff Layton is to make it a
> > module
> > parameter which is then set for all shares mounted on the client.
> > 
> > Any comments on the patch are welcome.
> > 
> > Sachin Prabhu
> > 
> > On Mon, 2015-12-14 at 18:17 +0530, Sachin Prabhu wrote:
> > > Currently the echo interval is set to 60 seconds using a macro.
> > > This
> > > setting determines the interval at which echo requests are sent
> > > to
> > > the
> > > server on an idling connection. This setting also affects the
> > > time
> > > required for a connection to an unresponsive server to timeout.
> > > 
> > > Making this setting a tunable allows users to control the echo
> > > interval
> > > times as well as control the time after which the connecting to
> > > an
> > > unresponsive server times out.
> > > 
> > > Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> > > ---
> > >  fs/cifs/cifsfs.c   |  2 ++
> > >  fs/cifs/cifsglob.h |  8 ++++++--
> > >  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
> > >  3 files changed, 35 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > > index cbc0f4b..eab2de6 100644
> > > --- a/fs/cifs/cifsfs.c
> > > +++ b/fs/cifs/cifsfs.c
> > > @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct
> > > dentry *root)
> > > 
> > >       seq_printf(s, ",rsize=%u", cifs_sb->rsize);
> > >       seq_printf(s, ",wsize=%u", cifs_sb->wsize);
> > > +     seq_printf(s, ",echo_interval=%lu",
> > > +                     tcon->ses->server->echo_interval / HZ);
> > >       /* convert actimeo and display it in seconds */
> > >       seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
> > > 
> > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > > index 2b510c5..56d3698 100644
> > > --- a/fs/cifs/cifsglob.h
> > > +++ b/fs/cifs/cifsglob.h
> > > @@ -70,8 +70,10 @@
> > >  #define SERVER_NAME_LENGTH 40
> > >  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
> > > 
> > > -/* SMB echo "timeout" -- FIXME: tunable? */
> > > -#define SMB_ECHO_INTERVAL (60 * HZ)
> > > +/* echo interval in seconds */
> > > +#define SMB_ECHO_INTERVAL_MIN 10
> > > +#define SMB_ECHO_INTERVAL_MAX 120
> > > +#define SMB_ECHO_INTERVAL_DEFAULT 60
> > > 
> > >  #include "cifspdu.h"
> > > 
> > > @@ -507,6 +509,7 @@ struct smb_vol {
> > >       struct sockaddr_storage dstaddr; /* destination address */
> > >       struct sockaddr_storage srcaddr; /* allow binding to a
> > > local
> > > IP */
> > >       struct nls_table *local_nls;
> > > +     unsigned int echo_interval; /* echo interval in secs */
> > >  };
> > > 
> > >  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID
> > > | \
> > > @@ -628,6 +631,7 @@ struct TCP_Server_Info {
> > >       unsigned int    max_read;
> > >       unsigned int    max_write;
> > >  #endif /* CONFIG_CIFS_SMB2 */
> > > +     unsigned long echo_interval;
> > >  };
> > > 
> > >  static inline unsigned int
> > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > > index ecb0803..3b44e9e 100644
> > > --- a/fs/cifs/connect.c
> > > +++ b/fs/cifs/connect.c
> > > @@ -95,6 +95,7 @@ enum {
> > >       Opt_cruid, Opt_gid, Opt_file_mode,
> > >       Opt_dirmode, Opt_port,
> > >       Opt_rsize, Opt_wsize, Opt_actimeo,
> > > +     Opt_echo_interval,
> > > 
> > >       /* Mount options which take string value */
> > >       Opt_user, Opt_pass, Opt_ip,
> > > @@ -188,6 +189,7 @@ static const match_table_t
> > > cifs_mount_option_tokens = {
> > >       { Opt_rsize, "rsize=%s" },
> > >       { Opt_wsize, "wsize=%s" },
> > >       { Opt_actimeo, "actimeo=%s" },
> > > +     { Opt_echo_interval, "echo_interval=%s" },
> > > 
> > >       { Opt_blank_user, "user=" },
> > >       { Opt_blank_user, "username=" },
> > > @@ -418,6 +420,7 @@ 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;
> > > 
> > >       /*
> > >        * We cannot send an echo if it is disabled or until the
> > > @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
> > >        */
> > >       if (!server->ops->need_neg || server->ops->need_neg(server)
> > > > > 
> > >           (server->ops->can_echo && !server->ops
> > > ->can_echo(server)) ||
> > > -         time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL
> > > -
> > > HZ))
> > > +         time_before(jiffies, server->lstrp + echo_interval -
> > > HZ))
> > >               goto requeue_echo;
> > > 
> > >       rc = server->ops->echo ? server->ops->echo(server) :
> > > -ENOSYS;
> > > @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
> > >                        server->hostname);
> > > 
> > >  requeue_echo:
> > > -     queue_delayed_work(cifsiod_wq, &server->echo,
> > > SMB_ECHO_INTERVAL);
> > > +     queue_delayed_work(cifsiod_wq, &server->echo,
> > > echo_interval);
> > >  }
> > > 
> > >  static bool
> > > @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info
> > > *server)
> > >        *     a response in >60s.
> > >        */
> > >       if (server->tcpStatus == CifsGood &&
> > > -         time_after(jiffies, server->lstrp + 2 *
> > > SMB_ECHO_INTERVAL)) {
> > > -             cifs_dbg(VFS, "Server %s has not responded in %d
> > > seconds. Reconnecting...\n",
> > > -                      server->hostname, (2 * SMB_ECHO_INTERVAL)
> > > /
> > > HZ);
> > > +         time_after(jiffies, server->lstrp + 2 * server
> > > ->echo_interval)) {
> > > +             cifs_dbg(VFS, "Server %s has not responded in %lu
> > > seconds. Reconnecting...\n",
> > > +                      server->hostname, (2 * server
> > > ->echo_interval) / HZ);
> > >               cifs_reconnect(server);
> > >               wake_up(&server->response_q);
> > >               return true;
> > > @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char
> > > *mountdata, const char *devname,
> > >                               goto cifs_parse_mount_err;
> > >                       }
> > >                       break;
> > > +             case Opt_echo_interval:
> > > +                     if (get_option_ul(args, &option)) {
> > > +                             cifs_dbg(VFS, "%s: Invalid echo
> > > interval value\n",
> > > +                                      __func__);
> > > +                             goto cifs_parse_mount_err;
> > > +                     }
> > > +                     vol->echo_interval = option;
> > > +                     break;
> > > 
> > >               /* String Arguments */
> > > 
> > > @@ -2089,6 +2100,9 @@ static int match_server(struct
> > > TCP_Server_Info
> > > *server, struct smb_vol *vol)
> > >       if (!match_security(server, vol))
> > >               return 0;
> > > 
> > > +     if (server->echo_interval != vol->echo_interval)
> > > +             return 0;
> > > +
> > >       return 1;
> > >  }
> > > 
> > > @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol
> > > *volume_info)
> > >       tcp_ses->tcpStatus = CifsNew;
> > >       ++tcp_ses->srv_count;
> > > 
> > > +     /* echo interval should be between 10 and 120 secs */
> > > +     if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
> > > +             volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
> > > +             tcp_ses->echo_interval = volume_info->echo_interval
> > > * HZ;
> > > +     else
> > > +             tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT
> > > *
> > > HZ;
> > > +
> > >       rc = ip_connect(tcp_ses);
> > >       if (rc < 0) {
> > >               cifs_dbg(VFS, "Error connecting to socket. Aborting
> > > operation.\n");
> > > @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol
> > > *volume_info)
> > >       cifs_fscache_get_client_cookie(tcp_ses);
> > > 
> > >       /* queue echo request delayed work */
> > > -     queue_delayed_work(cifsiod_wq, &tcp_ses->echo,
> > > SMB_ECHO_INTERVAL);
> > > +     queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses
> > > ->echo_interval);
> > > 
> > >       return tcp_ses;
> > > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > cifs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shirish Pargaonkar Dec. 17, 2015, 3:54 a.m. UTC | #4
Acked-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>

On Tue, Dec 15, 2015 at 4:21 AM, Sachin Prabhu <sprabhu@redhat.com> wrote:
> On Mon, 2015-12-14 at 21:58 -0600, Shirish Pargaonkar wrote:
>> Looks correct. Only comment would be to keep one definition of
>> echo_interval,
>> preferably unsigned int (instead of unsigned long also).
>
> Sirish,
>
> The event_timeout defined in struct smb_vol contains the timeout value
> in seconds while the one defined in struct TCP_Server_Info multiplies
> the value from the smb_vol by HZ to get the time in ticks.
>
> Sachin Prabhu
>
>>
>> On Mon, Dec 14, 2015 at 6:54 AM, Sachin Prabhu <sprabhu@redhat.com>
>> wrote:
>> > Another approach as pointed to by Jeff Layton is to make it a
>> > module
>> > parameter which is then set for all shares mounted on the client.
>> >
>> > Any comments on the patch are welcome.
>> >
>> > Sachin Prabhu
>> >
>> > On Mon, 2015-12-14 at 18:17 +0530, Sachin Prabhu wrote:
>> > > Currently the echo interval is set to 60 seconds using a macro.
>> > > This
>> > > setting determines the interval at which echo requests are sent
>> > > to
>> > > the
>> > > server on an idling connection. This setting also affects the
>> > > time
>> > > required for a connection to an unresponsive server to timeout.
>> > >
>> > > Making this setting a tunable allows users to control the echo
>> > > interval
>> > > times as well as control the time after which the connecting to
>> > > an
>> > > unresponsive server times out.
>> > >
>> > > Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
>> > > ---
>> > >  fs/cifs/cifsfs.c   |  2 ++
>> > >  fs/cifs/cifsglob.h |  8 ++++++--
>> > >  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
>> > >  3 files changed, 35 insertions(+), 8 deletions(-)
>> > >
>> > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> > > index cbc0f4b..eab2de6 100644
>> > > --- a/fs/cifs/cifsfs.c
>> > > +++ b/fs/cifs/cifsfs.c
>> > > @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct
>> > > dentry *root)
>> > >
>> > >       seq_printf(s, ",rsize=%u", cifs_sb->rsize);
>> > >       seq_printf(s, ",wsize=%u", cifs_sb->wsize);
>> > > +     seq_printf(s, ",echo_interval=%lu",
>> > > +                     tcon->ses->server->echo_interval / HZ);
>> > >       /* convert actimeo and display it in seconds */
>> > >       seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
>> > >
>> > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> > > index 2b510c5..56d3698 100644
>> > > --- a/fs/cifs/cifsglob.h
>> > > +++ b/fs/cifs/cifsglob.h
>> > > @@ -70,8 +70,10 @@
>> > >  #define SERVER_NAME_LENGTH 40
>> > >  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
>> > >
>> > > -/* SMB echo "timeout" -- FIXME: tunable? */
>> > > -#define SMB_ECHO_INTERVAL (60 * HZ)
>> > > +/* echo interval in seconds */
>> > > +#define SMB_ECHO_INTERVAL_MIN 10
>> > > +#define SMB_ECHO_INTERVAL_MAX 120
>> > > +#define SMB_ECHO_INTERVAL_DEFAULT 60
>> > >
>> > >  #include "cifspdu.h"
>> > >
>> > > @@ -507,6 +509,7 @@ struct smb_vol {
>> > >       struct sockaddr_storage dstaddr; /* destination address */
>> > >       struct sockaddr_storage srcaddr; /* allow binding to a
>> > > local
>> > > IP */
>> > >       struct nls_table *local_nls;
>> > > +     unsigned int echo_interval; /* echo interval in secs */
>> > >  };
>> > >
>> > >  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID
>> > > | \
>> > > @@ -628,6 +631,7 @@ struct TCP_Server_Info {
>> > >       unsigned int    max_read;
>> > >       unsigned int    max_write;
>> > >  #endif /* CONFIG_CIFS_SMB2 */
>> > > +     unsigned long echo_interval;
>> > >  };
>> > >
>> > >  static inline unsigned int
>> > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > > index ecb0803..3b44e9e 100644
>> > > --- a/fs/cifs/connect.c
>> > > +++ b/fs/cifs/connect.c
>> > > @@ -95,6 +95,7 @@ enum {
>> > >       Opt_cruid, Opt_gid, Opt_file_mode,
>> > >       Opt_dirmode, Opt_port,
>> > >       Opt_rsize, Opt_wsize, Opt_actimeo,
>> > > +     Opt_echo_interval,
>> > >
>> > >       /* Mount options which take string value */
>> > >       Opt_user, Opt_pass, Opt_ip,
>> > > @@ -188,6 +189,7 @@ static const match_table_t
>> > > cifs_mount_option_tokens = {
>> > >       { Opt_rsize, "rsize=%s" },
>> > >       { Opt_wsize, "wsize=%s" },
>> > >       { Opt_actimeo, "actimeo=%s" },
>> > > +     { Opt_echo_interval, "echo_interval=%s" },
>> > >
>> > >       { Opt_blank_user, "user=" },
>> > >       { Opt_blank_user, "username=" },
>> > > @@ -418,6 +420,7 @@ 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;
>> > >
>> > >       /*
>> > >        * We cannot send an echo if it is disabled or until the
>> > > @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
>> > >        */
>> > >       if (!server->ops->need_neg || server->ops->need_neg(server)
>> > > > >
>> > >           (server->ops->can_echo && !server->ops
>> > > ->can_echo(server)) ||
>> > > -         time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL
>> > > -
>> > > HZ))
>> > > +         time_before(jiffies, server->lstrp + echo_interval -
>> > > HZ))
>> > >               goto requeue_echo;
>> > >
>> > >       rc = server->ops->echo ? server->ops->echo(server) :
>> > > -ENOSYS;
>> > > @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
>> > >                        server->hostname);
>> > >
>> > >  requeue_echo:
>> > > -     queue_delayed_work(cifsiod_wq, &server->echo,
>> > > SMB_ECHO_INTERVAL);
>> > > +     queue_delayed_work(cifsiod_wq, &server->echo,
>> > > echo_interval);
>> > >  }
>> > >
>> > >  static bool
>> > > @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info
>> > > *server)
>> > >        *     a response in >60s.
>> > >        */
>> > >       if (server->tcpStatus == CifsGood &&
>> > > -         time_after(jiffies, server->lstrp + 2 *
>> > > SMB_ECHO_INTERVAL)) {
>> > > -             cifs_dbg(VFS, "Server %s has not responded in %d
>> > > seconds. Reconnecting...\n",
>> > > -                      server->hostname, (2 * SMB_ECHO_INTERVAL)
>> > > /
>> > > HZ);
>> > > +         time_after(jiffies, server->lstrp + 2 * server
>> > > ->echo_interval)) {
>> > > +             cifs_dbg(VFS, "Server %s has not responded in %lu
>> > > seconds. Reconnecting...\n",
>> > > +                      server->hostname, (2 * server
>> > > ->echo_interval) / HZ);
>> > >               cifs_reconnect(server);
>> > >               wake_up(&server->response_q);
>> > >               return true;
>> > > @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char
>> > > *mountdata, const char *devname,
>> > >                               goto cifs_parse_mount_err;
>> > >                       }
>> > >                       break;
>> > > +             case Opt_echo_interval:
>> > > +                     if (get_option_ul(args, &option)) {
>> > > +                             cifs_dbg(VFS, "%s: Invalid echo
>> > > interval value\n",
>> > > +                                      __func__);
>> > > +                             goto cifs_parse_mount_err;
>> > > +                     }
>> > > +                     vol->echo_interval = option;
>> > > +                     break;
>> > >
>> > >               /* String Arguments */
>> > >
>> > > @@ -2089,6 +2100,9 @@ static int match_server(struct
>> > > TCP_Server_Info
>> > > *server, struct smb_vol *vol)
>> > >       if (!match_security(server, vol))
>> > >               return 0;
>> > >
>> > > +     if (server->echo_interval != vol->echo_interval)
>> > > +             return 0;
>> > > +
>> > >       return 1;
>> > >  }
>> > >
>> > > @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol
>> > > *volume_info)
>> > >       tcp_ses->tcpStatus = CifsNew;
>> > >       ++tcp_ses->srv_count;
>> > >
>> > > +     /* echo interval should be between 10 and 120 secs */
>> > > +     if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
>> > > +             volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
>> > > +             tcp_ses->echo_interval = volume_info->echo_interval
>> > > * HZ;
>> > > +     else
>> > > +             tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT
>> > > *
>> > > HZ;
>> > > +
>> > >       rc = ip_connect(tcp_ses);
>> > >       if (rc < 0) {
>> > >               cifs_dbg(VFS, "Error connecting to socket. Aborting
>> > > operation.\n");
>> > > @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol
>> > > *volume_info)
>> > >       cifs_fscache_get_client_cookie(tcp_ses);
>> > >
>> > >       /* queue echo request delayed work */
>> > > -     queue_delayed_work(cifsiod_wq, &tcp_ses->echo,
>> > > SMB_ECHO_INTERVAL);
>> > > +     queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses
>> > > ->echo_interval);
>> > >
>> > >       return tcp_ses;
>> > >
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-
>> > cifs" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French Dec. 17, 2015, 4:37 a.m. UTC | #5
Why restrict the echo interval to such a narrow range?  If tunable
might as well allow a larger range, maybe 1 to 600 (ten minutes)?  I
am not certain of the range of possible future uses - maybe a
pseudo-real type workload connected to a really fast server wants
something really low (a few seconds?) and maybe very poor network
connections want something longer?  Not a strong objection, but seems
like we could allow a broader range without confusing the user.

Many years ago, Windows and OS/2 had an oplock break time (which also
controlled how long to wait for acks to come back from the server) -
and it ranged from 34 to 127 seconds for acks and 35 to 640 for oplock
break timeouts.     Don't see any reason to limit it less than that
(that was really old and networks vary a lot in performance now).

Thoughts?

On Mon, Dec 14, 2015 at 6:47 AM, Sachin Prabhu <sprabhu@redhat.com> wrote:
> Currently the echo interval is set to 60 seconds using a macro. This
> setting determines the interval at which echo requests are sent to the
> server on an idling connection. This setting also affects the time
> required for a connection to an unresponsive server to timeout.
>
> Making this setting a tunable allows users to control the echo interval
> times as well as control the time after which the connecting to an
> unresponsive server times out.
>
> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> ---
>  fs/cifs/cifsfs.c   |  2 ++
>  fs/cifs/cifsglob.h |  8 ++++++--
>  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
>  3 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index cbc0f4b..eab2de6 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
>
>         seq_printf(s, ",rsize=%u", cifs_sb->rsize);
>         seq_printf(s, ",wsize=%u", cifs_sb->wsize);
> +       seq_printf(s, ",echo_interval=%lu",
> +                       tcon->ses->server->echo_interval / HZ);
>         /* convert actimeo and display it in seconds */
>         seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 2b510c5..56d3698 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -70,8 +70,10 @@
>  #define SERVER_NAME_LENGTH 40
>  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
>
> -/* SMB echo "timeout" -- FIXME: tunable? */
> -#define SMB_ECHO_INTERVAL (60 * HZ)
> +/* echo interval in seconds */
> +#define SMB_ECHO_INTERVAL_MIN 10
> +#define SMB_ECHO_INTERVAL_MAX 120
> +#define SMB_ECHO_INTERVAL_DEFAULT 60
>
>  #include "cifspdu.h"
>
> @@ -507,6 +509,7 @@ struct smb_vol {
>         struct sockaddr_storage dstaddr; /* destination address */
>         struct sockaddr_storage srcaddr; /* allow binding to a local IP */
>         struct nls_table *local_nls;
> +       unsigned int echo_interval; /* echo interval in secs */
>  };
>
>  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID | \
> @@ -628,6 +631,7 @@ struct TCP_Server_Info {
>         unsigned int    max_read;
>         unsigned int    max_write;
>  #endif /* CONFIG_CIFS_SMB2 */
> +       unsigned long echo_interval;
>  };
>
>  static inline unsigned int
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ecb0803..3b44e9e 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -95,6 +95,7 @@ enum {
>         Opt_cruid, Opt_gid, Opt_file_mode,
>         Opt_dirmode, Opt_port,
>         Opt_rsize, Opt_wsize, Opt_actimeo,
> +       Opt_echo_interval,
>
>         /* Mount options which take string value */
>         Opt_user, Opt_pass, Opt_ip,
> @@ -188,6 +189,7 @@ static const match_table_t cifs_mount_option_tokens = {
>         { Opt_rsize, "rsize=%s" },
>         { Opt_wsize, "wsize=%s" },
>         { Opt_actimeo, "actimeo=%s" },
> +       { Opt_echo_interval, "echo_interval=%s" },
>
>         { Opt_blank_user, "user=" },
>         { Opt_blank_user, "username=" },
> @@ -418,6 +420,7 @@ 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;
>
>         /*
>          * We cannot send an echo if it is disabled or until the
> @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
>          */
>         if (!server->ops->need_neg || server->ops->need_neg(server) ||
>             (server->ops->can_echo && !server->ops->can_echo(server)) ||
> -           time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ))
> +           time_before(jiffies, server->lstrp + echo_interval - HZ))
>                 goto requeue_echo;
>
>         rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
> @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
>                          server->hostname);
>
>  requeue_echo:
> -       queue_delayed_work(cifsiod_wq, &server->echo, SMB_ECHO_INTERVAL);
> +       queue_delayed_work(cifsiod_wq, &server->echo, echo_interval);
>  }
>
>  static bool
> @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info *server)
>          *     a response in >60s.
>          */
>         if (server->tcpStatus == CifsGood &&
> -           time_after(jiffies, server->lstrp + 2 * SMB_ECHO_INTERVAL)) {
> -               cifs_dbg(VFS, "Server %s has not responded in %d seconds. Reconnecting...\n",
> -                        server->hostname, (2 * SMB_ECHO_INTERVAL) / HZ);
> +           time_after(jiffies, server->lstrp + 2 * server->echo_interval)) {
> +               cifs_dbg(VFS, "Server %s has not responded in %lu seconds. Reconnecting...\n",
> +                        server->hostname, (2 * server->echo_interval) / HZ);
>                 cifs_reconnect(server);
>                 wake_up(&server->response_q);
>                 return true;
> @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                                 goto cifs_parse_mount_err;
>                         }
>                         break;
> +               case Opt_echo_interval:
> +                       if (get_option_ul(args, &option)) {
> +                               cifs_dbg(VFS, "%s: Invalid echo interval value\n",
> +                                        __func__);
> +                               goto cifs_parse_mount_err;
> +                       }
> +                       vol->echo_interval = option;
> +                       break;
>
>                 /* String Arguments */
>
> @@ -2089,6 +2100,9 @@ static int match_server(struct TCP_Server_Info *server, struct smb_vol *vol)
>         if (!match_security(server, vol))
>                 return 0;
>
> +       if (server->echo_interval != vol->echo_interval)
> +               return 0;
> +
>         return 1;
>  }
>
> @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>         tcp_ses->tcpStatus = CifsNew;
>         ++tcp_ses->srv_count;
>
> +       /* echo interval should be between 10 and 120 secs */
> +       if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
> +               volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
> +               tcp_ses->echo_interval = volume_info->echo_interval * HZ;
> +       else
> +               tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT * HZ;
> +
>         rc = ip_connect(tcp_ses);
>         if (rc < 0) {
>                 cifs_dbg(VFS, "Error connecting to socket. Aborting operation.\n");
> @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>         cifs_fscache_get_client_cookie(tcp_ses);
>
>         /* queue echo request delayed work */
> -       queue_delayed_work(cifsiod_wq, &tcp_ses->echo, SMB_ECHO_INTERVAL);
> +       queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
>
>         return tcp_ses;
>
> --
> 2.4.3
>
Sachin Prabhu Dec. 17, 2015, 5:28 a.m. UTC | #6
On Wed, 2015-12-16 at 22:37 -0600, Steve French wrote:
> Why restrict the echo interval to such a narrow range?  If tunable
> might as well allow a larger range, maybe 1 to 600 (ten minutes)?  I
> am not certain of the range of possible future uses - maybe a
> pseudo-real type workload connected to a really fast server wants
> something really low (a few seconds?) and maybe very poor network
> connections want something longer?  Not a strong objection, but seems
> like we could allow a broader range without confusing the user.
> 
> Many years ago, Windows and OS/2 had an oplock break time (which also
> controlled how long to wait for acks to come back from the server) -
> and it ranged from 34 to 127 seconds for acks and 35 to 640 for
> oplock
> break timeouts.     Don't see any reason to limit it less than that
> (that was really old and networks vary a lot in performance now).
> 
> Thoughts?

the numbers I chose were arbitrary. I have no objections to changing
the min and max values. Do you want me to send a new patch with those
values?

Sachin Prabhu

> 
> On Mon, Dec 14, 2015 at 6:47 AM, Sachin Prabhu <sprabhu@redhat.com>
> wrote:
> > Currently the echo interval is set to 60 seconds using a macro.
> > This
> > setting determines the interval at which echo requests are sent to
> > the
> > server on an idling connection. This setting also affects the time
> > required for a connection to an unresponsive server to timeout.
> > 
> > Making this setting a tunable allows users to control the echo
> > interval
> > times as well as control the time after which the connecting to an
> > unresponsive server times out.
> > 
> > Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> > ---
> >  fs/cifs/cifsfs.c   |  2 ++
> >  fs/cifs/cifsglob.h |  8 ++++++--
> >  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
> >  3 files changed, 35 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index cbc0f4b..eab2de6 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct
> > dentry *root)
> > 
> >         seq_printf(s, ",rsize=%u", cifs_sb->rsize);
> >         seq_printf(s, ",wsize=%u", cifs_sb->wsize);
> > +       seq_printf(s, ",echo_interval=%lu",
> > +                       tcon->ses->server->echo_interval / HZ);
> >         /* convert actimeo and display it in seconds */
> >         seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
> > 
> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > index 2b510c5..56d3698 100644
> > --- a/fs/cifs/cifsglob.h
> > +++ b/fs/cifs/cifsglob.h
> > @@ -70,8 +70,10 @@
> >  #define SERVER_NAME_LENGTH 40
> >  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
> > 
> > -/* SMB echo "timeout" -- FIXME: tunable? */
> > -#define SMB_ECHO_INTERVAL (60 * HZ)
> > +/* echo interval in seconds */
> > +#define SMB_ECHO_INTERVAL_MIN 10
> > +#define SMB_ECHO_INTERVAL_MAX 120
> > +#define SMB_ECHO_INTERVAL_DEFAULT 60
> > 
> >  #include "cifspdu.h"
> > 
> > @@ -507,6 +509,7 @@ struct smb_vol {
> >         struct sockaddr_storage dstaddr; /* destination address */
> >         struct sockaddr_storage srcaddr; /* allow binding to a
> > local IP */
> >         struct nls_table *local_nls;
> > +       unsigned int echo_interval; /* echo interval in secs */
> >  };
> > 
> >  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
> > \
> > @@ -628,6 +631,7 @@ struct TCP_Server_Info {
> >         unsigned int    max_read;
> >         unsigned int    max_write;
> >  #endif /* CONFIG_CIFS_SMB2 */
> > +       unsigned long echo_interval;
> >  };
> > 
> >  static inline unsigned int
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index ecb0803..3b44e9e 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -95,6 +95,7 @@ enum {
> >         Opt_cruid, Opt_gid, Opt_file_mode,
> >         Opt_dirmode, Opt_port,
> >         Opt_rsize, Opt_wsize, Opt_actimeo,
> > +       Opt_echo_interval,
> > 
> >         /* Mount options which take string value */
> >         Opt_user, Opt_pass, Opt_ip,
> > @@ -188,6 +189,7 @@ static const match_table_t
> > cifs_mount_option_tokens = {
> >         { Opt_rsize, "rsize=%s" },
> >         { Opt_wsize, "wsize=%s" },
> >         { Opt_actimeo, "actimeo=%s" },
> > +       { Opt_echo_interval, "echo_interval=%s" },
> > 
> >         { Opt_blank_user, "user=" },
> >         { Opt_blank_user, "username=" },
> > @@ -418,6 +420,7 @@ 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;
> > 
> >         /*
> >          * We cannot send an echo if it is disabled or until the
> > @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
> >          */
> >         if (!server->ops->need_neg || server->ops->need_neg(server) 
> > ||
> >             (server->ops->can_echo && !server->ops-
> > >can_echo(server)) ||
> > -           time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL
> > - HZ))
> > +           time_before(jiffies, server->lstrp + echo_interval -
> > HZ))
> >                 goto requeue_echo;
> > 
> >         rc = server->ops->echo ? server->ops->echo(server) :
> > -ENOSYS;
> > @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
> >                          server->hostname);
> > 
> >  requeue_echo:
> > -       queue_delayed_work(cifsiod_wq, &server->echo,
> > SMB_ECHO_INTERVAL);
> > +       queue_delayed_work(cifsiod_wq, &server->echo,
> > echo_interval);
> >  }
> > 
> >  static bool
> > @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info
> > *server)
> >          *     a response in >60s.
> >          */
> >         if (server->tcpStatus == CifsGood &&
> > -           time_after(jiffies, server->lstrp + 2 *
> > SMB_ECHO_INTERVAL)) {
> > -               cifs_dbg(VFS, "Server %s has not responded in %d
> > seconds. Reconnecting...\n",
> > -                        server->hostname, (2 * SMB_ECHO_INTERVAL)
> > / HZ);
> > +           time_after(jiffies, server->lstrp + 2 * server-
> > >echo_interval)) {
> > +               cifs_dbg(VFS, "Server %s has not responded in %lu
> > seconds. Reconnecting...\n",
> > +                        server->hostname, (2 * server-
> > >echo_interval) / HZ);
> >                 cifs_reconnect(server);
> >                 wake_up(&server->response_q);
> >                 return true;
> > @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char
> > *mountdata, const char *devname,
> >                                 goto cifs_parse_mount_err;
> >                         }
> >                         break;
> > +               case Opt_echo_interval:
> > +                       if (get_option_ul(args, &option)) {
> > +                               cifs_dbg(VFS, "%s: Invalid echo
> > interval value\n",
> > +                                        __func__);
> > +                               goto cifs_parse_mount_err;
> > +                       }
> > +                       vol->echo_interval = option;
> > +                       break;
> > 
> >                 /* String Arguments */
> > 
> > @@ -2089,6 +2100,9 @@ static int match_server(struct
> > TCP_Server_Info *server, struct smb_vol *vol)
> >         if (!match_security(server, vol))
> >                 return 0;
> > 
> > +       if (server->echo_interval != vol->echo_interval)
> > +               return 0;
> > +
> >         return 1;
> >  }
> > 
> > @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol
> > *volume_info)
> >         tcp_ses->tcpStatus = CifsNew;
> >         ++tcp_ses->srv_count;
> > 
> > +       /* echo interval should be between 10 and 120 secs */
> > +       if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
> > +               volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
> > +               tcp_ses->echo_interval = volume_info->echo_interval 
> > * HZ;
> > +       else
> > +               tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT
> > * HZ;
> > +
> >         rc = ip_connect(tcp_ses);
> >         if (rc < 0) {
> >                 cifs_dbg(VFS, "Error connecting to socket. Aborting
> > operation.\n");
> > @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol
> > *volume_info)
> >         cifs_fscache_get_client_cookie(tcp_ses);
> > 
> >         /* queue echo request delayed work */
> > -       queue_delayed_work(cifsiod_wq, &tcp_ses->echo,
> > SMB_ECHO_INTERVAL);
> > +       queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses-
> > >echo_interval);
> > 
> >         return tcp_ses;
> > 
> > --
> > 2.4.3
> > 
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French Dec. 17, 2015, 7:04 a.m. UTC | #7
On Wed, Dec 16, 2015 at 11:28 PM, Sachin Prabhu <sprabhu@redhat.com> wrote:
> On Wed, 2015-12-16 at 22:37 -0600, Steve French wrote:
>> Why restrict the echo interval to such a narrow range?  If tunable
>> might as well allow a larger range, maybe 1 to 600 (ten minutes)?  I
>> am not certain of the range of possible future uses - maybe a
>> pseudo-real type workload connected to a really fast server wants
>> something really low (a few seconds?) and maybe very poor network
>> connections want something longer?  Not a strong objection, but seems
>> like we could allow a broader range without confusing the user.
>>
>> Many years ago, Windows and OS/2 had an oplock break time (which also
>> controlled how long to wait for acks to come back from the server) -
>> and it ranged from 34 to 127 seconds for acks and 35 to 640 for
>> oplock
>> break timeouts.     Don't see any reason to limit it less than that
>> (that was really old and networks vary a lot in performance now).
>>
>> Thoughts?
>
> the numbers I chose were arbitrary. I have no objections to changing
> the min and max values. Do you want me to send a new patch with those
> values?
>
> Sachin Prabhu

Yes - then I plan to merge unless any objections.

>>
>> On Mon, Dec 14, 2015 at 6:47 AM, Sachin Prabhu <sprabhu@redhat.com>
>> wrote:
>> > Currently the echo interval is set to 60 seconds using a macro.
>> > This
>> > setting determines the interval at which echo requests are sent to
>> > the
>> > server on an idling connection. This setting also affects the time
>> > required for a connection to an unresponsive server to timeout.
>> >
>> > Making this setting a tunable allows users to control the echo
>> > interval
>> > times as well as control the time after which the connecting to an
>> > unresponsive server times out.
>> >
>> > Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
>> > ---
>> >  fs/cifs/cifsfs.c   |  2 ++
>> >  fs/cifs/cifsglob.h |  8 ++++++--
>> >  fs/cifs/connect.c  | 33 +++++++++++++++++++++++++++------
>> >  3 files changed, 35 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> > index cbc0f4b..eab2de6 100644
>> > --- a/fs/cifs/cifsfs.c
>> > +++ b/fs/cifs/cifsfs.c
>> > @@ -507,6 +507,8 @@ cifs_show_options(struct seq_file *s, struct
>> > dentry *root)
>> >
>> >         seq_printf(s, ",rsize=%u", cifs_sb->rsize);
>> >         seq_printf(s, ",wsize=%u", cifs_sb->wsize);
>> > +       seq_printf(s, ",echo_interval=%lu",
>> > +                       tcon->ses->server->echo_interval / HZ);
>> >         /* convert actimeo and display it in seconds */
>> >         seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
>> >
>> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> > index 2b510c5..56d3698 100644
>> > --- a/fs/cifs/cifsglob.h
>> > +++ b/fs/cifs/cifsglob.h
>> > @@ -70,8 +70,10 @@
>> >  #define SERVER_NAME_LENGTH 40
>> >  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
>> >
>> > -/* SMB echo "timeout" -- FIXME: tunable? */
>> > -#define SMB_ECHO_INTERVAL (60 * HZ)
>> > +/* echo interval in seconds */
>> > +#define SMB_ECHO_INTERVAL_MIN 10
>> > +#define SMB_ECHO_INTERVAL_MAX 120
>> > +#define SMB_ECHO_INTERVAL_DEFAULT 60
>> >
>> >  #include "cifspdu.h"
>> >
>> > @@ -507,6 +509,7 @@ struct smb_vol {
>> >         struct sockaddr_storage dstaddr; /* destination address */
>> >         struct sockaddr_storage srcaddr; /* allow binding to a
>> > local IP */
>> >         struct nls_table *local_nls;
>> > +       unsigned int echo_interval; /* echo interval in secs */
>> >  };
>> >
>> >  #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
>> > \
>> > @@ -628,6 +631,7 @@ struct TCP_Server_Info {
>> >         unsigned int    max_read;
>> >         unsigned int    max_write;
>> >  #endif /* CONFIG_CIFS_SMB2 */
>> > +       unsigned long echo_interval;
>> >  };
>> >
>> >  static inline unsigned int
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index ecb0803..3b44e9e 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -95,6 +95,7 @@ enum {
>> >         Opt_cruid, Opt_gid, Opt_file_mode,
>> >         Opt_dirmode, Opt_port,
>> >         Opt_rsize, Opt_wsize, Opt_actimeo,
>> > +       Opt_echo_interval,
>> >
>> >         /* Mount options which take string value */
>> >         Opt_user, Opt_pass, Opt_ip,
>> > @@ -188,6 +189,7 @@ static const match_table_t
>> > cifs_mount_option_tokens = {
>> >         { Opt_rsize, "rsize=%s" },
>> >         { Opt_wsize, "wsize=%s" },
>> >         { Opt_actimeo, "actimeo=%s" },
>> > +       { Opt_echo_interval, "echo_interval=%s" },
>> >
>> >         { Opt_blank_user, "user=" },
>> >         { Opt_blank_user, "username=" },
>> > @@ -418,6 +420,7 @@ 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;
>> >
>> >         /*
>> >          * We cannot send an echo if it is disabled or until the
>> > @@ -427,7 +430,7 @@ cifs_echo_request(struct work_struct *work)
>> >          */
>> >         if (!server->ops->need_neg || server->ops->need_neg(server)
>> > ||
>> >             (server->ops->can_echo && !server->ops-
>> > >can_echo(server)) ||
>> > -           time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL
>> > - HZ))
>> > +           time_before(jiffies, server->lstrp + echo_interval -
>> > HZ))
>> >                 goto requeue_echo;
>> >
>> >         rc = server->ops->echo ? server->ops->echo(server) :
>> > -ENOSYS;
>> > @@ -436,7 +439,7 @@ cifs_echo_request(struct work_struct *work)
>> >                          server->hostname);
>> >
>> >  requeue_echo:
>> > -       queue_delayed_work(cifsiod_wq, &server->echo,
>> > SMB_ECHO_INTERVAL);
>> > +       queue_delayed_work(cifsiod_wq, &server->echo,
>> > echo_interval);
>> >  }
>> >
>> >  static bool
>> > @@ -487,9 +490,9 @@ server_unresponsive(struct TCP_Server_Info
>> > *server)
>> >          *     a response in >60s.
>> >          */
>> >         if (server->tcpStatus == CifsGood &&
>> > -           time_after(jiffies, server->lstrp + 2 *
>> > SMB_ECHO_INTERVAL)) {
>> > -               cifs_dbg(VFS, "Server %s has not responded in %d
>> > seconds. Reconnecting...\n",
>> > -                        server->hostname, (2 * SMB_ECHO_INTERVAL)
>> > / HZ);
>> > +           time_after(jiffies, server->lstrp + 2 * server-
>> > >echo_interval)) {
>> > +               cifs_dbg(VFS, "Server %s has not responded in %lu
>> > seconds. Reconnecting...\n",
>> > +                        server->hostname, (2 * server-
>> > >echo_interval) / HZ);
>> >                 cifs_reconnect(server);
>> >                 wake_up(&server->response_q);
>> >                 return true;
>> > @@ -1624,6 +1627,14 @@ cifs_parse_mount_options(const char
>> > *mountdata, const char *devname,
>> >                                 goto cifs_parse_mount_err;
>> >                         }
>> >                         break;
>> > +               case Opt_echo_interval:
>> > +                       if (get_option_ul(args, &option)) {
>> > +                               cifs_dbg(VFS, "%s: Invalid echo
>> > interval value\n",
>> > +                                        __func__);
>> > +                               goto cifs_parse_mount_err;
>> > +                       }
>> > +                       vol->echo_interval = option;
>> > +                       break;
>> >
>> >                 /* String Arguments */
>> >
>> > @@ -2089,6 +2100,9 @@ static int match_server(struct
>> > TCP_Server_Info *server, struct smb_vol *vol)
>> >         if (!match_security(server, vol))
>> >                 return 0;
>> >
>> > +       if (server->echo_interval != vol->echo_interval)
>> > +               return 0;
>> > +
>> >         return 1;
>> >  }
>> >
>> > @@ -2208,6 +2222,13 @@ cifs_get_tcp_session(struct smb_vol
>> > *volume_info)
>> >         tcp_ses->tcpStatus = CifsNew;
>> >         ++tcp_ses->srv_count;
>> >
>> > +       /* echo interval should be between 10 and 120 secs */
>> > +       if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
>> > +               volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
>> > +               tcp_ses->echo_interval = volume_info->echo_interval
>> > * HZ;
>> > +       else
>> > +               tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT
>> > * HZ;
>> > +
>> >         rc = ip_connect(tcp_ses);
>> >         if (rc < 0) {
>> >                 cifs_dbg(VFS, "Error connecting to socket. Aborting
>> > operation.\n");
>> > @@ -2237,7 +2258,7 @@ cifs_get_tcp_session(struct smb_vol
>> > *volume_info)
>> >         cifs_fscache_get_client_cookie(tcp_ses);
>> >
>> >         /* queue echo request delayed work */
>> > -       queue_delayed_work(cifsiod_wq, &tcp_ses->echo,
>> > SMB_ECHO_INTERVAL);
>> > +       queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses-
>> > >echo_interval);
>> >
>> >         return tcp_ses;
>> >
>> > --
>> > 2.4.3
>> >
>>
>>
>>
>
Scott Lovenberg Dec. 17, 2015, 7:10 a.m. UTC | #8
On Wed, Dec 16, 2015 at 10:37 PM, Steve French <smfrench@gmail.com> wrote:
> Why restrict the echo interval to such a narrow range?  If tunable
> might as well allow a larger range, maybe 1 to 600 (ten minutes)?  I
> am not certain of the range of possible future uses - maybe a
> pseudo-real type workload connected to a really fast server wants
> something really low (a few seconds?) and maybe very poor network
> connections want something longer?  Not a strong objection, but seems
> like we could allow a broader range without confusing the user.
>
> Many years ago, Windows and OS/2 had an oplock break time (which also
> controlled how long to wait for acks to come back from the server) -
> and it ranged from 34 to 127 seconds for acks and 35 to 640 for oplock
> break timeouts.     Don't see any reason to limit it less than that
> (that was really old and networks vary a lot in performance now).
>
> Thoughts?

For context, were those timeouts for when broadcasts and signaling
were over three protocols (IPX, IP, and NetBEUI?  I can't recall, but
Chris H. was telling tales of yore last month at a dinner and IIRC the
propagation for SMB was over three protocols because usually at least
one would eventually reach remote hosts under most conditions)?  I
have no issue with long timeouts, but what was the historical context
of the 10 minute oplock breaks?  Forgive me if I'm recalling
incorrectly, but I thought that the extended timeouts were for
much-less-than-optimal conditions over multiple protocols attached to
hubs.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index cbc0f4b..eab2de6 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -507,6 +507,8 @@  cifs_show_options(struct seq_file *s, struct dentry *root)
 
 	seq_printf(s, ",rsize=%u", cifs_sb->rsize);
 	seq_printf(s, ",wsize=%u", cifs_sb->wsize);
+	seq_printf(s, ",echo_interval=%lu",
+			tcon->ses->server->echo_interval / HZ);
 	/* convert actimeo and display it in seconds */
 	seq_printf(s, ",actimeo=%lu", cifs_sb->actimeo / HZ);
 
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 2b510c5..56d3698 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -70,8 +70,10 @@ 
 #define SERVER_NAME_LENGTH 40
 #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
 
-/* SMB echo "timeout" -- FIXME: tunable? */
-#define SMB_ECHO_INTERVAL (60 * HZ)
+/* echo interval in seconds */
+#define SMB_ECHO_INTERVAL_MIN 10
+#define SMB_ECHO_INTERVAL_MAX 120
+#define SMB_ECHO_INTERVAL_DEFAULT 60
 
 #include "cifspdu.h"
 
@@ -507,6 +509,7 @@  struct smb_vol {
 	struct sockaddr_storage dstaddr; /* destination address */
 	struct sockaddr_storage srcaddr; /* allow binding to a local IP */
 	struct nls_table *local_nls;
+	unsigned int echo_interval; /* echo interval in secs */
 };
 
 #define CIFS_MOUNT_MASK (CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID | \
@@ -628,6 +631,7 @@  struct TCP_Server_Info {
 	unsigned int	max_read;
 	unsigned int	max_write;
 #endif /* CONFIG_CIFS_SMB2 */
+	unsigned long echo_interval;
 };
 
 static inline unsigned int
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ecb0803..3b44e9e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -95,6 +95,7 @@  enum {
 	Opt_cruid, Opt_gid, Opt_file_mode,
 	Opt_dirmode, Opt_port,
 	Opt_rsize, Opt_wsize, Opt_actimeo,
+	Opt_echo_interval,
 
 	/* Mount options which take string value */
 	Opt_user, Opt_pass, Opt_ip,
@@ -188,6 +189,7 @@  static const match_table_t cifs_mount_option_tokens = {
 	{ Opt_rsize, "rsize=%s" },
 	{ Opt_wsize, "wsize=%s" },
 	{ Opt_actimeo, "actimeo=%s" },
+	{ Opt_echo_interval, "echo_interval=%s" },
 
 	{ Opt_blank_user, "user=" },
 	{ Opt_blank_user, "username=" },
@@ -418,6 +420,7 @@  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;
 
 	/*
 	 * We cannot send an echo if it is disabled or until the
@@ -427,7 +430,7 @@  cifs_echo_request(struct work_struct *work)
 	 */
 	if (!server->ops->need_neg || server->ops->need_neg(server) ||
 	    (server->ops->can_echo && !server->ops->can_echo(server)) ||
-	    time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ))
+	    time_before(jiffies, server->lstrp + echo_interval - HZ))
 		goto requeue_echo;
 
 	rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
@@ -436,7 +439,7 @@  cifs_echo_request(struct work_struct *work)
 			 server->hostname);
 
 requeue_echo:
-	queue_delayed_work(cifsiod_wq, &server->echo, SMB_ECHO_INTERVAL);
+	queue_delayed_work(cifsiod_wq, &server->echo, echo_interval);
 }
 
 static bool
@@ -487,9 +490,9 @@  server_unresponsive(struct TCP_Server_Info *server)
 	 *     a response in >60s.
 	 */
 	if (server->tcpStatus == CifsGood &&
-	    time_after(jiffies, server->lstrp + 2 * SMB_ECHO_INTERVAL)) {
-		cifs_dbg(VFS, "Server %s has not responded in %d seconds. Reconnecting...\n",
-			 server->hostname, (2 * SMB_ECHO_INTERVAL) / HZ);
+	    time_after(jiffies, server->lstrp + 2 * server->echo_interval)) {
+		cifs_dbg(VFS, "Server %s has not responded in %lu seconds. Reconnecting...\n",
+			 server->hostname, (2 * server->echo_interval) / HZ);
 		cifs_reconnect(server);
 		wake_up(&server->response_q);
 		return true;
@@ -1624,6 +1627,14 @@  cifs_parse_mount_options(const char *mountdata, const char *devname,
 				goto cifs_parse_mount_err;
 			}
 			break;
+		case Opt_echo_interval:
+			if (get_option_ul(args, &option)) {
+				cifs_dbg(VFS, "%s: Invalid echo interval value\n",
+					 __func__);
+				goto cifs_parse_mount_err;
+			}
+			vol->echo_interval = option;
+			break;
 
 		/* String Arguments */
 
@@ -2089,6 +2100,9 @@  static int match_server(struct TCP_Server_Info *server, struct smb_vol *vol)
 	if (!match_security(server, vol))
 		return 0;
 
+	if (server->echo_interval != vol->echo_interval)
+		return 0;
+
 	return 1;
 }
 
@@ -2208,6 +2222,13 @@  cifs_get_tcp_session(struct smb_vol *volume_info)
 	tcp_ses->tcpStatus = CifsNew;
 	++tcp_ses->srv_count;
 
+	/* echo interval should be between 10 and 120 secs */
+	if (volume_info->echo_interval > SMB_ECHO_INTERVAL_MIN ||
+		volume_info->echo_interval < SMB_ECHO_INTERVAL_MAX)
+		tcp_ses->echo_interval = volume_info->echo_interval * HZ;
+	else
+		tcp_ses->echo_interval = SMB_ECHO_INTERVAL_DEFAULT * HZ;
+
 	rc = ip_connect(tcp_ses);
 	if (rc < 0) {
 		cifs_dbg(VFS, "Error connecting to socket. Aborting operation.\n");
@@ -2237,7 +2258,7 @@  cifs_get_tcp_session(struct smb_vol *volume_info)
 	cifs_fscache_get_client_cookie(tcp_ses);
 
 	/* queue echo request delayed work */
-	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, SMB_ECHO_INTERVAL);
+	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
 
 	return tcp_ses;