Re: [RFC] [PATCH] cifs: retry kernel_sendmsg only in case of -EAGAIN

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

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>



-- 
Thanks,

Steve
--
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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux