* [PATCH AUTOSEL 5.9 127/147] Handle STATUS_IO_TIMEOUT gracefully
[not found] <20201026234905.1022767-1-sashal@kernel.org>
@ 2020-10-26 23:48 ` Sasha Levin
2020-10-26 23:48 ` [PATCH AUTOSEL 5.9 128/147] cifs: handle -EINTR in cifs_setattr Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2020-10-26 23:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Rohith Surabattula, Aurelien Aptel, Pavel Shilovsky, Steve French,
Sasha Levin, linux-cifs, samba-technical
From: Rohith Surabattula <rohiths@microsoft.com>
[ Upstream commit 8e670f77c4a55013db6d23b962f9bf6673a5e7b6 ]
Currently STATUS_IO_TIMEOUT is not treated as retriable error.
It is currently mapped to ETIMEDOUT and returned to userspace
for most system calls. STATUS_IO_TIMEOUT is returned by server
in case of unavailability or throttling errors.
This patch will map the STATUS_IO_TIMEOUT to EAGAIN, so that it
can be retried. Also, added a check to drop the connection to
not overload the server in case of ongoing unavailability.
Signed-off-by: Rohith Surabattula <rohiths@microsoft.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/cifs/cifsglob.h | 2 ++
fs/cifs/connect.c | 15 ++++++++++++++-
fs/cifs/smb2maperror.c | 2 +-
fs/cifs/smb2ops.c | 15 +++++++++++++++
4 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index b565d83ba89ed..5a491afafacc7 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -510,6 +510,8 @@ struct smb_version_operations {
struct fiemap_extent_info *, u64, u64);
/* version specific llseek implementation */
loff_t (*llseek)(struct file *, struct cifs_tcon *, loff_t, int);
+ /* Check for STATUS_IO_TIMEOUT */
+ bool (*is_status_io_timeout)(char *buf);
};
struct smb_version_values {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index a5731dd6e6566..1a3b7793095e4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -69,6 +69,9 @@ extern bool disable_legacy_dialects;
#define TLINK_ERROR_EXPIRE (1 * HZ)
#define TLINK_IDLE_EXPIRE (600 * HZ)
+/* Drop the connection to not overload the server */
+#define NUM_STATUS_IO_TIMEOUT 5
+
enum {
/* Mount options that take no arguments */
Opt_user_xattr, Opt_nouser_xattr,
@@ -1117,7 +1120,7 @@ cifs_demultiplex_thread(void *p)
struct task_struct *task_to_wake = NULL;
struct mid_q_entry *mids[MAX_COMPOUND];
char *bufs[MAX_COMPOUND];
- unsigned int noreclaim_flag;
+ unsigned int noreclaim_flag, num_io_timeout = 0;
noreclaim_flag = memalloc_noreclaim_save();
cifs_dbg(FYI, "Demultiplex PID: %d\n", task_pid_nr(current));
@@ -1213,6 +1216,16 @@ cifs_demultiplex_thread(void *p)
continue;
}
+ if (server->ops->is_status_io_timeout &&
+ server->ops->is_status_io_timeout(buf)) {
+ num_io_timeout++;
+ if (num_io_timeout > NUM_STATUS_IO_TIMEOUT) {
+ cifs_reconnect(server);
+ num_io_timeout = 0;
+ continue;
+ }
+ }
+
server->lstrp = jiffies;
for (i = 0; i < num_mids; i++) {
diff --git a/fs/cifs/smb2maperror.c b/fs/cifs/smb2maperror.c
index 7fde3775cb574..b004cf87692a7 100644
--- a/fs/cifs/smb2maperror.c
+++ b/fs/cifs/smb2maperror.c
@@ -488,7 +488,7 @@ static const struct status_to_posix_error smb2_error_map_table[] = {
{STATUS_PIPE_CONNECTED, -EIO, "STATUS_PIPE_CONNECTED"},
{STATUS_PIPE_LISTENING, -EIO, "STATUS_PIPE_LISTENING"},
{STATUS_INVALID_READ_MODE, -EIO, "STATUS_INVALID_READ_MODE"},
- {STATUS_IO_TIMEOUT, -ETIMEDOUT, "STATUS_IO_TIMEOUT"},
+ {STATUS_IO_TIMEOUT, -EAGAIN, "STATUS_IO_TIMEOUT"},
{STATUS_FILE_FORCED_CLOSED, -EIO, "STATUS_FILE_FORCED_CLOSED"},
{STATUS_PROFILING_NOT_STARTED, -EIO, "STATUS_PROFILING_NOT_STARTED"},
{STATUS_PROFILING_NOT_STOPPED, -EIO, "STATUS_PROFILING_NOT_STOPPED"},
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index d44df8f95bcd4..8c3b997e718a0 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2346,6 +2346,17 @@ smb2_is_session_expired(char *buf)
return true;
}
+static bool
+smb2_is_status_io_timeout(char *buf)
+{
+ struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf;
+
+ if (shdr->Status == STATUS_IO_TIMEOUT)
+ return true;
+ else
+ return false;
+}
+
static int
smb2_oplock_response(struct cifs_tcon *tcon, struct cifs_fid *fid,
struct cifsInodeInfo *cinode)
@@ -4809,6 +4820,7 @@ struct smb_version_operations smb20_operations = {
.make_node = smb2_make_node,
.fiemap = smb3_fiemap,
.llseek = smb3_llseek,
+ .is_status_io_timeout = smb2_is_status_io_timeout,
};
struct smb_version_operations smb21_operations = {
@@ -4909,6 +4921,7 @@ struct smb_version_operations smb21_operations = {
.make_node = smb2_make_node,
.fiemap = smb3_fiemap,
.llseek = smb3_llseek,
+ .is_status_io_timeout = smb2_is_status_io_timeout,
};
struct smb_version_operations smb30_operations = {
@@ -5019,6 +5032,7 @@ struct smb_version_operations smb30_operations = {
.make_node = smb2_make_node,
.fiemap = smb3_fiemap,
.llseek = smb3_llseek,
+ .is_status_io_timeout = smb2_is_status_io_timeout,
};
struct smb_version_operations smb311_operations = {
@@ -5130,6 +5144,7 @@ struct smb_version_operations smb311_operations = {
.make_node = smb2_make_node,
.fiemap = smb3_fiemap,
.llseek = smb3_llseek,
+ .is_status_io_timeout = smb2_is_status_io_timeout,
};
struct smb_version_values smb20_values = {
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* [PATCH AUTOSEL 5.9 128/147] cifs: handle -EINTR in cifs_setattr
[not found] <20201026234905.1022767-1-sashal@kernel.org>
2020-10-26 23:48 ` [PATCH AUTOSEL 5.9 127/147] Handle STATUS_IO_TIMEOUT gracefully Sasha Levin
@ 2020-10-26 23:48 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2020-10-26 23:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Ronnie Sahlberg, Steve French, Sasha Levin, linux-cifs,
samba-technical
From: Ronnie Sahlberg <lsahlber@redhat.com>
[ Upstream commit c6cc4c5a72505a0ecefc9b413f16bec512f38078 ]
RHBZ: 1848178
Some calls that set attributes, like utimensat(), are not supposed to return
-EINTR and thus do not have handlers for this in glibc which causes us
to leak -EINTR to the applications which are also unprepared to handle it.
For example tar will break if utimensat() return -EINTR and abort unpacking
the archive. Other applications may break too.
To handle this we add checks, and retry, for -EINTR in cifs_setattr()
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/cifs/inode.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 1f75b25e559a7..daec31be85718 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -2883,13 +2883,18 @@ cifs_setattr(struct dentry *direntry, struct iattr *attrs)
{
struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb);
struct cifs_tcon *pTcon = cifs_sb_master_tcon(cifs_sb);
+ int rc, retries = 0;
- if (pTcon->unix_ext)
- return cifs_setattr_unix(direntry, attrs);
-
- return cifs_setattr_nounix(direntry, attrs);
+ do {
+ if (pTcon->unix_ext)
+ rc = cifs_setattr_unix(direntry, attrs);
+ else
+ rc = cifs_setattr_nounix(direntry, attrs);
+ retries++;
+ } while (is_retryable_error(rc) && retries < 2);
/* BB: add cifs_setattr_legacy for really old servers */
+ return rc;
}
#if 0
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread