Commit 257b7809 authored by Ronnie Sahlberg's avatar Ronnie Sahlberg Committed by Steve French

cifs: simplify how we handle credits in compound_send_recv()

Since we can now wait for multiple requests atomically in
wait_for_free_request() we can now greatly simplify the handling
of the credits in this function.

This fixes a potential deadlock where many concurrent compound requests
could each have reserved 1 or 2 credits each but are all blocked
waiting for the final credits they need to be able to issue the requests
to the server.

Set a default timeout of 60 seconds for compounded requests.
Signed-off-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
Reviewed-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
parent 7937ca96
...@@ -606,6 +606,31 @@ wait_for_free_request(struct TCP_Server_Info *server, const int flags, ...@@ -606,6 +606,31 @@ wait_for_free_request(struct TCP_Server_Info *server, const int flags,
instance); instance);
} }
static int
wait_for_compound_request(struct TCP_Server_Info *server, int num,
const int flags, unsigned int *instance)
{
int *credits;
credits = server->ops->get_credits_field(server, flags & CIFS_OP_MASK);
spin_lock(&server->req_lock);
if (*credits < num) {
/*
* Return immediately if not too many requests in flight since
* we will likely be stuck on waiting for credits.
*/
if (server->in_flight < num - *credits) {
spin_unlock(&server->req_lock);
return -ENOTSUPP;
}
}
spin_unlock(&server->req_lock);
return wait_for_free_credits(server, num, 60000, flags,
instance);
}
int int
cifs_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size, cifs_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size,
unsigned int *num, struct cifs_credits *credits) unsigned int *num, struct cifs_credits *credits)
...@@ -934,7 +959,6 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, ...@@ -934,7 +959,6 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
{ .value = 0, .instance = 0 } { .value = 0, .instance = 0 }
}; };
unsigned int instance; unsigned int instance;
unsigned int first_instance = 0;
char *buf; char *buf;
optype = flags & CIFS_OP_MASK; optype = flags & CIFS_OP_MASK;
...@@ -950,80 +974,24 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, ...@@ -950,80 +974,24 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
if (ses->server->tcpStatus == CifsExiting) if (ses->server->tcpStatus == CifsExiting)
return -ENOENT; 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 * Wait for all the requests to become available.
* there are requests in flight that may bring credits from the server.
* This approach still leaves the possibility to be stuck waiting for * This approach still leaves the possibility to be stuck waiting for
* credits if the server doesn't grant credits to the outstanding * credits if the server doesn't grant credits to the outstanding
* requests. This should be fixed by returning immediately and letting * requests and if the client is completely idle, not generating any
* a caller fallback to sequential commands instead of compounding. * other requests.
* Ensure we obtain 1 credit per request in the compound chain. * This can be handled by the eventual session reconnect.
*/ */
for (i = 0; i < num_rqst; i++) { rc = wait_for_compound_request(ses->server, num_rqst, flags,
rc = wait_for_free_request(ses->server, flags, &instance); &instance);
if (rc)
return rc;
if (rc == 0) { for (i = 0; i < num_rqst; i++) {
credits[i].value = 1; credits[i].value = 1;
credits[i].instance = instance; 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) {
/*
* 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;
}
} }
setup_rqsts:
/* /*
* Make sure that we sign in the same order that we send on this socket * 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 * and avoid races inside tcp sendmsg code that could cause corruption
...@@ -1034,14 +1002,12 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, ...@@ -1034,14 +1002,12 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
/* /*
* All the parts of the compound chain belong obtained credits from the * All the parts of the compound chain belong obtained credits from the
* same session (see the appropriate checks above). In the same time * same session. We can not use credits obtained from the previous
* there might be reconnects after those checks but before we acquired
* the srv_mutex. We can not use credits obtained from the previous
* session to send this request. Check if there were reconnects after * session to send this request. Check if there were reconnects after
* we obtained credits and return -EAGAIN in such cases to let callers * we obtained credits and return -EAGAIN in such cases to let callers
* handle it. * handle it.
*/ */
if (first_instance != ses->server->reconnect_instance) { if (instance != ses->server->reconnect_instance) {
mutex_unlock(&ses->server->srv_mutex); mutex_unlock(&ses->server->srv_mutex);
for (j = 0; j < num_rqst; j++) for (j = 0; j < num_rqst; j++)
add_credits(ses->server, &credits[j], optype); add_credits(ses->server, &credits[j], optype);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment