Patchwork return error code for a partial/short send for a retry of the send

login
register
mail settings
Submitter Shirish Pargaonkar
Date Sept. 11, 2009, 3:58 p.m.
Message ID <4a4634330909110858x6ca83353y8d666663b7bf2723@mail.gmail.com>
Download mbox | patch
Permalink /patch/33471/
State New
Headers show

Comments

Shirish Pargaonkar - Sept. 11, 2009, 3:58 p.m.
On Fri, Sep 11, 2009 at 8:43 AM, Jeff Layton <jlayton@redhat.com> wrote:

>  On Wed, 9 Sep 2009 03:40:13 -0500
> Shirish Pargaonkar <shirishpargaonkar@gmail.com> 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 <jlayton@redhat.com>
>


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 <shirishpargaonkar@gmail.com>


*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;
        }

Patch

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