Commit 6a468d59 authored by Josef Bacik's avatar Josef Bacik Committed by Jens Axboe

nbd: don't start req until after the dead connection logic

We can end up sleeping for a while waiting for the dead timeout, which
means we could get the per request timer to fire.  We did handle this
case, but if the dead timeout happened right after we submitted we'd
either tear down the connection or possibly requeue as we're handling an
error and race with the endio which can lead to panics and other
hilarity.

Fixes: 560bc4b3 ("nbd: handle dead connections")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarJosef Bacik <jbacik@fb.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent ff57dc94
...@@ -289,15 +289,6 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req, ...@@ -289,15 +289,6 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
cmd->status = BLK_STS_TIMEOUT; cmd->status = BLK_STS_TIMEOUT;
return BLK_EH_HANDLED; return BLK_EH_HANDLED;
} }
/* If we are waiting on our dead timer then we could get timeout
* callbacks for our request. For this we just want to reset the timer
* and let the queue side take care of everything.
*/
if (!completion_done(&cmd->send_complete)) {
nbd_config_put(nbd);
return BLK_EH_RESET_TIMER;
}
config = nbd->config; config = nbd->config;
if (config->num_connections > 1) { if (config->num_connections > 1) {
...@@ -732,6 +723,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index) ...@@ -732,6 +723,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
if (!refcount_inc_not_zero(&nbd->config_refs)) { if (!refcount_inc_not_zero(&nbd->config_refs)) {
dev_err_ratelimited(disk_to_dev(nbd->disk), dev_err_ratelimited(disk_to_dev(nbd->disk),
"Socks array is empty\n"); "Socks array is empty\n");
blk_mq_start_request(req);
return -EINVAL; return -EINVAL;
} }
config = nbd->config; config = nbd->config;
...@@ -740,6 +732,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index) ...@@ -740,6 +732,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
dev_err_ratelimited(disk_to_dev(nbd->disk), dev_err_ratelimited(disk_to_dev(nbd->disk),
"Attempted send on invalid socket\n"); "Attempted send on invalid socket\n");
nbd_config_put(nbd); nbd_config_put(nbd);
blk_mq_start_request(req);
return -EINVAL; return -EINVAL;
} }
cmd->status = BLK_STS_OK; cmd->status = BLK_STS_OK;
...@@ -763,6 +756,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index) ...@@ -763,6 +756,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
*/ */
sock_shutdown(nbd); sock_shutdown(nbd);
nbd_config_put(nbd); nbd_config_put(nbd);
blk_mq_start_request(req);
return -EIO; return -EIO;
} }
goto again; goto again;
...@@ -773,6 +767,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index) ...@@ -773,6 +767,7 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
* here so that it gets put _after_ the request that is already on the * here so that it gets put _after_ the request that is already on the
* dispatch list. * dispatch list.
*/ */
blk_mq_start_request(req);
if (unlikely(nsock->pending && nsock->pending != req)) { if (unlikely(nsock->pending && nsock->pending != req)) {
blk_mq_requeue_request(req, true); blk_mq_requeue_request(req, true);
ret = 0; ret = 0;
...@@ -785,10 +780,10 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index) ...@@ -785,10 +780,10 @@ static int nbd_handle_cmd(struct nbd_cmd *cmd, int index)
ret = nbd_send_cmd(nbd, cmd, index); ret = nbd_send_cmd(nbd, cmd, index);
if (ret == -EAGAIN) { if (ret == -EAGAIN) {
dev_err_ratelimited(disk_to_dev(nbd->disk), dev_err_ratelimited(disk_to_dev(nbd->disk),
"Request send failed trying another connection\n"); "Request send failed, requeueing\n");
nbd_mark_nsock_dead(nbd, nsock, 1); nbd_mark_nsock_dead(nbd, nsock, 1);
mutex_unlock(&nsock->tx_lock); blk_mq_requeue_request(req, true);
goto again; ret = 0;
} }
out: out:
mutex_unlock(&nsock->tx_lock); mutex_unlock(&nsock->tx_lock);
...@@ -812,7 +807,6 @@ static blk_status_t nbd_queue_rq(struct blk_mq_hw_ctx *hctx, ...@@ -812,7 +807,6 @@ static blk_status_t nbd_queue_rq(struct blk_mq_hw_ctx *hctx,
* done sending everything over the wire. * done sending everything over the wire.
*/ */
init_completion(&cmd->send_complete); init_completion(&cmd->send_complete);
blk_mq_start_request(bd->rq);
/* We can be called directly from the user space process, which means we /* We can be called directly from the user space process, which means we
* could possibly have signals pending so our sendmsg will fail. In * could possibly have signals pending so our sendmsg will fail. In
......
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