Re: [PATCH v2 3/9] cifs: cork the socket before a send and uncork it afterward

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

 



2012/8/5 Steve French <smfrench@xxxxxxxxx>:
> I suspect that there is a relationship between this and the queueing
> problems we saw with Windows Vista and Windows 7 depending on max
> multiplex request size (for cifs).
>
> On Sat, Aug 4, 2012 at 3:51 PM, Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>> 2012/8/1 Jeff Layton <jlayton@xxxxxxxxx>:
>>> On Wed, 1 Aug 2012 15:37:57 +0200
>>> Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>>>
>>>> 2012/7/31 Jeff Layton <jlayton@xxxxxxxxx>:
>>>> > On Mon, 30 Jul 2012 21:17:53 -0400
>>>> > Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>>>> >
>>>> >> On Mon, 30 Jul 2012 23:11:10 +0200
>>>> >> Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>>>> >>
>>>> >> > 2012/7/29 Jeff Layton <jlayton@xxxxxxxxxx>:
>>>> >> > > On Fri, 27 Jul 2012 10:05:32 +0400
>>>> >> > > Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>>>> >> > >
>>>> >> > >> 2012/7/27 Jeff Layton <jlayton@xxxxxxxxxx>:
>>>> >> > >> > On Fri, 27 Jul 2012 03:57:44 +0400
>>>> >> > >> > Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>>>> >> > >> >
>>>> >> > >> >> 2012/7/25 Jeff Layton <jlayton@xxxxxxxxxx>:
>>>> >> > >> >> > We want to send SMBs as "atomically" as possible. Prior to sending any
>>>> >> > >> >> > data on the socket, cork it to make sure that no non-full frames go
>>>> >> > >> >> > out. Afterward, uncork it to make sure all of the data gets pushed out
>>>> >> > >> >> > to the wire.
>>>> >> > >> >> >
>>>> >> > >> >> > Note that this more or less renders the socket=TCP_NODELAY mount option
>>>> >> > >> >> > obsolete. When TCP_CORK and TCP_NODELAY are used on the same socket,
>>>> >> > >> >> > TCP_NODELAY is essentially ignored.
>>>> >> > >> >> >
>>>> >> > >> >> > Acked-by: Pavel Shilovsky <pshilovsky@xxxxxxxxx>
>>>> >> > >> >> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>>>> >> > >> >> > ---
>>>> >> > >> >> >  fs/cifs/connect.c   |  4 ++++
>>>> >> > >> >> >  fs/cifs/transport.c | 12 ++++++++++++
>>>> >> > >> >> >  2 files changed, 16 insertions(+)
>>>> >> > >> >> >
>>>> >> > >> >> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>>> >> > >> >> > index 6df6fa1..a828a8c 100644
>>>> >> > >> >> > --- a/fs/cifs/connect.c
>>>> >> > >> >> > +++ b/fs/cifs/connect.c
>>>> >> > >> >> > @@ -1676,6 +1676,10 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>>>> >> > >> >> >                         if (string == NULL)
>>>> >> > >> >> >                                 goto out_nomem;
>>>> >> > >> >> >
>>>> >> > >> >> > +                       /*
>>>> >> > >> >> > +                        * FIXME: since we now cork/uncork the socket while
>>>> >> > >> >> > +                        *        sending, should we deprecate this option?
>>>> >> > >> >> > +                        */
>>>> >> > >> >> >                         if (strnicmp(string, "TCP_NODELAY", 11) == 0)
>>>> >> > >> >> >                                 vol->sockopt_tcp_nodelay = 1;
>>>> >> > >> >> >                         break;
>>>> >> > >> >> > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>>>> >> > >> >> > index d93f15d..a3e58b2 100644
>>>> >> > >> >> > --- a/fs/cifs/transport.c
>>>> >> > >> >> > +++ b/fs/cifs/transport.c
>>>> >> > >> >> > @@ -27,6 +27,7 @@
>>>> >> > >> >> >  #include <linux/net.h>
>>>> >> > >> >> >  #include <linux/delay.h>
>>>> >> > >> >> >  #include <linux/freezer.h>
>>>> >> > >> >> > +#include <linux/tcp.h>
>>>> >> > >> >> >  #include <asm/uaccess.h>
>>>> >> > >> >> >  #include <asm/processor.h>
>>>> >> > >> >> >  #include <linux/mempool.h>
>>>> >> > >> >> > @@ -247,12 +248,23 @@ smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
>>>> >> > >> >> >         int n_vec = rqst->rq_nvec;
>>>> >> > >> >> >         unsigned int smb_buf_length = get_rfc1002_length(iov[0].iov_base);
>>>> >> > >> >> >         size_t total_len;
>>>> >> > >> >> > +       struct socket *ssocket = server->ssocket;
>>>> >> > >> >> > +       int val = 1;
>>>> >> > >> >> >
>>>> >> > >> >> >         cFYI(1, "Sending smb: smb_len=%u", smb_buf_length);
>>>> >> > >> >> >         dump_smb(iov[0].iov_base, iov[0].iov_len);
>>>> >> > >> >> >
>>>> >> > >> >> > +       /* cork the socket */
>>>> >> > >> >> > +       kernel_setsockopt(ssocket, SOL_TCP, TCP_CORK,
>>>> >> > >> >> > +                               (char *)&val, sizeof(val));
>>>> >> > >> >> > +
>>>> >> > >> >> >         rc = smb_send_kvec(server, iov, n_vec, &total_len);
>>>> >> > >> >> >
>>>> >> > >> >> > +       /* uncork it */
>>>> >> > >> >> > +       val = 0;
>>>> >> > >> >> > +       kernel_setsockopt(ssocket, SOL_TCP, TCP_CORK,
>>>> >> > >> >> > +                               (char *)&val, sizeof(val));
>>>> >> > >> >> > +
>>>> >> > >> >> >         if ((total_len > 0) && (total_len != smb_buf_length + 4)) {
>>>> >> > >> >> >                 cFYI(1, "partial send (wanted=%u sent=%zu): terminating "
>>>> >> > >> >> >                         "session", smb_buf_length + 4, total_len);
>>>> >> > >> >> > --
>>>> >> > >> >> > 1.7.11.2
>>>> >> > >> >> >
>>>> >> > >> >> > --
>>>> >> > >> >> > 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
>>>> >> > >> >>
>>>> >> > >> >> I tested it with SMB2 against Windows 7 server. When iosize is 64K
>>>> >> > >> >> everything is ok but when we increase iosize to 1M (by using
>>>> >> > >> >> multicredit requests) and the server loses the network connection and
>>>> >> > >> >> only reboot helps.
>>>> >> > >> >>
>>>> >> > >> >> Also if I commented corking/uncorking the socket - everything is ok. I
>>>> >> > >> >> think this change needs some more investigation (how does it deals
>>>> >> > >> >> with 1M iosize on Samba, etc?)
>>>> >> > >> >>
>>>> >> > >> >
>>>> >> > >> > Hmm, haven't seen that with a 1M iosize with smb1 against samba.
>>>> >> > >> >
>>>> >> > >> > I'll see if I can reproduce it.
>>>> >> > >> >
>>>> >> > >> > --
>>>> >> > >> > Jeff Layton <jlayton@xxxxxxxxxx>
>>>> >> > >>
>>>> >> > >> Forgot to mentioned how I reproduce it - dbench with 5 clients.
>>>> >> > >>
>>>> >> > >
>>>> >> > > Ok, I've built a cifs.ko from your smb2-dev-cifs-3.6 branch. Here are my mount options:
>>>> >> > >
>>>> >> > > //win7.poochiereds.net/export /mnt/win7 cifs rw,relatime,vers=2.1,sec=ntlmsspi,cache=strict,unc=\\win7.poochiereds.net\export,username=testuser,domain=WIN7,uid=0,noforceuid,gid=0,noforcegid,addr=192.168.1.32,file_mode=0755,dir_mode=0755,nounix,serverino,rsize=1048576,wsize=1048576,actimeo=1 0 0
>>>> >> >
>>>> >> > I don't set rsize and wsize explicitly but I don't think it's related.
>>>> >> > On what connection did you test it? I use 100Mbit LAN.
>>>> >> >
>>>> >>
>>>> >> The clients and servers are both KVM guests. I'll give it a go over a
>>>> >> physical network tomorrow.
>>>> >>
>>>> >
>>>> > Ok, ran it with client as a KVM guest and the server as win7 running on
>>>> > bare metal over a gigE network. It still ran just fine.
>>>> >
>>>> > So what are the symptoms that you see here? Does dbench just hang? If
>>>> > so, could you collect /proc/<pid>/stack from the hung process(es)?
>>>> > Maybe that would tell us what's going on...
>>>>
>>>> Windows 7 server doesn't response to packets after some time running
>>>> dbench. Also, I even can't ping google.com from this Windows machine.
>>>> It seems that everything ok with dbench and Linux machine.
>>>>
>>>> So, it looks like Windows server problem on my configuration but of
>>>> course seems very strage. I will try this patch with Samba server
>>>> further.
>>>>
>>>> This patch doesn't break things with Windows untill we use multicredit
>>>> requests (more than 64K, that are not targeted to 3.6 kernel). But I
>>>> am going to target multicredit requests feature for 3.7. May be we
>>>> should make cork/nodelay switchable? Or just merge the patchset
>>>> without this patch for 3.6 and delay this patch for 3.7 - we will have
>>>> much time to investigate this strange behavior?
>>>>
>>>> Thoughts?
>>>>
>>>
>>> If the server isn't responding then that seems like something is broken
>>> on the server. Maybe you have a broken network driver? Do you have any
>>> captures?
>>
>> Now I don't have them but I can collect captures when return from vacations.
>>
>> --
>> Best regards,
>> Pavel Shilovsky.
>
>
>
> --
> Thanks,
>
> Steve

Tested it again with the same work environment and couldn't reproduce
it - seems very strange, because later it was 100% reproducible.

-- 
Best regards,
Pavel Shilovsky.
--
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