On 05/26/2010 07:43 AM, shirishpargaonkar@xxxxxxxxx wrote:
> From 13bc9a6e9808c6a12c26ef25ead8a70210e75bcc Mon Sep 17 00:00:00 2001
> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> Date: Tue, 25 May 2010 21:07:24 -0500
> Subject: [PATCH] add support for implementing hard mount option behaviour
First of, I think the hard mounts behavior for cifs is not well defined
and documented. Along with a descriptive changelog that explains the
past and the behavior this patch introduces would help.
For e.g. whether the requests are retried indefinitely or not (nfs
usually retries indefinitely) or have a timeout, are they interruptible
or not, are there specific calls which are always soft/hard etc.
I think the mount.cifs man page section explaining 'hard' option also
badly needs an update (possible mentioning when hard is
recommended/useful). In current form, I think it is not clear when and
why one should use soft/hard mounts.
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> ---
> fs/cifs/cifsproto.h | 5 ++-
> fs/cifs/cifssmb.c | 99 ++++++++++++++++++++++++++------------------------
> fs/cifs/connect.c | 5 ++-
> fs/cifs/sess.c | 2 +-
> fs/cifs/transport.c | 60 ++++++++++++++++++++++++++++---
> 5 files changed, 113 insertions(+), 58 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 2208f06..7ddc602 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1809,6 +1809,9 @@ cifs_get_tcon(struct cifsSesInfo *ses, struct smb_vol *volume_info)
> }
> }
>
> + if (volume_info->retry)
> + tcon->retry = volume_info->retry;
> +
> if (strchr(volume_info->UNC + 3, '\\') == NULL
> && strchr(volume_info->UNC + 3, '/') == NULL) {
> cERROR(1, "Missing share name");
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 82f78c4..eb76d1a 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -311,6 +311,44 @@ static int allocate_mid(struct cifsSesInfo *ses, struct smb_hdr *in_buf,
> return 0;
> }
>
> +static int
> +wait_for_response_hard(struct cifsSesInfo *ses, struct mid_q_entry *midQ)
> +{
> + int rc;
> + bool cmdresp = true;
> + unsigned long timeout = 60 * HZ;
> + unsigned long curr_timeout;
> +
> +wait_again:
> + curr_timeout = timeout + jiffies;
> + rc = wait_event_interruptible_timeout(ses->server->response_q,
> + (midQ->midState != MID_REQUEST_SUBMITTED), timeout);
> +
> + if (rc < 0) {
IIRC, the earlier post checked for only -ERESTARTSYS. Just curious what
caused this change and why the earlier check was not sufficient?
> + cFYI(1, "command 0x%x interrupted", midQ->command);
> + return -1;
> + }
> + if (!time_before(jiffies, curr_timeout)) {
> + cmdresp = false;
> + cFYI(1, "server not responding...");
> + goto wait_again;
> + }
> + spin_lock(&GlobalMid_Lock);
Do we really need to acquire the lock here?
> + if (midQ->midState != MID_REQUEST_SUBMITTED) {
> + if (midQ->midState == MID_RESPONSE_RECEIVED) {
> + if (!cmdresp)
> + cFYI(1, "server is ok...");
The cFYI message could be little more useful..
> + rc = 0;
> + } else {
> + cFYI(1, "command 0x%x aborted", midQ->command);
> + rc = -1;
> + }
> + }
> + spin_unlock(&GlobalMid_Lock);
> + return rc;
> +}
> +
> +
Thanks,
--
Suresh Jayaraman
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
