* [PATCH 1/3] cifs: fix credit leaks in async callback
@ 2023-06-29 8:58 Winston Wen
2023-06-29 8:58 ` [PATCH 2/3] cifs: stop waiting for credits if there are no more requests in flight Winston Wen
2023-06-29 8:58 ` [PATCH 3/3] cifs: add signal check in the loop in smb2_get_dfs_refer() Winston Wen
0 siblings, 2 replies; 3+ messages in thread
From: Winston Wen @ 2023-06-29 8:58 UTC (permalink / raw
To: smfrench, linux-cifs, pc, nspmangalore; +Cc: Winston Wen
Initialize credits.value to 1, which will be passed to add_credits() if
mid->mid_state is not MID_RESPONSE_RECEIVED or MID_RESPONSE_MALFORMED.
Signed-off-by: Winston Wen <wentao@uniontech.com>
---
fs/smb/client/smb2pdu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index e04766fe6f80..4c71979fca6d 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3760,7 +3760,7 @@ smb2_echo_callback(struct mid_q_entry *mid)
{
struct TCP_Server_Info *server = mid->callback_data;
struct smb2_echo_rsp *rsp = (struct smb2_echo_rsp *)mid->resp_buf;
- struct cifs_credits credits = { .value = 0, .instance = 0 };
+ struct cifs_credits credits = { .value = 1, .instance = 0 };
if (mid->mid_state == MID_RESPONSE_RECEIVED
|| mid->mid_state == MID_RESPONSE_MALFORMED) {
@@ -4150,7 +4150,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
struct TCP_Server_Info *server = rdata->server;
struct smb2_hdr *shdr =
(struct smb2_hdr *)rdata->iov[0].iov_base;
- struct cifs_credits credits = { .value = 0, .instance = 0 };
+ struct cifs_credits credits = { .value = 1, .instance = 0 };
struct smb_rqst rqst = { .rq_iov = &rdata->iov[1], .rq_nvec = 1 };
if (rdata->got_bytes) {
@@ -4402,7 +4402,7 @@ smb2_writev_callback(struct mid_q_entry *mid)
struct TCP_Server_Info *server = wdata->server;
unsigned int written;
struct smb2_write_rsp *rsp = (struct smb2_write_rsp *)mid->resp_buf;
- struct cifs_credits credits = { .value = 0, .instance = 0 };
+ struct cifs_credits credits = { .value = 1, .instance = 0 };
WARN_ONCE(wdata->server != mid->server,
"wdata server %p != mid server %p",
--
2.40.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/3] cifs: stop waiting for credits if there are no more requests in flight
2023-06-29 8:58 [PATCH 1/3] cifs: fix credit leaks in async callback Winston Wen
@ 2023-06-29 8:58 ` Winston Wen
2023-06-29 8:58 ` [PATCH 3/3] cifs: add signal check in the loop in smb2_get_dfs_refer() Winston Wen
1 sibling, 0 replies; 3+ messages in thread
From: Winston Wen @ 2023-06-29 8:58 UTC (permalink / raw
To: smfrench, linux-cifs, pc, nspmangalore; +Cc: Winston Wen
A compound request will wait for credits if free credits are not enough
now but there are in flight requests which might bring back some credits
to meet our needs in the near future.
But if the in-flight requests don't bring back enough credits, the
compound request will continue to wait unnecessarily until it times out
(60s now).
So add a helper has_credits_or_insufficient() to check if we should stop
waiting for credits in the loop to return faster.
Signed-off-by: Winston Wen <wentao@uniontech.com>
---
fs/smb/client/cifsglob.h | 13 +++++++++++++
fs/smb/client/transport.c | 16 +++++++++++++---
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index cb38c29b9a73..43d0a675b543 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -800,6 +800,19 @@ has_credits(struct TCP_Server_Info *server, int *credits, int num_credits)
return num >= num_credits;
}
+static inline bool
+has_credits_or_insufficient(struct TCP_Server_Info *server, int *credits, int num_credits)
+{
+ int scredits;
+ int in_flight;
+
+ spin_lock(&server->req_lock);
+ scredits = *credits;
+ in_flight = server->in_flight;
+ spin_unlock(&server->req_lock);
+ return scredits >= num_credits || in_flight == 0;
+}
+
static inline void
add_credits(struct TCP_Server_Info *server, const struct cifs_credits *credits,
const int optype)
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index f280502a2aee..82071142d72b 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -534,11 +534,21 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
spin_lock(&server->req_lock);
if (*credits < num_credits) {
scredits = *credits;
+ in_flight = server->in_flight;
+ if (in_flight == 0) {
+ spin_unlock(&server->req_lock);
+ trace_smb3_insufficient_credits(server->CurrentMid,
+ server->conn_id, server->hostname, scredits,
+ num_credits, in_flight);
+ cifs_dbg(FYI, "%s: %d requests in flight, needed %d total=%d\n",
+ __func__, in_flight, num_credits, scredits);
+ return -EDEADLK;
+ }
spin_unlock(&server->req_lock);
cifs_num_waiters_inc(server);
rc = wait_event_killable_timeout(server->request_q,
- has_credits(server, credits, num_credits), t);
+ has_credits_or_insufficient(server, credits, num_credits), t);
cifs_num_waiters_dec(server);
if (!rc) {
spin_lock(&server->req_lock);
@@ -578,8 +588,8 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
cifs_num_waiters_inc(server);
rc = wait_event_killable_timeout(
server->request_q,
- has_credits(server, credits,
- MAX_COMPOUND + 1),
+ has_credits_or_insufficient(server, credits,
+ MAX_COMPOUND + 1),
t);
cifs_num_waiters_dec(server);
if (!rc) {
--
2.40.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 3/3] cifs: add signal check in the loop in smb2_get_dfs_refer()
2023-06-29 8:58 [PATCH 1/3] cifs: fix credit leaks in async callback Winston Wen
2023-06-29 8:58 ` [PATCH 2/3] cifs: stop waiting for credits if there are no more requests in flight Winston Wen
@ 2023-06-29 8:58 ` Winston Wen
1 sibling, 0 replies; 3+ messages in thread
From: Winston Wen @ 2023-06-29 8:58 UTC (permalink / raw
To: smfrench, linux-cifs, pc, nspmangalore; +Cc: Winston Wen
If a process has a pending fatal signal, the request will not be sent in
__smb_send_rqst(), but will return -ERESTARTSYS instead.
In the loop in smb2_get_dfs_refer(), -ERESTARTSYS returned from
SMB_ioctl will cause an retry that still can't succeed and will do some
unnecessary work, like allocating/releasing buffer, getting/adding
credits.
So let us add signal check in the loop to avoid unnecessary retries and
return faster.
Signed-off-by: Winston Wen <wentao@uniontech.com>
---
fs/smb/client/smb2ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index eb1340b9125e..64f78e1b5ea7 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -2818,7 +2818,7 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
FSCTL_DFS_GET_REFERRALS,
(char *)dfs_req, dfs_req_size, CIFSMaxBufSize,
(char **)&dfs_rsp, &dfs_rsp_size);
- if (!is_retryable_error(rc))
+ if (!is_retryable_error(rc) || fatal_signal_pending(current))
break;
usleep_range(512, 2048);
} while (++retry_count < 5);
--
2.40.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-29 9:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-29 8:58 [PATCH 1/3] cifs: fix credit leaks in async callback Winston Wen
2023-06-29 8:58 ` [PATCH 2/3] cifs: stop waiting for credits if there are no more requests in flight Winston Wen
2023-06-29 8:58 ` [PATCH 3/3] cifs: add signal check in the loop in smb2_get_dfs_refer() Winston Wen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).