On Mon, 1 Oct 2012 20:46:01 -0500
Steve French <smfrench@xxxxxxxxx> wrote:
> How can we be certain that ENOSPC is no longer returned? When it was
> returned then the obvious thing was to block briefly and retry.
>
> Does it do any harm?
>
Can you explain the situation that caused it to return -ENOSPC in the
first place? Why that and not -EAGAIN? Or -ENOBUFS?
> On Mon, Oct 1, 2012 at 8:40 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Mon, 01 Oct 2012 13:10:45 +0530
> > Suresh Jayaraman <sjayaraman@xxxxxxxx> wrote:
> >
> >> In smb_sendv(), we seem to retry if kernel_sendmsg() returned either
> >> -ENOSPC or -EAGAIN. In either case after multiple attempts, we set the
> >> error to -EAGAIN before breaking out of the loop.
> >>
> >> First, it is not clear to me when kernel_sendmsg() can return -ENOSPC,
> >> and what it would mean and why should we retry. It makes me wonder
> >> whether this check is part of some old code. Also, there seem to be no
> >> need to set the error back to -EAGAIN before we break out the loop.
> >> Fix this by making cifs retry only if kernel_sendmsg() returns -EAGAIN.
> >>
> >> If the above discussion make sense, here is a patch to fix this.
> >> ---
> >>
> >> Retry kernel_sendmsg() only in case of -EAGAIN and remove redundant
> >> error assignment.
> >>
> >>
> >> Signed-off-by: Suresh Jayaraman <sjayaraman@xxxxxxxx>
> >> ---
> >> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> >> index d9b639b..a33db4c 100644
> >> --- a/fs/cifs/transport.c
> >> +++ b/fs/cifs/transport.c
> >> @@ -154,7 +154,7 @@ smb_sendv(struct TCP_Server_Info *server, struct kvec *iov, int n_vec)
> >> while (total_len) {
> >> rc = kernel_sendmsg(ssocket, &smb_msg, &iov[first_vec],
> >> n_vec - first_vec, total_len);
> >> - if ((rc == -ENOSPC) || (rc == -EAGAIN)) {
> >> + if (rc == -EAGAIN) {
> >> i++;
> >> /*
> >> * If blocking send we try 3 times, since each can block
> >> @@ -177,7 +177,6 @@ smb_sendv(struct TCP_Server_Info *server, struct kvec *iov, int n_vec)
> >> if ((i >= 14) || (!server->noblocksnd && (i > 2))) {
> >> cERROR(1, "sends on sock %p stuck for 15 seconds",
> >> ssocket);
> >> - rc = -EAGAIN;
> >> break;
> >> }
> >> msleep(1 << i);
> >> --
> >> 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
> >
> > That ENOSPC has been there a long time, and I think you're correct that
> > it's of questionable value.
> >
> > Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>
>
>
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
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
