[2/2] cifs: simplify how we handle credits in compond_send_recv()
diff mbox series

Message ID 20190220061145.28241-3-lsahlber@redhat.com
State New
Headers show
Series
  • Simplify handling of credits in compound_send_recv()
Related show

Commit Message

Ronnie Sahlberg Feb. 20, 2019, 6:11 a.m. UTC
Since we can now wait for multiple requests atimically in
wait_for_free_request() we can now greatly simplify the handling
of the credits in this function.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/transport.c | 74 ++++++++++-------------------------------------------
 1 file changed, 13 insertions(+), 61 deletions(-)

Patch
diff mbox series

diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 8ce5b6e787c6..b8f2643048e3 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -871,7 +871,6 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 		{ .value = 0, .instance = 0 }
 	};
 	unsigned int instance;
-	unsigned int first_instance = 0;
 	char *buf;
 
 	timeout = flags & CIFS_TIMEOUT_MASK;
@@ -898,71 +897,28 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 			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.
+	 * Wait for all the requests to become available.
 	 * 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.
+	 * requests and if the client is completely idle, not generating any
+	 * other requests.
+	 * This can be handled by the eventual session reconnect.
 	 */
-	for (i = 0; i < num_rqst; i++) {
-		rc = wait_for_free_request(ses->server, timeout, 1, optype,
-					   &instance);
+	rc = wait_for_free_request(ses->server, timeout, num_rqst, optype,
+				   &instance);
 
-		if (rc == 0) {
-			credits[i].value = 1;
-			credits[i].instance = instance;
-			/*
-			 * All parts of the compound chain must get credits from
-			 * the same session, otherwise we may end up using more
-			 * credits than the server granted. If there were
-			 * reconnects in between, return -EAGAIN and let callers
-			 * handle it.
-			 */
-			if (i == 0)
-				first_instance = instance;
-			else if (first_instance != instance) {
-				i++;
-				rc = -EAGAIN;
-			}
-		}
+	if (rc)
+		return rc;
 
-		if (rc) {
-			/*
-			 * We haven't sent an SMB packet to the server yet but
-			 * we already obtained credits for i requests in the
-			 * compound chain - need to return those credits back
-			 * for future use. Note that we need to call add_credits
-			 * multiple times to match the way we obtained credits
-			 * in the first place and to account for in flight
-			 * requests correctly.
-			 */
-			for (j = 0; j < i; j++)
-				add_credits(ses->server, &credits[j], optype);
-			return rc;
-		}
+	for (i = 0; i < num_rqst; i++) {
+		credits[i].value = 1;
+		credits[i].instance = instance;
 	}
 
-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
@@ -973,17 +929,13 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 
 	/*
 	 * All the parts of the compound chain belong obtained credits from the
-	 * same session (see the appropriate checks above). In the same time
-	 * there might be reconnects after those checks but before we acquired
-	 * the srv_mutex. We can not use credits obtained from the previous
+	 * same session. We can not use credits obtained from the previous
 	 * session to send this request. Check if there were reconnects after
 	 * we obtained credits and return -EAGAIN in such cases to let callers
 	 * handle it.
 	 */
-	if (first_instance != ses->server->reconnect_instance) {
+	if (instance != ses->server->reconnect_instance) {
 		mutex_unlock(&ses->server->srv_mutex);
-		for (j = 0; j < num_rqst; j++)
-			add_credits(ses->server, &credits[j], optype);
 		return -EAGAIN;
 	}