[linux-cifs-client] Re: [PATCH 6/6] cifs: don't retry on blocking sends

[Steve French]

Presumably there is slightly less overhead (since 99%+ of all tcp
kernel send message calls don't timeout/retry) in not having to reset
the send timeout over and over.   We need a longer send timeout for
large writes (> 56K) than for typical requests (100 bytes or so for
the header is all that is sent)... so presumably is more efficient to
set a low timeout (somewhere from 1 to 5 seconds) and retry a few
times for the large write case.

On Sun, Nov 30, 2008 at 4:18 PM, Jeff Layton <jlayton at redhat.com> wrote:
> On Sun, 30 Nov 2008 15:39:17 -0600
> "Steve French" <smfrench at gmail.com> wrote:
>
>> We probably do need to retry, but closer to the 20 second range total which
>> is only slightly longer than the large (non-blocking) send retry used to wait.
>>
>
> I'm not sure I understand this. Why would we need to ever retry the
> send? Why not just set the timeout to the value we want and let kernel_sendmsg
> do the work?
>
> The timeout value is certainly negotiable though...
>
>> On Sun, Nov 30, 2008 at 7:57 AM, Jeff Layton <jlayton at redhat.com> wrote:
>> > smb_send2 will attempt to send a message and retry if it doesn't work.
>> > It's rather pointless to do this in the case of blocking sends. We can
>> > just set the socket sndtimeo to a reasonable value and try the send
>> > once.
>> >
>> > Why change the timeout to 30s? That seems like a reasonable amount of
>> > time to wait for the send. It's also what sunrpc uses for sndtimeo
>> > with TCP sockets. At some point we might consider making this
>> > tunable, but a generic "timeo" setting similar to what NFS has is
>> > probably more user friendly.
>> >
>> > Signed-off-by: Jeff Layton <jlayton at redhat.com>
>> > ---
>> >  fs/cifs/connect.c   |    4 ++--
>> >  fs/cifs/transport.c |   12 ++++++++++++
>> >  2 files changed, 14 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 25f7cba..16afe2d 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -1753,7 +1753,7 @@ ipv4_connect(struct TCP_Server_Info *server)
>> >         *  user space buffer
>> >         */
>> >        socket->sk->sk_rcvtimeo = 7 * HZ;
>> > -       socket->sk->sk_sndtimeo = 3 * HZ;
>> > +       socket->sk->sk_sndtimeo = 30 * HZ;
>> >
>> >        /* make the bufsizes depend on wsize/rsize and max requests */
>> >        if (server->noautotune) {
>> > @@ -1904,7 +1904,7 @@ ipv6_connect(struct TCP_Server_Info *server)
>> >         * user space buffer
>> >         */
>> >        socket->sk->sk_rcvtimeo = 7 * HZ;
>> > -       socket->sk->sk_sndtimeo = 3 * HZ;
>> > +       socket->sk->sk_sndtimeo = 30 * HZ;
>> >        server->ssocket = socket;
>> >
>> >        return rc;
>> > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>> > index 2d1bd28..e4bf0b4 100644
>> > --- a/fs/cifs/transport.c
>> > +++ b/fs/cifs/transport.c
>> > @@ -197,6 +197,18 @@ smb_send2(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);
>> > +
>> > +               /*
>> > +                * blocking send -- just update total_len and break out of
>> > +                * the loop. No sense in retrying since we've already given
>> > +                * it a long timeout.
>> > +                */
>> > +               if (!server->noblocksnd) {
>> > +                       if (rc >= 0)
>> > +                               total_len -= rc;
>> > +                       break;
>> > +               }
>> > +
>> >                if ((rc == -ENOSPC) || (rc == -EAGAIN)) {
>> >                        i++;
>> >                        if (i >= 14) {
>> > --
>> > 1.5.5.1
>> >
>> >
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>
>
> --
> Jeff Layton <jlayton at redhat.com>
>



-- 
Thanks,

Steve