* [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling
@ 2024-01-21 3:32 nspmangalore
2024-01-21 3:32 ` [PATCH 2/7] cifs: cifs_pick_channel should try selecting active channels nspmangalore
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
Some servers like Azure SMB servers always advertise multichannel
capability in server capabilities list. Such servers return error
STATUS_NOT_IMPLEMENTED for ioctl calls to query server interfaces,
and expect clients to consider that as a sign that they do not support
multichannel.
We already handled this at mount time. Soon after the tree connect,
we query server interfaces. And when server returned STATUS_NOT_IMPLEMENTED,
we kept interface list as empty. When cifs_try_adding_channels gets
called, it would not find any interfaces, so will not add channels.
For the case where an active multichannel mount exists, and multichannel
is disabled by such a server, this change will now allow the client
to disable secondary channels on the mount. It will check the return
status of query server interfaces call soon after a tree reconnect.
If the return status is EOPNOTSUPP, then instead of the check to add
more channels, we'll disable the secondary channels instead.
For better code reuse, this change also moves the common code for
disabling multichannel to a helper function.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/smb2pdu.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 288199f0b987..5126f5f97969 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -167,7 +167,7 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
if (SERVER_IS_CHAN(server)) {
cifs_dbg(VFS,
- "server %s does not support multichannel anymore. Skip secondary channel\n",
+ "server %s does not support multichannel anymore. Skip secondary channel\n",
ses->server->hostname);
spin_lock(&ses->chan_lock);
@@ -195,15 +195,13 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
pserver = server->primary_server;
cifs_signal_cifsd_for_reconnect(pserver, false);
skip_terminate:
- mutex_unlock(&ses->session_mutex);
return -EHOSTDOWN;
}
cifs_server_dbg(VFS,
- "server does not support multichannel anymore. Disable all other channels\n");
+ "server does not support multichannel anymore. Disable all other channels\n");
cifs_disable_secondary_channels(ses);
-
return 0;
}
@@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
}
skip_add_channels:
+skip_add_channels:
if (smb2_command != SMB2_INTERNAL_CMD)
mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/7] cifs: cifs_pick_channel should try selecting active channels
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
@ 2024-01-21 3:32 ` nspmangalore
2024-01-21 3:32 ` [PATCH 3/7] cifs: smb2_close_getattr should also update i_size nspmangalore
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
cifs_pick_channel today just selects a channel based
on the policy of least loaded channel. However, it
does not take into account if the channel needs
reconnect. As a result, we can have failures in send
that can be completely avoided.
This change doesn't make a channel a candidate for
this selection if it needs reconnect.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/transport.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 4f717ad7c21b..8695c9961f5a 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -1026,6 +1026,9 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
if (!server || server->terminate)
continue;
+ if (CIFS_CHAN_NEEDS_RECONNECT(ses, i))
+ continue;
+
/*
* strictly speaking, we should pick up req_lock to read
* server->in_flight. But it shouldn't matter much here if we
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/7] cifs: smb2_close_getattr should also update i_size
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
2024-01-21 3:32 ` [PATCH 2/7] cifs: cifs_pick_channel should try selecting active channels nspmangalore
@ 2024-01-21 3:32 ` nspmangalore
2024-01-23 7:22 ` Shyam Prasad N
2024-01-21 3:32 ` [PATCH 4/7] cifs: translate network errors on send to -ECONNABORTED nspmangalore
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB
flag set is already used by the code for SMB3+.
smb2_close_getattr is the function that uses this to
update the inode attributes.
However, we were skipping the EndOfFile info that's returned
by the server. There is a small chance that the file size
may have been changed in the small window between the client
sending the close request (thereby giving up lease if it had)
to the point that the server returns the response.
This change uses the field to update the inode size.
Also, it is a valid case for a zero AllocationSize to be returned
by the server for the file. We were discarding such values, thereby
resulting in stale i_blocks value. Fixed that here too.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/smb2ops.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index d9553c2556a2..e23577584ed6 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1433,9 +1433,9 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
* but instead 512 byte (2**9) size is required for
* calculating num blocks.
*/
- if (le64_to_cpu(file_inf.AllocationSize) > 4096)
- inode->i_blocks =
- (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
+ inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
+
+ inode->i_size = le64_to_cpu(file_inf.EndOfFile);
/* End of file and Attributes should not have to be updated on close */
spin_unlock(&inode->i_lock);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/7] cifs: translate network errors on send to -ECONNABORTED
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
2024-01-21 3:32 ` [PATCH 2/7] cifs: cifs_pick_channel should try selecting active channels nspmangalore
2024-01-21 3:32 ` [PATCH 3/7] cifs: smb2_close_getattr should also update i_size nspmangalore
@ 2024-01-21 3:32 ` nspmangalore
2024-01-21 3:32 ` [PATCH 5/7] cifs: helper function to check replayable error codes nspmangalore
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
When the network stack returns various errors, we today bubble
up the error to the user (in case of soft mounts).
This change translates all network errors except -EINTR and
-EAGAIN to -ECONNABORTED. A similar approach is taken when
we receive network errors when reading from the socket.
The change also forces the cifsd thread to reconnect during
it's next activity.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/transport.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 8695c9961f5a..e00278fcfa4f 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -400,10 +400,17 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
server->conn_id, server->hostname);
}
smbd_done:
- if (rc < 0 && rc != -EINTR)
+ /*
+ * there's hardly any use for the layers above to know the
+ * actual error code here. All they should do at this point is
+ * to retry the connection and hope it goes away.
+ */
+ if (rc < 0 && rc != -EINTR && rc != -EAGAIN) {
cifs_server_dbg(VFS, "Error %d sending data on socket to server\n",
rc);
- else if (rc > 0)
+ rc = -ECONNABORTED;
+ cifs_signal_cifsd_for_reconnect(server, false);
+ } else if (rc > 0)
rc = 0;
out:
cifs_in_send_dec(server);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/7] cifs: helper function to check replayable error codes
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
` (2 preceding siblings ...)
2024-01-21 3:32 ` [PATCH 4/7] cifs: translate network errors on send to -ECONNABORTED nspmangalore
@ 2024-01-21 3:32 ` nspmangalore
2024-01-21 3:32 ` [PATCH 6/7] cifs: commands that are retried should have replay flag set nspmangalore
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
The code to check for replay is not just -EAGAIN. In some
cases, the send request or receive response may result in
network errors, which we're now mapping to -ECONNABORTED.
This change introduces a helper function which checks
if the error returned in one of the above two errors.
And all checks for replays will now use this helper.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/cached_dir.c | 1 +
fs/smb/client/cifsglob.h | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 971892620504..5730c65ffb40 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -367,6 +367,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
atomic_inc(&tcon->num_remote_opens);
}
kfree(utf16_path);
+
return rc;
}
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 20036fb16cec..6e4cfaec98e3 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1831,6 +1831,13 @@ static inline bool is_retryable_error(int error)
return false;
}
+static inline bool is_replayable_error(int error)
+{
+ if (error == -EAGAIN || error == -ECONNABORTED)
+ return true;
+ return false;
+}
+
/* cifs_get_writable_file() flags */
#define FIND_WR_ANY 0
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/7] cifs: commands that are retried should have replay flag set
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
` (3 preceding siblings ...)
2024-01-21 3:32 ` [PATCH 5/7] cifs: helper function to check replayable error codes nspmangalore
@ 2024-01-21 3:32 ` nspmangalore
2024-01-21 3:32 ` [PATCH 7/7] cifs: set replay flag for retries of write command nspmangalore
2024-01-21 4:02 ` [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling Steve French
6 siblings, 0 replies; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
MS-SMB2 states that the header flag SMB2_FLAGS_REPLAY_OPERATION
needs to be set when a command needs to be retried, so that
the server is aware that this is a replay for an operation that
appeared before.
This can be very important, for example, for state changing
operations and opens which get retried following a reconnect;
since the client maybe unaware of the status of the previous
open.
This is particularly important for multichannel scenario, since
disconnection of one connection does not mean that the session
is lost. The requests can be replayed on another channel.
This change also makes use of exponential back-off before replays
and also limits the number of retries to "retrans" mount option
value.
Also, this change does not modify the read/write codepath.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/cached_dir.c | 23 +++-
fs/smb/client/cifsglob.h | 5 +
fs/smb/client/smb2inode.c | 33 ++++-
fs/smb/client/smb2ops.c | 123 ++++++++++++++++--
fs/smb/client/smb2pdu.c | 260 +++++++++++++++++++++++++++++++++----
fs/smb/client/smb2proto.h | 5 +
6 files changed, 404 insertions(+), 45 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 5730c65ffb40..1daeb5714faa 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -145,21 +145,27 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
struct cached_fid *cfid;
struct cached_fids *cfids;
const char *npath;
+ int retries = 0, cur_sleep = 1;
if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
is_smb1_server(tcon->ses->server) || (dir_cache_timeout == 0))
return -EOPNOTSUPP;
ses = tcon->ses;
- server = cifs_pick_channel(ses);
cfids = tcon->cfids;
- if (!server->ops->new_lease_key)
- return -EIO;
-
if (cifs_sb->root == NULL)
return -ENOENT;
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ oplock = SMB2_OPLOCK_LEVEL_II;
+ server = cifs_pick_channel(ses);
+
+ if (!server->ops->new_lease_key)
+ return -EIO;
+
utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
if (!utf16_path)
return -ENOMEM;
@@ -268,6 +274,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
*/
cfid->has_lease = true;
+ if (retries) {
+ smb2_set_replay(server, &rqst[0]);
+ smb2_set_replay(server, &rqst[1]);
+ }
+
rc = compound_send_recv(xid, ses, server,
flags, 2, rqst,
resp_buftype, rsp_iov);
@@ -368,6 +379,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
}
kfree(utf16_path);
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 6e4cfaec98e3..b5abe4d6f478 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -49,6 +49,11 @@
*/
#define CIFS_DEF_ACTIMEO (1 * HZ)
+/*
+ * max sleep time before retry to server
+ */
+#define CIFS_MAX_SLEEP 2000
+
/*
* max attribute cache timeout (jiffies) - 2^30
*/
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index a652200540c8..05818cd6d932 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -120,6 +120,14 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
unsigned int size[2];
void *data[2];
int len;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ oplock = SMB2_OPLOCK_LEVEL_NONE;
+ num_rqst = 0;
+ server = cifs_pick_channel(ses);
vars = kzalloc(sizeof(*vars), GFP_ATOMIC);
if (vars == NULL)
@@ -127,8 +135,6 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
rqst = &vars->rqst[0];
rsp_iov = &vars->rsp_iov[0];
- server = cifs_pick_channel(ses);
-
if (smb3_encryption_required(tcon))
flags |= CIFS_TRANSFORM_REQ;
@@ -463,15 +469,24 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
num_rqst++;
if (cfile) {
+ if (retries)
+ for (i = 1; i < num_rqst - 2; i++)
+ smb2_set_replay(server, &rqst[i]);
+
rc = compound_send_recv(xid, ses, server,
flags, num_rqst - 2,
&rqst[1], &resp_buftype[1],
&rsp_iov[1]);
- } else
+ } else {
+ if (retries)
+ for (i = 0; i < num_rqst; i++)
+ smb2_set_replay(server, &rqst[i]);
+
rc = compound_send_recv(xid, ses, server,
flags, num_rqst,
rqst, resp_buftype,
rsp_iov);
+ }
finished:
num_rqst = 0;
@@ -620,9 +635,6 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
}
SMB2_close_free(&rqst[num_rqst]);
- if (cfile)
- cifsFileInfo_put(cfile);
-
num_cmds += 2;
if (out_iov && out_buftype) {
memcpy(out_iov, rsp_iov, num_cmds * sizeof(*out_iov));
@@ -632,7 +644,16 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
for (i = 0; i < num_cmds; i++)
free_rsp_buf(resp_buftype[i], rsp_iov[i].iov_base);
}
+ num_cmds -= 2; /* correct num_cmds as there could be a retry */
kfree(vars);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
+ if (cfile)
+ cifsFileInfo_put(cfile);
+
return rc;
}
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index e23577584ed6..17df0cd78698 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1108,7 +1108,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
{
struct smb2_compound_vars *vars;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
struct smb_rqst *rqst;
struct kvec *rsp_iov;
__le16 *utf16_path = NULL;
@@ -1124,6 +1124,13 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
struct smb2_file_full_ea_info *ea = NULL;
struct smb2_query_info_rsp *rsp;
int rc, used_len = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = CIFS_CP_CREATE_CLOSE_OP;
+ oplock = SMB2_OPLOCK_LEVEL_NONE;
+ server = cifs_pick_channel(ses);
if (smb3_encryption_required(tcon))
flags |= CIFS_TRANSFORM_REQ;
@@ -1244,6 +1251,12 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
goto sea_exit;
smb2_set_related(&rqst[2]);
+ if (retries) {
+ smb2_set_replay(server, &rqst[0]);
+ smb2_set_replay(server, &rqst[1]);
+ smb2_set_replay(server, &rqst[2]);
+ }
+
rc = compound_send_recv(xid, ses, server,
flags, 3, rqst,
resp_buftype, rsp_iov);
@@ -1260,6 +1273,11 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
kfree(vars);
out_free_path:
kfree(utf16_path);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
#endif
@@ -1484,7 +1502,7 @@ smb2_ioctl_query_info(const unsigned int xid,
struct smb_rqst *rqst;
struct kvec *rsp_iov;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
char __user *arg = (char __user *)p;
struct smb_query_info qi;
struct smb_query_info __user *pqi;
@@ -1501,6 +1519,13 @@ smb2_ioctl_query_info(const unsigned int xid,
void *data[2];
int create_options = is_dir ? CREATE_NOT_FILE : CREATE_NOT_DIR;
void (*free_req1_func)(struct smb_rqst *r);
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = CIFS_CP_CREATE_CLOSE_OP;
+ oplock = SMB2_OPLOCK_LEVEL_NONE;
+ server = cifs_pick_channel(ses);
vars = kzalloc(sizeof(*vars), GFP_ATOMIC);
if (vars == NULL)
@@ -1641,6 +1666,12 @@ smb2_ioctl_query_info(const unsigned int xid,
goto free_req_1;
smb2_set_related(&rqst[2]);
+ if (retries) {
+ smb2_set_replay(server, &rqst[0]);
+ smb2_set_replay(server, &rqst[1]);
+ smb2_set_replay(server, &rqst[2]);
+ }
+
rc = compound_send_recv(xid, ses, server,
flags, 3, rqst,
resp_buftype, rsp_iov);
@@ -1701,6 +1732,11 @@ smb2_ioctl_query_info(const unsigned int xid,
kfree(buffer);
free_vars:
kfree(vars);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -2227,8 +2263,14 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
struct cifs_open_parms oparms;
struct smb2_query_directory_rsp *qd_rsp = NULL;
struct smb2_create_rsp *op_rsp = NULL;
- struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
- int retry_count = 0;
+ struct TCP_Server_Info *server;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ oplock = SMB2_OPLOCK_LEVEL_NONE;
+ server = cifs_pick_channel(tcon->ses);
utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
if (!utf16_path)
@@ -2278,14 +2320,15 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
smb2_set_related(&rqst[1]);
-again:
+ if (retries) {
+ smb2_set_replay(server, &rqst[0]);
+ smb2_set_replay(server, &rqst[1]);
+ }
+
rc = compound_send_recv(xid, tcon->ses, server,
flags, 2, rqst,
resp_buftype, rsp_iov);
- if (rc == -EAGAIN && retry_count++ < 10)
- goto again;
-
/* If the open failed there is nothing to do */
op_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
if (op_rsp == NULL || op_rsp->hdr.Status != STATUS_SUCCESS) {
@@ -2333,6 +2376,11 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
SMB2_query_directory_free(&rqst[1]);
free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -2457,6 +2505,22 @@ smb2_oplock_response(struct cifs_tcon *tcon, __u64 persistent_fid,
CIFS_CACHE_READ(cinode) ? 1 : 0);
}
+void
+smb2_set_replay(struct TCP_Server_Info *server, struct smb_rqst *rqst)
+{
+ struct smb2_hdr *shdr;
+
+ if (server->dialect < SMB30_PROT_ID)
+ return;
+
+ shdr = (struct smb2_hdr *)(rqst->rq_iov[0].iov_base);
+ if (shdr == NULL) {
+ cifs_dbg(FYI, "shdr NULL in smb2_set_related\n");
+ return;
+ }
+ shdr->Flags |= SMB2_FLAGS_REPLAY_OPERATION;
+}
+
void
smb2_set_related(struct smb_rqst *rqst)
{
@@ -2529,6 +2593,27 @@ smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
shdr->NextCommand = cpu_to_le32(len);
}
+/*
+ * helper function for exponential backoff and check if replayable
+ */
+bool smb2_should_replay(struct cifs_tcon *tcon,
+ int *pretries,
+ int *pcur_sleep)
+{
+ if (!pretries || !pcur_sleep)
+ return false;
+
+ if (tcon->retry || (*pretries)++ < tcon->ses->server->retrans) {
+ msleep(*pcur_sleep);
+ (*pcur_sleep) = ((*pcur_sleep) << 1);
+ if ((*pcur_sleep) > CIFS_MAX_SLEEP)
+ (*pcur_sleep) = CIFS_MAX_SLEEP;
+ return true;
+ }
+
+ return false;
+}
+
/*
* Passes the query info response back to the caller on success.
* Caller need to free this with free_rsp_buf().
@@ -2542,7 +2627,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
{
struct smb2_compound_vars *vars;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
int flags = CIFS_CP_CREATE_CLOSE_OP;
struct smb_rqst *rqst;
int resp_buftype[3];
@@ -2553,6 +2638,13 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
int rc;
__le16 *utf16_path;
struct cached_fid *cfid = NULL;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = CIFS_CP_CREATE_CLOSE_OP;
+ oplock = SMB2_OPLOCK_LEVEL_NONE;
+ server = cifs_pick_channel(ses);
if (!path)
path = "";
@@ -2633,6 +2725,14 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
goto qic_exit;
smb2_set_related(&rqst[2]);
+ if (retries) {
+ if (!cfid) {
+ smb2_set_replay(server, &rqst[0]);
+ smb2_set_replay(server, &rqst[2]);
+ }
+ smb2_set_replay(server, &rqst[1]);
+ }
+
if (cfid) {
rc = compound_send_recv(xid, ses, server,
flags, 1, &rqst[1],
@@ -2665,6 +2765,11 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
kfree(vars);
out_free_path:
kfree(utf16_path);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 5126f5f97969..0291482a3f51 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2764,7 +2764,14 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
int flags = 0;
unsigned int total_len;
__le16 *utf16_path = NULL;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ n_iov = 2;
+ server = cifs_pick_channel(ses);
cifs_dbg(FYI, "mkdir\n");
@@ -2868,6 +2875,10 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
/* no need to inc num_remote_opens because we close it just below */
trace_smb3_posix_mkdir_enter(xid, tcon->tid, ses->Suid, full_path, CREATE_NOT_FILE,
FILE_WRITE_ATTRIBUTES);
+
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
/* resource #4: response buffer */
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
@@ -2905,6 +2916,11 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
cifs_small_buf_release(req);
err_free_path:
kfree(utf16_path);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -3100,12 +3116,18 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
struct smb2_create_rsp *rsp = NULL;
struct cifs_tcon *tcon = oparms->tcon;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
struct kvec iov[SMB2_CREATE_IOV_SIZE];
struct kvec rsp_iov = {NULL, 0};
int resp_buftype = CIFS_NO_BUFFER;
int rc = 0;
int flags = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
cifs_dbg(FYI, "create/open\n");
if (!ses || !server)
@@ -3127,6 +3149,9 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
trace_smb3_open_enter(xid, tcon->tid, tcon->ses->Suid, oparms->path,
oparms->create_options, oparms->desired_access);
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags,
&rsp_iov);
@@ -3180,6 +3205,11 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
creat_exit:
SMB2_open_free(&rqst);
free_rsp_buf(resp_buftype, rsp);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -3304,15 +3334,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
int resp_buftype = CIFS_NO_BUFFER;
int rc = 0;
int flags = 0;
-
- cifs_dbg(FYI, "SMB2 IOCTL\n");
-
- if (out_data != NULL)
- *out_data = NULL;
-
- /* zero out returned data len, in case of error */
- if (plen)
- *plen = 0;
+ int retries = 0, cur_sleep = 1;
if (!tcon)
return -EIO;
@@ -3321,10 +3343,23 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
if (!ses)
return -EIO;
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
server = cifs_pick_channel(ses);
+
if (!server)
return -EIO;
+ cifs_dbg(FYI, "SMB2 IOCTL\n");
+
+ if (out_data != NULL)
+ *out_data = NULL;
+
+ /* zero out returned data len, in case of error */
+ if (plen)
+ *plen = 0;
+
if (smb3_encryption_required(tcon))
flags |= CIFS_TRANSFORM_REQ;
@@ -3339,6 +3374,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
if (rc)
goto ioctl_exit;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags,
&rsp_iov);
@@ -3408,6 +3446,11 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
ioctl_exit:
SMB2_ioctl_free(&rqst);
free_rsp_buf(resp_buftype, rsp);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -3479,13 +3522,20 @@ __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
struct smb_rqst rqst;
struct smb2_close_rsp *rsp = NULL;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
struct kvec iov[1];
struct kvec rsp_iov;
int resp_buftype = CIFS_NO_BUFFER;
int rc = 0;
int flags = 0;
bool query_attrs = false;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ query_attrs = false;
+ server = cifs_pick_channel(ses);
cifs_dbg(FYI, "Close\n");
@@ -3511,6 +3561,9 @@ __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
if (rc)
goto close_exit;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
rsp = (struct smb2_close_rsp *)rsp_iov.iov_base;
@@ -3544,6 +3597,11 @@ __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
cifs_dbg(VFS, "handle cancelled close fid 0x%llx returned error %d\n",
persistent_fid, tmp_rc);
}
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -3674,12 +3732,19 @@ query_info(const unsigned int xid, struct cifs_tcon *tcon,
struct TCP_Server_Info *server;
int flags = 0;
bool allocated = false;
+ int retries = 0, cur_sleep = 1;
cifs_dbg(FYI, "Query Info\n");
if (!ses)
return -EIO;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ allocated = false;
server = cifs_pick_channel(ses);
+
if (!server)
return -EIO;
@@ -3701,6 +3766,9 @@ query_info(const unsigned int xid, struct cifs_tcon *tcon,
trace_smb3_query_info_enter(xid, persistent_fid, tcon->tid,
ses->Suid, info_class, (__u32)info_type);
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
rsp = (struct smb2_query_info_rsp *)rsp_iov.iov_base;
@@ -3743,6 +3811,11 @@ query_info(const unsigned int xid, struct cifs_tcon *tcon,
qinf_exit:
SMB2_query_info_free(&rqst);
free_rsp_buf(resp_buftype, rsp);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -3843,7 +3916,7 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
u32 *plen /* returned data len */)
{
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
struct smb_rqst rqst;
struct smb2_change_notify_rsp *smb_rsp;
struct kvec iov[1];
@@ -3851,6 +3924,12 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
int resp_buftype = CIFS_NO_BUFFER;
int flags = 0;
int rc = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
cifs_dbg(FYI, "change notify\n");
if (!ses || !server)
@@ -3875,6 +3954,10 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
trace_smb3_notify_enter(xid, persistent_fid, tcon->tid, ses->Suid,
(u8)watch_tree, completion_filter);
+
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
@@ -3909,6 +3992,11 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
if (rqst.rq_iov)
cifs_small_buf_release(rqst.rq_iov[0].iov_base); /* request */
free_rsp_buf(resp_buftype, rsp_iov.iov_base);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -4151,10 +4239,16 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
struct smb_rqst rqst;
struct kvec iov[1];
struct kvec rsp_iov = {NULL, 0};
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
int resp_buftype = CIFS_NO_BUFFER;
int flags = 0;
int rc = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
cifs_dbg(FYI, "flush\n");
if (!ses || !(ses->server))
@@ -4174,6 +4268,10 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
goto flush_exit;
trace_smb3_flush_enter(xid, persistent_fid, tcon->tid, ses->Suid);
+
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
@@ -4188,6 +4286,11 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
flush_exit:
SMB2_flush_free(&rqst);
free_rsp_buf(resp_buftype, rsp_iov.iov_base);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -4825,18 +4928,21 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
int flags = 0;
unsigned int total_len;
struct TCP_Server_Info *server;
+ int retries = 0, cur_sleep = 1;
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
*nbytes = 0;
-
- if (n_vec < 1)
- return rc;
-
if (!io_parms->server)
io_parms->server = cifs_pick_channel(io_parms->tcon->ses);
server = io_parms->server;
if (server == NULL)
return -ECONNABORTED;
+ if (n_vec < 1)
+ return rc;
+
rc = smb2_plain_req_init(SMB2_WRITE, io_parms->tcon, server,
(void **) &req, &total_len);
if (rc)
@@ -4870,6 +4976,9 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
rqst.rq_iov = iov;
rqst.rq_nvec = n_vec + 1;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, io_parms->tcon->ses, server,
&rqst,
&resp_buftype, flags, &rsp_iov);
@@ -4894,6 +5003,11 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms,
cifs_small_buf_release(req);
free_rsp_buf(resp_buftype, rsp);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(io_parms->tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5205,8 +5319,14 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
struct kvec rsp_iov;
int rc = 0;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
int flags = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
if (!ses || !(ses->server))
return -EIO;
@@ -5226,6 +5346,9 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
if (rc)
goto qdir_exit;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
rsp = (struct smb2_query_directory_rsp *)rsp_iov.iov_base;
@@ -5260,6 +5383,11 @@ SMB2_query_directory(const unsigned int xid, struct cifs_tcon *tcon,
qdir_exit:
SMB2_query_directory_free(&rqst);
free_rsp_buf(resp_buftype, rsp);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5326,8 +5454,14 @@ send_set_info(const unsigned int xid, struct cifs_tcon *tcon,
int rc = 0;
int resp_buftype;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
int flags = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
if (!ses || !server)
return -EIO;
@@ -5355,6 +5489,8 @@ send_set_info(const unsigned int xid, struct cifs_tcon *tcon,
return rc;
}
+ if (retries)
+ smb2_set_replay(server, &rqst);
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags,
@@ -5370,6 +5506,11 @@ send_set_info(const unsigned int xid, struct cifs_tcon *tcon,
free_rsp_buf(resp_buftype, rsp);
kfree(iov);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5422,12 +5563,18 @@ SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
int rc;
struct smb2_oplock_break *req = NULL;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
int flags = CIFS_OBREAK_OP;
unsigned int total_len;
struct kvec iov[1];
struct kvec rsp_iov;
int resp_buf_type;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = CIFS_OBREAK_OP;
+ server = cifs_pick_channel(ses);
cifs_dbg(FYI, "SMB2_oplock_break\n");
rc = smb2_plain_req_init(SMB2_OPLOCK_BREAK, tcon, server,
@@ -5452,15 +5599,21 @@ SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
rqst.rq_iov = iov;
rqst.rq_nvec = 1;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buf_type, flags, &rsp_iov);
cifs_small_buf_release(req);
-
if (rc) {
cifs_stats_fail_inc(tcon, SMB2_OPLOCK_BREAK_HE);
cifs_dbg(FYI, "Send error in Oplock Break = %d\n", rc);
}
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5546,9 +5699,15 @@ SMB311_posix_qfs_info(const unsigned int xid, struct cifs_tcon *tcon,
int rc = 0;
int resp_buftype;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
FILE_SYSTEM_POSIX_INFO *info = NULL;
int flags = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
rc = build_qfs_info_req(&iov, tcon, server,
FS_POSIX_INFORMATION,
@@ -5564,6 +5723,9 @@ SMB311_posix_qfs_info(const unsigned int xid, struct cifs_tcon *tcon,
rqst.rq_iov = &iov;
rqst.rq_nvec = 1;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
free_qfs_info_req(&iov);
@@ -5583,6 +5745,11 @@ SMB311_posix_qfs_info(const unsigned int xid, struct cifs_tcon *tcon,
posix_qfsinf_exit:
free_rsp_buf(resp_buftype, rsp_iov.iov_base);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5597,9 +5764,15 @@ SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
int rc = 0;
int resp_buftype;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
struct smb2_fs_full_size_info *info = NULL;
int flags = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
rc = build_qfs_info_req(&iov, tcon, server,
FS_FULL_SIZE_INFORMATION,
@@ -5615,6 +5788,9 @@ SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
rqst.rq_iov = &iov;
rqst.rq_nvec = 1;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
free_qfs_info_req(&iov);
@@ -5634,6 +5810,11 @@ SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
qfsinf_exit:
free_rsp_buf(resp_buftype, rsp_iov.iov_base);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5648,9 +5829,15 @@ SMB2_QFS_attr(const unsigned int xid, struct cifs_tcon *tcon,
int rc = 0;
int resp_buftype, max_len, min_len;
struct cifs_ses *ses = tcon->ses;
- struct TCP_Server_Info *server = cifs_pick_channel(ses);
+ struct TCP_Server_Info *server;
unsigned int rsp_len, offset;
int flags = 0;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = 0;
+ server = cifs_pick_channel(ses);
if (level == FS_DEVICE_INFORMATION) {
max_len = sizeof(FILE_SYSTEM_DEVICE_INFO);
@@ -5682,6 +5869,9 @@ SMB2_QFS_attr(const unsigned int xid, struct cifs_tcon *tcon,
rqst.rq_iov = &iov;
rqst.rq_nvec = 1;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, ses, server,
&rqst, &resp_buftype, flags, &rsp_iov);
free_qfs_info_req(&iov);
@@ -5719,6 +5909,11 @@ SMB2_QFS_attr(const unsigned int xid, struct cifs_tcon *tcon,
qfsattr_exit:
free_rsp_buf(resp_buftype, rsp_iov.iov_base);
+
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
@@ -5736,7 +5931,13 @@ smb2_lockv(const unsigned int xid, struct cifs_tcon *tcon,
unsigned int count;
int flags = CIFS_NO_RSP_BUF;
unsigned int total_len;
- struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
+ struct TCP_Server_Info *server;
+ int retries = 0, cur_sleep = 1;
+
+replay_again:
+ /* reinitialize for possible replay */
+ flags = CIFS_NO_RSP_BUF;
+ server = cifs_pick_channel(tcon->ses);
cifs_dbg(FYI, "smb2_lockv num lock %d\n", num_lock);
@@ -5767,6 +5968,9 @@ smb2_lockv(const unsigned int xid, struct cifs_tcon *tcon,
rqst.rq_iov = iov;
rqst.rq_nvec = 2;
+ if (retries)
+ smb2_set_replay(server, &rqst);
+
rc = cifs_send_recv(xid, tcon->ses, server,
&rqst, &resp_buf_type, flags,
&rsp_iov);
@@ -5778,6 +5982,10 @@ smb2_lockv(const unsigned int xid, struct cifs_tcon *tcon,
tcon->ses->Suid, rc);
}
+ if (is_replayable_error(rc) &&
+ smb2_should_replay(tcon, &retries, &cur_sleep))
+ goto replay_again;
+
return rc;
}
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index 0034b537b0b3..b3069911e9dd 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -122,6 +122,11 @@ extern unsigned long smb_rqst_len(struct TCP_Server_Info *server,
extern void smb2_set_next_command(struct cifs_tcon *tcon,
struct smb_rqst *rqst);
extern void smb2_set_related(struct smb_rqst *rqst);
+extern void smb2_set_replay(struct TCP_Server_Info *server,
+ struct smb_rqst *rqst);
+extern bool smb2_should_replay(struct cifs_tcon *tcon,
+ int *pretries,
+ int *pcur_sleep);
/*
* SMB2 Worker functions - most of protocol specific implementation details
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/7] cifs: set replay flag for retries of write command
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
` (4 preceding siblings ...)
2024-01-21 3:32 ` [PATCH 6/7] cifs: commands that are retried should have replay flag set nspmangalore
@ 2024-01-21 3:32 ` nspmangalore
2024-01-21 4:02 ` Steve French
2024-01-21 4:02 ` [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling Steve French
6 siblings, 1 reply; 13+ messages in thread
From: nspmangalore @ 2024-01-21 3:32 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
From: Shyam Prasad N <sprasad@microsoft.com>
Similar to the rest of the commands, this is a change
to add replay flags on retry. This one does not add a
back-off, considering that we may want to flush a write
ASAP to the server. Considering that this will be a
flush of cached pages, the retrans value is also not
honoured.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
fs/smb/client/cifsglob.h | 1 +
fs/smb/client/file.c | 1 +
fs/smb/client/smb2pdu.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index b5abe4d6f478..acda357e1dfd 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1506,6 +1506,7 @@ struct cifs_writedata {
struct smbd_mr *mr;
#endif
struct cifs_credits credits;
+ bool replay;
};
/*
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 1b4262aff8fa..49d262d1df5f 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -3300,6 +3300,7 @@ cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
if (wdata->cfile->invalidHandle)
rc = -EAGAIN;
else {
+ wdata->replay = true;
#ifdef CONFIG_CIFS_SMB_DIRECT
if (wdata->mr) {
wdata->mr->need_invalidate = true;
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 0291482a3f51..a8ac9240a854 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4770,7 +4770,7 @@ smb2_async_writev(struct cifs_writedata *wdata,
struct cifs_io_parms *io_parms = NULL;
int credit_request;
- if (!wdata->server)
+ if (!wdata->server || wdata->replay)
server = wdata->server = cifs_pick_channel(tcon->ses);
/*
@@ -4855,6 +4855,8 @@ smb2_async_writev(struct cifs_writedata *wdata,
rqst.rq_nvec = 1;
rqst.rq_iter = wdata->iter;
rqst.rq_iter_size = iov_iter_count(&rqst.rq_iter);
+ if (wdata->replay)
+ smb2_set_replay(server, &rqst);
#ifdef CONFIG_CIFS_SMB_DIRECT
if (wdata->mr)
iov[0].iov_len += sizeof(struct smbd_buffer_descriptor_v1);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
` (5 preceding siblings ...)
2024-01-21 3:32 ` [PATCH 7/7] cifs: set replay flag for retries of write command nspmangalore
@ 2024-01-21 4:02 ` Steve French
2024-01-23 4:39 ` Shyam Prasad N
6 siblings, 1 reply; 13+ messages in thread
From: Steve French @ 2024-01-21 4:02 UTC (permalink / raw)
To: nspmangalore; +Cc: linux-cifs, pc, bharathsm, tom, Shyam Prasad N
Has a duplicate label warning that I corrected by removing the
following part of patch 1
@@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
}
skip_add_channels:
+skip_add_channels:
if (smb2_command != SMB2_INTERNAL_CMD)
mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
On Sat, Jan 20, 2024 at 9:33 PM <nspmangalore@gmail.com> wrote:
>
> From: Shyam Prasad N <sprasad@microsoft.com>
>
> Some servers like Azure SMB servers always advertise multichannel
> capability in server capabilities list. Such servers return error
> STATUS_NOT_IMPLEMENTED for ioctl calls to query server interfaces,
> and expect clients to consider that as a sign that they do not support
> multichannel.
>
> We already handled this at mount time. Soon after the tree connect,
> we query server interfaces. And when server returned STATUS_NOT_IMPLEMENTED,
> we kept interface list as empty. When cifs_try_adding_channels gets
> called, it would not find any interfaces, so will not add channels.
>
> For the case where an active multichannel mount exists, and multichannel
> is disabled by such a server, this change will now allow the client
> to disable secondary channels on the mount. It will check the return
> status of query server interfaces call soon after a tree reconnect.
> If the return status is EOPNOTSUPP, then instead of the check to add
> more channels, we'll disable the secondary channels instead.
>
> For better code reuse, this change also moves the common code for
> disabling multichannel to a helper function.
>
> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> ---
> fs/smb/client/smb2pdu.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index 288199f0b987..5126f5f97969 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -167,7 +167,7 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
>
> if (SERVER_IS_CHAN(server)) {
> cifs_dbg(VFS,
> - "server %s does not support multichannel anymore. Skip secondary channel\n",
> + "server %s does not support multichannel anymore. Skip secondary channel\n",
> ses->server->hostname);
>
> spin_lock(&ses->chan_lock);
> @@ -195,15 +195,13 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
> pserver = server->primary_server;
> cifs_signal_cifsd_for_reconnect(pserver, false);
> skip_terminate:
> - mutex_unlock(&ses->session_mutex);
> return -EHOSTDOWN;
> }
>
> cifs_server_dbg(VFS,
> - "server does not support multichannel anymore. Disable all other channels\n");
> + "server does not support multichannel anymore. Disable all other channels\n");
> cifs_disable_secondary_channels(ses);
>
> -
> return 0;
> }
>
> @@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> }
> skip_add_channels:
>
> +skip_add_channels:
> if (smb2_command != SMB2_INTERNAL_CMD)
> mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
>
> --
> 2.34.1
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/7] cifs: set replay flag for retries of write command
2024-01-21 3:32 ` [PATCH 7/7] cifs: set replay flag for retries of write command nspmangalore
@ 2024-01-21 4:02 ` Steve French
0 siblings, 0 replies; 13+ messages in thread
From: Steve French @ 2024-01-21 4:02 UTC (permalink / raw)
To: nspmangalore; +Cc: linux-cifs, pc, bharathsm, tom, Shyam Prasad N
tentatively merged to cifs-2.6.git for-next pending review and testing
(after correcting the duplicate label problem with patch 1)
On Sat, Jan 20, 2024 at 9:33 PM <nspmangalore@gmail.com> wrote:
>
> From: Shyam Prasad N <sprasad@microsoft.com>
>
> Similar to the rest of the commands, this is a change
> to add replay flags on retry. This one does not add a
> back-off, considering that we may want to flush a write
> ASAP to the server. Considering that this will be a
> flush of cached pages, the retrans value is also not
> honoured.
>
> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> ---
> fs/smb/client/cifsglob.h | 1 +
> fs/smb/client/file.c | 1 +
> fs/smb/client/smb2pdu.c | 4 +++-
> 3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index b5abe4d6f478..acda357e1dfd 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1506,6 +1506,7 @@ struct cifs_writedata {
> struct smbd_mr *mr;
> #endif
> struct cifs_credits credits;
> + bool replay;
> };
>
> /*
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index 1b4262aff8fa..49d262d1df5f 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -3300,6 +3300,7 @@ cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
> if (wdata->cfile->invalidHandle)
> rc = -EAGAIN;
> else {
> + wdata->replay = true;
> #ifdef CONFIG_CIFS_SMB_DIRECT
> if (wdata->mr) {
> wdata->mr->need_invalidate = true;
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index 0291482a3f51..a8ac9240a854 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -4770,7 +4770,7 @@ smb2_async_writev(struct cifs_writedata *wdata,
> struct cifs_io_parms *io_parms = NULL;
> int credit_request;
>
> - if (!wdata->server)
> + if (!wdata->server || wdata->replay)
> server = wdata->server = cifs_pick_channel(tcon->ses);
>
> /*
> @@ -4855,6 +4855,8 @@ smb2_async_writev(struct cifs_writedata *wdata,
> rqst.rq_nvec = 1;
> rqst.rq_iter = wdata->iter;
> rqst.rq_iter_size = iov_iter_count(&rqst.rq_iter);
> + if (wdata->replay)
> + smb2_set_replay(server, &rqst);
> #ifdef CONFIG_CIFS_SMB_DIRECT
> if (wdata->mr)
> iov[0].iov_len += sizeof(struct smbd_buffer_descriptor_v1);
> --
> 2.34.1
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling
2024-01-21 4:02 ` [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling Steve French
@ 2024-01-23 4:39 ` Shyam Prasad N
0 siblings, 0 replies; 13+ messages in thread
From: Shyam Prasad N @ 2024-01-23 4:39 UTC (permalink / raw)
To: Steve French; +Cc: linux-cifs, pc, bharathsm, tom, Shyam Prasad N
On Sun, Jan 21, 2024 at 9:32 AM Steve French <smfrench@gmail.com> wrote:
>
> Has a duplicate label warning that I corrected by removing the
> following part of patch 1
>
> @@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> }
> skip_add_channels:
>
> +skip_add_channels:
> if (smb2_command != SMB2_INTERNAL_CMD)
> mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
>
> On Sat, Jan 20, 2024 at 9:33 PM <nspmangalore@gmail.com> wrote:
> >
> > From: Shyam Prasad N <sprasad@microsoft.com>
> >
> > Some servers like Azure SMB servers always advertise multichannel
> > capability in server capabilities list. Such servers return error
> > STATUS_NOT_IMPLEMENTED for ioctl calls to query server interfaces,
> > and expect clients to consider that as a sign that they do not support
> > multichannel.
> >
> > We already handled this at mount time. Soon after the tree connect,
> > we query server interfaces. And when server returned STATUS_NOT_IMPLEMENTED,
> > we kept interface list as empty. When cifs_try_adding_channels gets
> > called, it would not find any interfaces, so will not add channels.
> >
> > For the case where an active multichannel mount exists, and multichannel
> > is disabled by such a server, this change will now allow the client
> > to disable secondary channels on the mount. It will check the return
> > status of query server interfaces call soon after a tree reconnect.
> > If the return status is EOPNOTSUPP, then instead of the check to add
> > more channels, we'll disable the secondary channels instead.
> >
> > For better code reuse, this change also moves the common code for
> > disabling multichannel to a helper function.
> >
> > Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> > ---
> > fs/smb/client/smb2pdu.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > index 288199f0b987..5126f5f97969 100644
> > --- a/fs/smb/client/smb2pdu.c
> > +++ b/fs/smb/client/smb2pdu.c
> > @@ -167,7 +167,7 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
> >
> > if (SERVER_IS_CHAN(server)) {
> > cifs_dbg(VFS,
> > - "server %s does not support multichannel anymore. Skip secondary channel\n",
> > + "server %s does not support multichannel anymore. Skip secondary channel\n",
> > ses->server->hostname);
> >
> > spin_lock(&ses->chan_lock);
> > @@ -195,15 +195,13 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
> > pserver = server->primary_server;
> > cifs_signal_cifsd_for_reconnect(pserver, false);
> > skip_terminate:
> > - mutex_unlock(&ses->session_mutex);
> > return -EHOSTDOWN;
> > }
> >
> > cifs_server_dbg(VFS,
> > - "server does not support multichannel anymore. Disable all other channels\n");
> > + "server does not support multichannel anymore. Disable all other channels\n");
> > cifs_disable_secondary_channels(ses);
> >
> > -
> > return 0;
> > }
> >
> > @@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> > }
> > skip_add_channels:
> >
> > +skip_add_channels:
> > if (smb2_command != SMB2_INTERNAL_CMD)
> > mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> >
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
>
> Steve
Please ignore this one. I'll send an updated patch.
--
Regards,
Shyam
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/7] cifs: smb2_close_getattr should also update i_size
2024-01-21 3:32 ` [PATCH 3/7] cifs: smb2_close_getattr should also update i_size nspmangalore
@ 2024-01-23 7:22 ` Shyam Prasad N
2024-01-23 7:47 ` Shyam Prasad N
0 siblings, 1 reply; 13+ messages in thread
From: Shyam Prasad N @ 2024-01-23 7:22 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
On Sun, Jan 21, 2024 at 9:03 AM <nspmangalore@gmail.com> wrote:
>
> From: Shyam Prasad N <sprasad@microsoft.com>
>
> SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB
> flag set is already used by the code for SMB3+.
> smb2_close_getattr is the function that uses this to
> update the inode attributes.
>
> However, we were skipping the EndOfFile info that's returned
> by the server. There is a small chance that the file size
> may have been changed in the small window between the client
> sending the close request (thereby giving up lease if it had)
> to the point that the server returns the response.
>
> This change uses the field to update the inode size.
> Also, it is a valid case for a zero AllocationSize to be returned
> by the server for the file. We were discarding such values, thereby
> resulting in stale i_blocks value. Fixed that here too.
>
> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> ---
> fs/smb/client/smb2ops.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index d9553c2556a2..e23577584ed6 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -1433,9 +1433,9 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
> * but instead 512 byte (2**9) size is required for
> * calculating num blocks.
> */
> - if (le64_to_cpu(file_inf.AllocationSize) > 4096)
> - inode->i_blocks =
> - (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> + inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> +
> + inode->i_size = le64_to_cpu(file_inf.EndOfFile);
>
> /* End of file and Attributes should not have to be updated on close */
> spin_unlock(&inode->i_lock);
> --
> 2.34.1
>
Noticed a couple of things in other places in the code:
1. i_size_write() should be called instead of updating i_size directly.
2. There's a server_eof field that is generally maintained in sync
with i_size. Need to check how that needs to be done here.
I'll submit a v2 of this patch soon.
--
Regards,
Shyam
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/7] cifs: smb2_close_getattr should also update i_size
2024-01-23 7:22 ` Shyam Prasad N
@ 2024-01-23 7:47 ` Shyam Prasad N
2024-01-24 2:27 ` Steve French
0 siblings, 1 reply; 13+ messages in thread
From: Shyam Prasad N @ 2024-01-23 7:47 UTC (permalink / raw)
To: linux-cifs, smfrench, pc, bharathsm, tom; +Cc: Shyam Prasad N
[-- Attachment #1: Type: text/plain, Size: 2472 bytes --]
On Tue, Jan 23, 2024 at 12:52 PM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> On Sun, Jan 21, 2024 at 9:03 AM <nspmangalore@gmail.com> wrote:
> >
> > From: Shyam Prasad N <sprasad@microsoft.com>
> >
> > SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB
> > flag set is already used by the code for SMB3+.
> > smb2_close_getattr is the function that uses this to
> > update the inode attributes.
> >
> > However, we were skipping the EndOfFile info that's returned
> > by the server. There is a small chance that the file size
> > may have been changed in the small window between the client
> > sending the close request (thereby giving up lease if it had)
> > to the point that the server returns the response.
> >
> > This change uses the field to update the inode size.
> > Also, it is a valid case for a zero AllocationSize to be returned
> > by the server for the file. We were discarding such values, thereby
> > resulting in stale i_blocks value. Fixed that here too.
> >
> > Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> > ---
> > fs/smb/client/smb2ops.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > index d9553c2556a2..e23577584ed6 100644
> > --- a/fs/smb/client/smb2ops.c
> > +++ b/fs/smb/client/smb2ops.c
> > @@ -1433,9 +1433,9 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
> > * but instead 512 byte (2**9) size is required for
> > * calculating num blocks.
> > */
> > - if (le64_to_cpu(file_inf.AllocationSize) > 4096)
> > - inode->i_blocks =
> > - (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> > + inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> > +
> > + inode->i_size = le64_to_cpu(file_inf.EndOfFile);
> >
> > /* End of file and Attributes should not have to be updated on close */
> > spin_unlock(&inode->i_lock);
> > --
> > 2.34.1
> >
> Noticed a couple of things in other places in the code:
> 1. i_size_write() should be called instead of updating i_size directly.
> 2. There's a server_eof field that is generally maintained in sync
> with i_size. Need to check how that needs to be done here.
>
> I'll submit a v2 of this patch soon.
>
> --
> Regards,
> Shyam
Attached updated patch.
--
Regards,
Shyam
[-- Attachment #2: 0001-cifs-smb2_close_getattr-should-also-update-i_size.patch --]
[-- Type: application/octet-stream, Size: 1956 bytes --]
From e470df39e5eca4f29413b60421c207d625fe5bf7 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Sun, 21 Jan 2024 03:32:44 +0000
Subject: [PATCH] cifs: smb2_close_getattr should also update i_size
SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB
flag set is already used by the code for SMB3+.
smb2_close_getattr is the function that uses this to
update the inode attributes.
However, we were skipping the EndOfFile info that's returned
by the server. There is a small chance that the file size
may have been changed in the small window between the client
sending the close request (thereby giving up lease if it had)
to the point that the server returns the response.
This change uses the field to update the inode size.
Also, it is a valid case for a zero AllocationSize to be returned
by the server for the file. We were discarding such values, thereby
resulting in stale i_blocks value. Fixed that here too.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
---
fs/smb/client/smb2ops.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index d9553c2556a2..ccb348e966f6 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1433,9 +1433,10 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
* but instead 512 byte (2**9) size is required for
* calculating num blocks.
*/
- if (le64_to_cpu(file_inf.AllocationSize) > 4096)
- inode->i_blocks =
- (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
+ inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
+
+ CIFS_I(inode)->server_eof = le64_to_cpu(file_inf.EndOfFile);
+ i_size_write(inode, CIFS_I(inode)->server_eof);
/* End of file and Attributes should not have to be updated on close */
spin_unlock(&inode->i_lock);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/7] cifs: smb2_close_getattr should also update i_size
2024-01-23 7:47 ` Shyam Prasad N
@ 2024-01-24 2:27 ` Steve French
0 siblings, 0 replies; 13+ messages in thread
From: Steve French @ 2024-01-24 2:27 UTC (permalink / raw)
To: Shyam Prasad N; +Cc: linux-cifs, pc, bharathsm, tom, Shyam Prasad N
[-- Attachment #1: Type: text/plain, Size: 2891 bytes --]
This was the patch that was causing test generic/074 to regress so I
took it out of for-next temporarily till we can fix it. The version
of the patch I was using is attached
On Tue, Jan 23, 2024 at 1:48 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> On Tue, Jan 23, 2024 at 12:52 PM Shyam Prasad N <nspmangalore@gmail.com> wrote:
> >
> > On Sun, Jan 21, 2024 at 9:03 AM <nspmangalore@gmail.com> wrote:
> > >
> > > From: Shyam Prasad N <sprasad@microsoft.com>
> > >
> > > SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB
> > > flag set is already used by the code for SMB3+.
> > > smb2_close_getattr is the function that uses this to
> > > update the inode attributes.
> > >
> > > However, we were skipping the EndOfFile info that's returned
> > > by the server. There is a small chance that the file size
> > > may have been changed in the small window between the client
> > > sending the close request (thereby giving up lease if it had)
> > > to the point that the server returns the response.
> > >
> > > This change uses the field to update the inode size.
> > > Also, it is a valid case for a zero AllocationSize to be returned
> > > by the server for the file. We were discarding such values, thereby
> > > resulting in stale i_blocks value. Fixed that here too.
> > >
> > > Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> > > ---
> > > fs/smb/client/smb2ops.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > > index d9553c2556a2..e23577584ed6 100644
> > > --- a/fs/smb/client/smb2ops.c
> > > +++ b/fs/smb/client/smb2ops.c
> > > @@ -1433,9 +1433,9 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
> > > * but instead 512 byte (2**9) size is required for
> > > * calculating num blocks.
> > > */
> > > - if (le64_to_cpu(file_inf.AllocationSize) > 4096)
> > > - inode->i_blocks =
> > > - (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> > > + inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
> > > +
> > > + inode->i_size = le64_to_cpu(file_inf.EndOfFile);
> > >
> > > /* End of file and Attributes should not have to be updated on close */
> > > spin_unlock(&inode->i_lock);
> > > --
> > > 2.34.1
> > >
> > Noticed a couple of things in other places in the code:
> > 1. i_size_write() should be called instead of updating i_size directly.
> > 2. There's a server_eof field that is generally maintained in sync
> > with i_size. Need to check how that needs to be done here.
> >
> > I'll submit a v2 of this patch soon.
> >
> > --
> > Regards,
> > Shyam
>
> Attached updated patch.
>
> --
> Regards,
> Shyam
--
Thanks,
Steve
[-- Attachment #2: 0001-cifs-smb2_close_getattr-should-also-update-i_size.patch --]
[-- Type: text/x-patch, Size: 1978 bytes --]
From 3665f29562a6fefefd36a601d478854a6b672255 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Sun, 21 Jan 2024 03:32:44 +0000
Subject: [PATCH 1/6] cifs: smb2_close_getattr should also update i_size
SMB2 CLOSE command with SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB
flag set is already used by the code for SMB3+.
smb2_close_getattr is the function that uses this to
update the inode attributes.
However, we were skipping the EndOfFile info that's returned
by the server. There is a small chance that the file size
may have been changed in the small window between the client
sending the close request (thereby giving up lease if it had)
to the point that the server returns the response.
This change uses the field to update the inode size.
Also, it is a valid case for a zero AllocationSize to be returned
by the server for the file. We were discarding such values, thereby
resulting in stale i_blocks value. Fixed that here too.
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
---
fs/smb/client/smb2ops.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 27f8caccff7f..eebd2b720945 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1433,9 +1433,10 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
* but instead 512 byte (2**9) size is required for
* calculating num blocks.
*/
- if (le64_to_cpu(file_inf.AllocationSize) > 4096)
- inode->i_blocks =
- (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
+ inode->i_blocks = (512 - 1 + le64_to_cpu(file_inf.AllocationSize)) >> 9;
+
+ CIFS_I(inode)->netfs.remote_i_size = le64_to_cpu(file_inf.EndOfFile);
+ i_size_write(inode, CIFS_I(inode)->netfs.remote_i_size);
/* End of file and Attributes should not have to be updated on close */
spin_unlock(&inode->i_lock);
--
2.40.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-01-24 2:27 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-21 3:32 [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling nspmangalore
2024-01-21 3:32 ` [PATCH 2/7] cifs: cifs_pick_channel should try selecting active channels nspmangalore
2024-01-21 3:32 ` [PATCH 3/7] cifs: smb2_close_getattr should also update i_size nspmangalore
2024-01-23 7:22 ` Shyam Prasad N
2024-01-23 7:47 ` Shyam Prasad N
2024-01-24 2:27 ` Steve French
2024-01-21 3:32 ` [PATCH 4/7] cifs: translate network errors on send to -ECONNABORTED nspmangalore
2024-01-21 3:32 ` [PATCH 5/7] cifs: helper function to check replayable error codes nspmangalore
2024-01-21 3:32 ` [PATCH 6/7] cifs: commands that are retried should have replay flag set nspmangalore
2024-01-21 3:32 ` [PATCH 7/7] cifs: set replay flag for retries of write command nspmangalore
2024-01-21 4:02 ` Steve French
2024-01-21 4:02 ` [PATCH 1/7] cifs: handle servers that still advertise multichannel after disabling Steve French
2024-01-23 4:39 ` Shyam Prasad N
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox