[CIFS] Reduce number of socket retries in large write path
CIFS in some heavy stress conditions cifs could get EAGAIN
repeatedly in smb_send2 which led to repeated retries and eventually
failure of large writes which could lead to data corruption.
There are three changes that were suggested by various network
developers:
1) convert cifs from non-blocking to blocking tcp sendmsg
(we left in the retry on failure)
2) change cifs to not set sendbuf and rcvbuf size for the socket
(let tcp autotune the buffer sizes since that works much better
in the TCP stack now)
3) if we have a partial frame sent in smb_send2, mark the tcp
session as invalid (close the socket and reconnect) so we do
not corrupt the remaining part of the SMB with the beginning
of the next SMB.
This does not appear to hurt performance measurably and has
been run in various scenarios, but it definately removes
a corruption that we were seeing in some high stress
test cases.
Acked-by: Shirish Pargaonkar <shirishp@us.ibm.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
index 8f528ea..8855331 100644
--- a/fs/cifs/CHANGES
+++ b/fs/cifs/CHANGES
@@ -4,7 +4,11 @@
(when delete of an open file fails we mark the file as "delete-on-close"
in a way that more servers accept, but only if we can first rename the
file to a temporary name). Add experimental support for more safely
-handling fcntl(F_SETLEASE).
+handling fcntl(F_SETLEASE). Convert cifs to using blocking tcp
+sends, and also let tcp autotune the socket send and receive buffers.
+This reduces the number of EAGAIN errors returned by TCP/IP in
+high stress workloads (and the number of retries on socket writes
+when sending large SMBWriteX requests).
Version 1.54
------------
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index c791e5b..1cb1189 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -141,6 +141,8 @@
char versionMajor;
char versionMinor;
bool svlocal:1; /* local server or remote */
+ bool noblocksnd; /* use blocking sendmsg */
+ bool noautotune; /* do not autotune send buf sizes */
atomic_t socketUseCount; /* number of open cifs sessions on socket */
atomic_t inFlight; /* number of requests on the wire to server */
#ifdef CONFIG_CIFS_STATS2
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 0cff7fe..6f21ecb 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -36,7 +36,7 @@
extern struct smb_hdr *cifs_small_buf_get(void);
extern void cifs_small_buf_release(void *);
extern int smb_send(struct socket *, struct smb_hdr *,
- unsigned int /* length */ , struct sockaddr *);
+ unsigned int /* length */ , struct sockaddr *, bool);
extern unsigned int _GetXid(void);
extern void _FreeXid(unsigned int);
#define GetXid() (int)_GetXid(); cFYI(1,("CIFS VFS: in %s as Xid: %d with uid: %d",__func__, xid,current->fsuid));
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 71b7661..e9f9248 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -92,6 +92,8 @@
bool seal:1; /* request transport encryption on share */
bool nodfs:1; /* Do not request DFS, even if available */
bool local_lease:1; /* check leases only on local system, not remote */
+ bool noblocksnd:1;
+ bool noautotune:1;
unsigned int rsize;
unsigned int wsize;
unsigned int sockopt;
@@ -102,9 +104,11 @@
static int ipv4_connect(struct sockaddr_in *psin_server,
struct socket **csocket,
char *netb_name,
- char *server_netb_name);
+ char *server_netb_name,
+ bool noblocksnd,
+ bool nosndbuf); /* ipv6 never set sndbuf size */
static int ipv6_connect(struct sockaddr_in6 *psin_server,
- struct socket **csocket);
+ struct socket **csocket, bool noblocksnd);
/*
@@ -191,12 +195,13 @@
try_to_freeze();
if (server->protocolType == IPV6) {
rc = ipv6_connect(&server->addr.sockAddr6,
- &server->ssocket);
+ &server->ssocket, server->noautotune);
} else {
rc = ipv4_connect(&server->addr.sockAddr,
&server->ssocket,
server->workstation_RFC1001_name,
- server->server_RFC1001_name);
+ server->server_RFC1001_name,
+ server->noblocksnd, server->noautotune);
}
if (rc) {
cFYI(1, ("reconnect error %d", rc));
@@ -1192,6 +1197,10 @@
/* ignore */
} else if (strnicmp(data, "rw", 2) == 0) {
vol->rw = true;
+ } else if (strnicmp(data, "noblocksend", 11) == 0) {
+ vol->noblocksnd = 1;
+ } else if (strnicmp(data, "noautotune", 10) == 0) {
+ vol->noautotune = 1;
} else if ((strnicmp(data, "suid", 4) == 0) ||
(strnicmp(data, "nosuid", 6) == 0) ||
(strnicmp(data, "exec", 4) == 0) ||
@@ -1518,7 +1527,8 @@
static int
ipv4_connect(struct sockaddr_in *psin_server, struct socket **csocket,
- char *netbios_name, char *target_name)
+ char *netbios_name, char *target_name,
+ bool noblocksnd, bool noautotune)
{
int rc = 0;
int connected = 0;
@@ -1590,11 +1600,16 @@
(*csocket)->sk->sk_sndbuf,
(*csocket)->sk->sk_rcvbuf, (*csocket)->sk->sk_rcvtimeo));
(*csocket)->sk->sk_rcvtimeo = 7 * HZ;
+ if (!noblocksnd)
+ (*csocket)->sk->sk_sndtimeo = 3 * HZ;
+
/* make the bufsizes depend on wsize/rsize and max requests */
- if ((*csocket)->sk->sk_sndbuf < (200 * 1024))
- (*csocket)->sk->sk_sndbuf = 200 * 1024;
- if ((*csocket)->sk->sk_rcvbuf < (140 * 1024))
- (*csocket)->sk->sk_rcvbuf = 140 * 1024;
+ if (noautotune) {
+ if ((*csocket)->sk->sk_sndbuf < (200 * 1024))
+ (*csocket)->sk->sk_sndbuf = 200 * 1024;
+ if ((*csocket)->sk->sk_rcvbuf < (140 * 1024))
+ (*csocket)->sk->sk_rcvbuf = 140 * 1024;
+ }
/* send RFC1001 sessinit */
if (psin_server->sin_port == htons(RFC1001_PORT)) {
@@ -1631,7 +1646,7 @@
/* sizeof RFC1002_SESSION_REQUEST with no scope */
smb_buf->smb_buf_length = 0x81000044;
rc = smb_send(*csocket, smb_buf, 0x44,
- (struct sockaddr *)psin_server);
+ (struct sockaddr *)psin_server, noblocksnd);
kfree(ses_init_buf);
msleep(1); /* RFC1001 layer in at least one server
requires very short break before negprot
@@ -1651,7 +1666,8 @@
}
static int
-ipv6_connect(struct sockaddr_in6 *psin_server, struct socket **csocket)
+ipv6_connect(struct sockaddr_in6 *psin_server, struct socket **csocket,
+ bool noblocksnd)
{
int rc = 0;
int connected = 0;
@@ -1720,6 +1736,9 @@
the default. sock_setsockopt not used because it expects
user space buffer */
(*csocket)->sk->sk_rcvtimeo = 7 * HZ;
+ if (!noblocksnd)
+ (*csocket)->sk->sk_sndtimeo = 3 * HZ;
+
return rc;
}
@@ -1983,11 +2002,14 @@
cFYI(1, ("attempting ipv6 connect"));
/* BB should we allow ipv6 on port 139? */
/* other OS never observed in Wild doing 139 with v6 */
- rc = ipv6_connect(&sin_server6, &csocket);
+ rc = ipv6_connect(&sin_server6, &csocket,
+ volume_info.noblocksnd);
} else
rc = ipv4_connect(&sin_server, &csocket,
volume_info.source_rfc1001_name,
- volume_info.target_rfc1001_name);
+ volume_info.target_rfc1001_name,
+ volume_info.noblocksnd,
+ volume_info.noautotune);
if (rc < 0) {
cERROR(1, ("Error connecting to IPv4 socket. "
"Aborting operation"));
@@ -2002,6 +2024,8 @@
sock_release(csocket);
goto out;
} else {
+ srvTcp->noblocksnd = volume_info.noblocksnd;
+ srvTcp->noautotune = volume_info.noautotune;
memcpy(&srvTcp->addr.sockAddr, &sin_server,
sizeof(struct sockaddr_in));
atomic_set(&srvTcp->inFlight, 0);
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index bf0e6d8..ba4d666 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -161,7 +161,7 @@
int
smb_send(struct socket *ssocket, struct smb_hdr *smb_buffer,
- unsigned int smb_buf_length, struct sockaddr *sin)
+ unsigned int smb_buf_length, struct sockaddr *sin, bool noblocksnd)
{
int rc = 0;
int i = 0;
@@ -178,7 +178,10 @@
smb_msg.msg_namelen = sizeof(struct sockaddr);
smb_msg.msg_control = NULL;
smb_msg.msg_controllen = 0;
- smb_msg.msg_flags = MSG_DONTWAIT + MSG_NOSIGNAL; /* BB add more flags?*/
+ if (noblocksnd)
+ smb_msg.msg_flags = MSG_DONTWAIT + MSG_NOSIGNAL;
+ else
+ smb_msg.msg_flags = MSG_NOSIGNAL;
/* smb header is converted in header_assemble. bcc and rest of SMB word
area, and byte area if necessary, is converted to littleendian in
@@ -229,8 +232,8 @@
}
static int
-smb_send2(struct socket *ssocket, struct kvec *iov, int n_vec,
- struct sockaddr *sin)
+smb_send2(struct TCP_Server_Info *server, struct kvec *iov, int n_vec,
+ struct sockaddr *sin, bool noblocksnd)
{
int rc = 0;
int i = 0;
@@ -240,6 +243,7 @@
unsigned int total_len;
int first_vec = 0;
unsigned int smb_buf_length = smb_buffer->smb_buf_length;
+ struct socket *ssocket = server->ssocket;
if (ssocket == NULL)
return -ENOTSOCK; /* BB eventually add reconnect code here */
@@ -248,7 +252,10 @@
smb_msg.msg_namelen = sizeof(struct sockaddr);
smb_msg.msg_control = NULL;
smb_msg.msg_controllen = 0;
- smb_msg.msg_flags = MSG_DONTWAIT + MSG_NOSIGNAL; /* BB add more flags?*/
+ if (noblocksnd)
+ smb_msg.msg_flags = MSG_DONTWAIT + MSG_NOSIGNAL;
+ else
+ smb_msg.msg_flags = MSG_NOSIGNAL;
/* smb header is converted in header_assemble. bcc and rest of SMB word
area, and byte area if necessary, is converted to littleendian in
@@ -312,6 +319,16 @@
i = 0; /* in case we get ENOSPC on the next send */
}
+ if ((total_len > 0) && (total_len != smb_buf_length + 4)) {
+ cFYI(1, ("partial send (%d remaining), terminating session",
+ total_len));
+ /* If we have only sent part of an SMB then the next SMB
+ could be taken as the remainder of this one. We need
+ to kill the socket so the server throws away the partial
+ SMB */
+ server->tcpStatus = CifsNeedReconnect;
+ }
+
if (rc < 0) {
cERROR(1, ("Error %d sending data on socket to server", rc));
} else
@@ -518,8 +535,9 @@
#ifdef CONFIG_CIFS_STATS2
atomic_inc(&ses->server->inSend);
#endif
- rc = smb_send2(ses->server->ssocket, iov, n_vec,
- (struct sockaddr *) &(ses->server->addr.sockAddr));
+ rc = smb_send2(ses->server, iov, n_vec,
+ (struct sockaddr *) &(ses->server->addr.sockAddr),
+ ses->server->noblocksnd);
#ifdef CONFIG_CIFS_STATS2
atomic_dec(&ses->server->inSend);
midQ->when_sent = jiffies;
@@ -711,7 +729,8 @@
atomic_inc(&ses->server->inSend);
#endif
rc = smb_send(ses->server->ssocket, in_buf, in_buf->smb_buf_length,
- (struct sockaddr *) &(ses->server->addr.sockAddr));
+ (struct sockaddr *) &(ses->server->addr.sockAddr),
+ ses->server->noblocksnd);
#ifdef CONFIG_CIFS_STATS2
atomic_dec(&ses->server->inSend);
midQ->when_sent = jiffies;
@@ -851,7 +870,8 @@
return rc;
}
rc = smb_send(ses->server->ssocket, in_buf, in_buf->smb_buf_length,
- (struct sockaddr *) &(ses->server->addr.sockAddr));
+ (struct sockaddr *) &(ses->server->addr.sockAddr),
+ ses->server->noblocksnd);
up(&ses->server->tcpSem);
return rc;
}
@@ -941,7 +961,8 @@
atomic_inc(&ses->server->inSend);
#endif
rc = smb_send(ses->server->ssocket, in_buf, in_buf->smb_buf_length,
- (struct sockaddr *) &(ses->server->addr.sockAddr));
+ (struct sockaddr *) &(ses->server->addr.sockAddr),
+ ses->server->noblocksnd);
#ifdef CONFIG_CIFS_STATS2
atomic_dec(&ses->server->inSend);
midQ->when_sent = jiffies;