* [PATCH] CIFS: Fix read after write for files with read caching
2019-03-05 23:51 [PATCH 0/3] more reconnect fixes Pavel Shilovsky
@ 2019-03-05 23:51 ` Pavel Shilovsky
2019-03-05 23:51 ` [PATCH 1/3] CIFS: Only send SMB2_NEGOTIATE command on new TCP connections Pavel Shilovsky
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-03-05 23:51 UTC (permalink / raw)
To: linux-cifs, smfrench
When we have a READ lease for a file and have just issued a write
operation to the server we need to purge the cache and set oplock/lease
level to NONE to avoid reading stale data. Currently we do that
only if a write operation succedeed thus not covering cases when
a request was sent to the server but a negative error code was
returned later for some other reasons (e.g. -EIOCBQUEUED or -EINTR).
Fix this by turning off caching regardless of the error code being
returned.
The patches fixes generic tests 075 and 112 from the xfs-tests.
Cc: <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/file.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 9b53f33..4c144c1 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3096,14 +3096,16 @@ cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from)
* these pages but not on the region from pos to ppos+len-1.
*/
written = cifs_user_writev(iocb, from);
- if (written > 0 && CIFS_CACHE_READ(cinode)) {
+ if (CIFS_CACHE_READ(cinode)) {
/*
- * Windows 7 server can delay breaking level2 oplock if a write
- * request comes - break it on the client to prevent reading
- * an old data.
+ * We have read level caching and we have just sent a write
+ * request to the server thus making data in the cache stale.
+ * Zap the cache and set oplock/lease level to NONE to avoid
+ * reading stale data from the cache. All subsequent read
+ * operations will read new data from the server.
*/
cifs_zap_mapping(inode);
- cifs_dbg(FYI, "Set no oplock for inode=%p after a write operation\n",
+ cifs_dbg(FYI, "Set Oplock/Lease to NONE for inode=%p after write\n",
inode);
cinode->oplock = 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/3] CIFS: Only send SMB2_NEGOTIATE command on new TCP connections
2019-03-05 23:51 [PATCH 0/3] more reconnect fixes Pavel Shilovsky
2019-03-05 23:51 ` [PATCH] CIFS: Fix read after write for files with read caching Pavel Shilovsky
@ 2019-03-05 23:51 ` Pavel Shilovsky
2019-03-05 23:51 ` [PATCH 2/3] CIFS: Return -EAGAIN instead of -ENOTSOCK Pavel Shilovsky
2019-03-05 23:51 ` [PATCH 3/3] CIFS: Mask off signals when sending SMB packets Pavel Shilovsky
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-03-05 23:51 UTC (permalink / raw)
To: linux-cifs, smfrench
Do not allow commands other than SMB2_NEGOTIATE to be sent over
recently established TCP connections. Return -EAGAIN to let upper
layers handle it properly.
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/smb2transport.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index fa1fec2..d118157 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -619,6 +619,10 @@ smb2_get_mid_entry(struct cifs_ses *ses, struct smb2_sync_hdr *shdr,
return -EAGAIN;
}
+ if (ses->server->tcpStatus == CifsNeedNegotiate &&
+ shdr->Command != SMB2_NEGOTIATE)
+ return -EAGAIN;
+
if (ses->status == CifsNew) {
if ((shdr->Command != SMB2_SESSION_SETUP) &&
(shdr->Command != SMB2_NEGOTIATE))
@@ -702,6 +706,10 @@ smb2_setup_async_request(struct TCP_Server_Info *server, struct smb_rqst *rqst)
(struct smb2_sync_hdr *)rqst->rq_iov[0].iov_base;
struct mid_q_entry *mid;
+ if (server->tcpStatus == CifsNeedNegotiate &&
+ shdr->Command != SMB2_NEGOTIATE)
+ return ERR_PTR(-EAGAIN);
+
smb2_seq_num_into_buf(server, shdr);
mid = smb2_mid_entry_alloc(shdr, server);
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] CIFS: Return -EAGAIN instead of -ENOTSOCK
2019-03-05 23:51 [PATCH 0/3] more reconnect fixes Pavel Shilovsky
2019-03-05 23:51 ` [PATCH] CIFS: Fix read after write for files with read caching Pavel Shilovsky
2019-03-05 23:51 ` [PATCH 1/3] CIFS: Only send SMB2_NEGOTIATE command on new TCP connections Pavel Shilovsky
@ 2019-03-05 23:51 ` Pavel Shilovsky
2019-03-05 23:51 ` [PATCH 3/3] CIFS: Mask off signals when sending SMB packets Pavel Shilovsky
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-03-05 23:51 UTC (permalink / raw)
To: linux-cifs, smfrench
When we attempt to send a packet while the demultiplex thread
is in the middle of cifs_reconnect() we may end up returning
-ENOTSOCK to upper layers. The intent here is to retry the request
once the TCP connection is up, so change it to return -EAGAIN
instead. The latter error code is retryable and the upper layers
will retry the request if needed.
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/transport.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 9c3a680..9f23a45 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -301,8 +301,9 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
rc = smbd_send(server, rqst);
goto smbd_done;
}
+
if (ssocket == NULL)
- return -ENOTSOCK;
+ return -EAGAIN;
/* cork the socket */
kernel_setsockopt(ssocket, SOL_TCP, TCP_CORK,
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] CIFS: Mask off signals when sending SMB packets
2019-03-05 23:51 [PATCH 0/3] more reconnect fixes Pavel Shilovsky
` (2 preceding siblings ...)
2019-03-05 23:51 ` [PATCH 2/3] CIFS: Return -EAGAIN instead of -ENOTSOCK Pavel Shilovsky
@ 2019-03-05 23:51 ` Pavel Shilovsky
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-03-05 23:51 UTC (permalink / raw)
To: linux-cifs, smfrench
We don't want to break SMB sessions if we receive signals when
sending packets through the network. Fix it by masking off signals
inside __smb_send_rqst() to avoid partial packet sends due to
interrupts.
Return -EINTR if a signal is pending and only a part of the packet
was sent. Return a success status code if the whole packet was sent
regardless of signal being pending or not. This keeps a mid entry
for the request in the pending queue and allows the demultiplex
thread to handle a response from the server properly.
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/transport.c | 41 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 9f23a45..7ce8a58 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -33,6 +33,7 @@
#include <linux/uaccess.h>
#include <asm/processor.h>
#include <linux/mempool.h>
+#include <linux/signal.h>
#include "cifspdu.h"
#include "cifsglob.h"
#include "cifsproto.h"
@@ -291,6 +292,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
int n_vec;
unsigned int send_length = 0;
unsigned int i, j;
+ sigset_t mask, oldmask;
size_t total_len = 0, sent, size;
struct socket *ssocket = server->ssocket;
struct msghdr smb_msg;
@@ -305,6 +307,11 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
if (ssocket == NULL)
return -EAGAIN;
+ if (signal_pending(current)) {
+ cifs_dbg(FYI, "signal is pending before sending any data\n");
+ return -EINTR;
+ }
+
/* cork the socket */
kernel_setsockopt(ssocket, SOL_TCP, TCP_CORK,
(char *)&val, sizeof(val));
@@ -313,6 +320,16 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
send_length += smb_rqst_len(server, &rqst[j]);
rfc1002_marker = cpu_to_be32(send_length);
+ /*
+ * We should not allow signals to interrupt the network send because
+ * any partial send will cause session reconnects thus increasing
+ * latency of system calls and overload a server with unnecessary
+ * requests.
+ */
+
+ sigfillset(&mask);
+ sigprocmask(SIG_BLOCK, &mask, &oldmask);
+
/* Generate a rfc1002 marker for SMB2+ */
if (server->vals->header_preamble_size == 0) {
struct kvec hiov = {
@@ -322,7 +339,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
iov_iter_kvec(&smb_msg.msg_iter, WRITE, &hiov, 1, 4);
rc = smb_send_kvec(server, &smb_msg, &sent);
if (rc < 0)
- goto uncork;
+ goto unmask;
total_len += sent;
send_length += 4;
@@ -344,7 +361,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
rc = smb_send_kvec(server, &smb_msg, &sent);
if (rc < 0)
- goto uncork;
+ goto unmask;
total_len += sent;
@@ -366,7 +383,25 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
}
}
-uncork:
+unmask:
+ sigprocmask(SIG_SETMASK, &oldmask, NULL);
+
+ /*
+ * If signal is pending but we have already sent the whole packet to
+ * the server we need to return success status to allow a corresponding
+ * mid entry to be kept in the pending requests queue thus allowing
+ * to handle responses from the server by the client.
+ *
+ * If only part of the packet has been sent there is no need to hide
+ * interrupt because the session will be reconnected anyway, so there
+ * won't be any response from the server to handle.
+ */
+
+ if (signal_pending(current) && (total_len != send_length)) {
+ cifs_dbg(FYI, "signal is pending after attempt to send\n");
+ rc = -EINTR;
+ }
+
/* uncork it */
val = 0;
kernel_setsockopt(ssocket, SOL_TCP, TCP_CORK,
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread