From patchwork Fri Sep 11 15:58:54 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shirish Pargaonkar X-Patchwork-Id: 46879 Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n8BFx24U004981 for ; Fri, 11 Sep 2009 15:59:02 GMT Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 7D29046591; Fri, 11 Sep 2009 09:48:17 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=3.8 tests=AWL, BAYES_00, HTML_MESSAGE, SPF_PASS autolearn=no version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from an-out-0708.google.com (an-out-0708.google.com [209.85.132.243]) by lists.samba.org (Postfix) with ESMTP id 2E804AD0DD for ; Fri, 11 Sep 2009 09:48:10 -0600 (MDT) Received: by an-out-0708.google.com with SMTP id c5so409628anc.35 for ; Fri, 11 Sep 2009 08:58:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type; bh=UE9U5PzsyVrUEGDHApEI1NOUWklyLf4w5KeQwKSSe0w=; b=vn6LhUgtU/xLGQIh72NAF8uRVE1sF5xWI0kJud7SahheF77800WlvBJHT3r6ec38vL Uw3MRw4kDBLu5LiXKpyX0HmGa1qVa0+M/4X3Nc23CV4vr3AN0XvissVks0/qxSnyHJ9s iAyn2F0Mw2kqElL/eAL2r4tVE7KQj7cXWC2So= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=ffmGsKl1IyJGMYeZoAm00ee1B9JSj92WuB7GFFefRrvmkguGDvjGC2MrRnBSwwYgII FIj9HHocpmsWc2+Aq1vNELFJg5B5AaZeH1LOIAydzfskVf0HizWuaBjvteOW+eaeOoq0 p5ZNbblAGOfytpzR7sdHB4witcSww6jO7etLU= MIME-Version: 1.0 Received: by 10.101.102.12 with SMTP id e12mr3393521anm.144.1252684734775; Fri, 11 Sep 2009 08:58:54 -0700 (PDT) In-Reply-To: <20090911094326.285f2663@tupile.poochiereds.net> References: <4a4634330909090140n1d40029ds6e510f6a9963efcc@mail.gmail.com> <20090911094326.285f2663@tupile.poochiereds.net> Date: Fri, 11 Sep 2009 10:58:54 -0500 Message-ID: <4a4634330909110858x6ca83353y8d666663b7bf2723@mail.gmail.com> From: Shirish Pargaonkar To: Jeff Layton Cc: linux-cifs-client@lists.samba.org Subject: Re: [linux-cifs-client] [PATCH] return error code for a partial/short send for a retry of the send X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org On Fri, Sep 11, 2009 at 8:43 AM, Jeff Layton wrote: > On Wed, 9 Sep 2009 03:40:13 -0500 > Shirish Pargaonkar wrote: > > > - Enable such that partial/shorts sends can be resent/retried_for_send > > - Keep the midQ entry by marking it for resend/retry_sending in case > > of EAGAIN error > > > > > > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > > index 1da4ab2..92fa1ad 100644 > > --- a/fs/cifs/transport.c > > +++ b/fs/cifs/transport.c > > @@ -269,6 +269,7 @@ smb_sendv(struct TCP_Server_Info *server, struct > > kvec *iov, int n_vec) > > to kill the socket so the server throws away the > partial > > SMB */ > > server->tcpStatus = CifsNeedReconnect; > > + rc = -EAGAIN; > > } > > > > if (rc < 0) { > > @@ -505,8 +506,13 @@ SendReceive2(const unsigned int xid, struct > > cifsSesInfo *ses, > > mutex_unlock(&ses->server->srv_mutex); > > cifs_small_buf_release(in_buf); > > > > - if (rc < 0) > > - goto out; > > + if (rc < 0) { > > + if (rc == -EAGAIN) { > > + midQ->midState = MID_RETRY_NEEDED; > > + goto outagain; > > + } else > > + goto out; > > + } > > > > if (long_op == CIFS_STD_OP) > > timeout = 15 * HZ; > > @@ -623,6 +629,7 @@ SendReceive2(const unsigned int xid, struct > > cifsSesInfo *ses, > > > > out: > > DeleteMidQEntry(midQ); > > +outagain: > > atomic_dec(&ses->server->inFlight); > > wake_up(&ses->server->request_q); > > > > @@ -697,8 +704,13 @@ SendReceive(const unsigned int xid, struct > > cifsSesInfo *ses, > > #endif > > mutex_unlock(&ses->server->srv_mutex); > > > > - if (rc < 0) > > - goto out; > > + if (rc < 0) { > > + if (rc == -EAGAIN) { > > + midQ->midState = MID_RETRY_NEEDED; > > + goto outagain; > > + } else > > + goto out; > > + } > > > > if (long_op == CIFS_STD_OP) > > timeout = 15 * HZ; > > @@ -807,6 +819,7 @@ SendReceive(const unsigned int xid, struct > cifsSesInfo *ses, > > > > out: > > DeleteMidQEntry(midQ); > > +outagain: > > atomic_dec(&ses->server->inFlight); > > wake_up(&ses->server->request_q); > > Patch looks reasonably sane. Could you outline what problem this is > intended to solve? > > Also what about SendReceiveBlockingLock? Doesn't it need a similar > change? > > -- > Jeff Layton > When a command is partially sent, cifs reconnects, so the partially sent command needs to be re-sent, so then we want to percolate up that error so we can retry the command above SendReceivexxx (not sure yet where exactly above). I did not see in test runs a locking operation returning with EAGAIN error. Actually EAGAIN is more likely to occurr during large sends but it is safer to handle (percolate up for retry) EAGAIN in SendReceive and SendReceiveBlockingLock. Handling egain in SendReceiveBlockingLock too. Here is the respin. signed-off-by: Shirish Pargaonkar *ses, out: DeleteMidQEntry(midQ); +outagain: atomic_dec(&ses->server->inFlight); wake_up(&ses->server->request_q); @@ -697,8 +704,13 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses, #endif mutex_unlock(&ses->server->srv_mutex); - if (rc < 0) - goto out; + if (rc < 0) { + if (rc == -EAGAIN) { + midQ->midState = MID_RETRY_NEEDED; + goto outagain; + } else + goto out; + } if (long_op == CIFS_STD_OP) timeout = 15 * HZ; @@ -807,6 +819,7 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses, out: DeleteMidQEntry(midQ); +outagain: atomic_dec(&ses->server->inFlight); wake_up(&ses->server->request_q); @@ -931,7 +944,10 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifsTconInfo *tcon, mutex_unlock(&ses->server->srv_mutex); if (rc < 0) { - DeleteMidQEntry(midQ); + if (rc == -EAGAIN) + midQ->midState = MID_RETRY_NEEDED; + else + DeleteMidQEntry(midQ); return rc; } diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 1da4ab2..61b7a58 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -269,6 +269,7 @@ smb_sendv(struct TCP_Server_Info *server, struct kvec *iov, int n_vec) to kill the socket so the server throws away the partial SMB */ server->tcpStatus = CifsNeedReconnect; + rc = -EAGAIN; } if (rc < 0) { @@ -505,8 +506,13 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses, mutex_unlock(&ses->server->srv_mutex); cifs_small_buf_release(in_buf); - if (rc < 0) - goto out; + if (rc < 0) { + if (rc == -EAGAIN) { + midQ->midState = MID_RETRY_NEEDED; + goto outagain; + } else + goto out; + } if (long_op == CIFS_STD_OP) timeout = 15 * HZ; @@ -623,6 +629,7 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo