diff mbox series

[23/23] CIFS: Try to acquire credits at once for compound requests

Message ID 1549051452-5968-25-git-send-email-pshilov@microsoft.com (mailing list archive)
State New, archived
Headers show
Series Improve credits and error handling on reconnects | expand

Commit Message

Pavel Shilovsky Feb. 1, 2019, 8:04 p.m. UTC
Currently we get one credit per compound part of the request
individually. This may lead to being stuck on waiting for credits
if multiple compounded operations happen in parallel. Try acquire
credits for all compound parts at once. Return immediately if not
enough credits and too few requests are in flight currently thus
narrowing the possibility of infinite waiting for credits.

The more advance fix is to return right away if not enough credits
for the compound request and do not look at the number of requests
in flight. The caller should handle such situations by falling back
to sequential execution of SMB commands instead of compounding.

Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/transport.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 2187bc3..d4f1224f 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -886,13 +886,41 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 	if (ses->server->tcpStatus == CifsExiting)
 		return -ENOENT;
 
+	spin_lock(&ses->server->req_lock);
+	if (ses->server->credits < num_rqst) {
+		/*
+		 * Return immediately if not too many requests in flight since
+		 * we will likely be stuck on waiting for credits.
+		 */
+		if (ses->server->in_flight < num_rqst - ses->server->credits) {
+			spin_unlock(&ses->server->req_lock);
+			return -ENOTSUPP;
+		}
+	} else {
+		/* enough credits to send the whole compounded request */
+		ses->server->credits -= num_rqst;
+		ses->server->in_flight += num_rqst;
+		first_instance = ses->server->reconnect_instance;
+	}
+	spin_unlock(&ses->server->req_lock);
+
+	if (first_instance) {
+		cifs_dbg(FYI, "Acquired %d credits at once\n", num_rqst);
+		for (i = 0; i < num_rqst; i++) {
+			credits[i].value = 1;
+			credits[i].instance = first_instance;
+		}
+		goto setup_rqsts;
+	}
+
 	/*
+	 * There are not enough credits to send the whole compound request but
+	 * there are requests in flight that may bring credits from the server.
+	 * This approach still leaves the possibility to be stuck waiting for
+	 * credits if the server doesn't grant credits to the outstanding
+	 * requests. This should be fixed by returning immediately and letting
+	 * a caller fallback to sequential commands instead of compounding.
 	 * Ensure we obtain 1 credit per request in the compound chain.
-	 * It can be optimized further by waiting for all the credits
-	 * at once but this can wait long enough if we don't have enough
-	 * credits due to some heavy operations in progress or the server
-	 * not granting us much, so a fallback to the current approach is
-	 * needed anyway.
 	 */
 	for (i = 0; i < num_rqst; i++) {
 		rc = wait_for_free_request(ses->server, timeout, optype,
@@ -932,6 +960,7 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 		}
 	}
 
+setup_rqsts:
 	/*
 	 * Make sure that we sign in the same order that we send on this socket
 	 * and avoid races inside tcp sendmsg code that could cause corruption