IB/srpt: Fix how aborted commands are processed
srpt_abort_cmd() must not be called in state SRPT_STATE_DATA_IN. Issue
a warning if this occurs.
srpt_abort_cmd() must not invoke target_put_sess_cmd() for commands
in state SRPT_STATE_DONE because the srpt_abort_cmd() callers already
do this when necessary. Hence remove this call.
If an RDMA read fails the corresponding SCSI command must fail. Hence
add a transport_generic_request_failure() call.
Remove an incorrect srpt_abort_cmd() call from srpt_rdma_write_done().
Avoid that srpt_send_done() calls srpt_abort_cmd() for finished SCSI
commands.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alex Estrin <alex.estrin@intel.com>
Cc: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 5b0fbc1..64e9107 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1267,10 +1267,7 @@
/*
* If the command is in a state where the target core is waiting for
- * the ib_srpt driver, change the state to the next state. Changing
- * the state of the command from SRPT_STATE_NEED_DATA to
- * SRPT_STATE_DATA_IN ensures that srpt_xmit_response() will call this
- * function a second time.
+ * the ib_srpt driver, change the state to the next state.
*/
spin_lock_irqsave(&ioctx->spinlock, flags);
@@ -1279,25 +1276,17 @@
case SRPT_STATE_NEED_DATA:
ioctx->state = SRPT_STATE_DATA_IN;
break;
- case SRPT_STATE_DATA_IN:
case SRPT_STATE_CMD_RSP_SENT:
case SRPT_STATE_MGMT_RSP_SENT:
ioctx->state = SRPT_STATE_DONE;
break;
default:
+ WARN_ONCE(true, "%s: unexpected I/O context state %d\n",
+ __func__, state);
break;
}
spin_unlock_irqrestore(&ioctx->spinlock, flags);
- if (state == SRPT_STATE_DONE) {
- struct srpt_rdma_ch *ch = ioctx->ch;
-
- BUG_ON(ch->sess == NULL);
-
- target_put_sess_cmd(&ioctx->cmd);
- goto out;
- }
-
pr_debug("Aborting cmd with state %d and tag %lld\n", state,
ioctx->cmd.tag);
@@ -1305,19 +1294,16 @@
case SRPT_STATE_NEW:
case SRPT_STATE_DATA_IN:
case SRPT_STATE_MGMT:
+ case SRPT_STATE_DONE:
/*
* Do nothing - defer abort processing until
* srpt_queue_response() is invoked.
*/
- WARN_ON(!transport_check_aborted_status(&ioctx->cmd, false));
break;
case SRPT_STATE_NEED_DATA:
- /* DMA_TO_DEVICE (write) - RDMA read error. */
-
- /* XXX(hch): this is a horrible layering violation.. */
- spin_lock_irqsave(&ioctx->cmd.t_state_lock, flags);
- ioctx->cmd.transport_state &= ~CMD_T_ACTIVE;
- spin_unlock_irqrestore(&ioctx->cmd.t_state_lock, flags);
+ pr_debug("tag %#llx: RDMA read error\n", ioctx->cmd.tag);
+ transport_generic_request_failure(&ioctx->cmd,
+ TCM_CHECK_CONDITION_ABORT_CMD);
break;
case SRPT_STATE_CMD_RSP_SENT:
/*
@@ -1325,18 +1311,16 @@
* not been received in time.
*/
srpt_unmap_sg_to_ib_sge(ioctx->ch, ioctx);
- target_put_sess_cmd(&ioctx->cmd);
+ transport_generic_free_cmd(&ioctx->cmd, 0);
break;
case SRPT_STATE_MGMT_RSP_SENT:
- srpt_set_cmd_state(ioctx, SRPT_STATE_DONE);
- target_put_sess_cmd(&ioctx->cmd);
+ transport_generic_free_cmd(&ioctx->cmd, 0);
break;
default:
WARN(1, "Unexpected command state (%d)", state);
break;
}
-out:
return state;
}
@@ -1376,9 +1360,14 @@
container_of(wc->wr_cqe, struct srpt_send_ioctx, rdma_cqe);
if (unlikely(wc->status != IB_WC_SUCCESS)) {
+ /*
+ * Note: if an RDMA write error completion is received that
+ * means that a SEND also has been posted. Defer further
+ * processing of the associated command until the send error
+ * completion has been received.
+ */
pr_info("RDMA_WRITE for ioctx 0x%p failed with status %d\n",
ioctx, wc->status);
- srpt_abort_cmd(ioctx);
}
}
@@ -1721,15 +1710,10 @@
atomic_inc(&ch->sq_wr_avail);
- if (wc->status != IB_WC_SUCCESS) {
+ if (wc->status != IB_WC_SUCCESS)
pr_info("sending response for ioctx 0x%p failed"
" with status %d\n", ioctx, wc->status);
- atomic_dec(&ch->req_lim);
- srpt_abort_cmd(ioctx);
- goto out;
- }
-
if (state != SRPT_STATE_DONE) {
srpt_unmap_sg_to_ib_sge(ch, ioctx);
transport_generic_free_cmd(&ioctx->cmd, 0);
@@ -1738,7 +1722,6 @@
" wr_id = %u.\n", ioctx->ioctx.index);
}
-out:
while (!list_empty(&ch->cmd_wait_list) &&
ch->state == CH_LIVE &&
(ioctx = srpt_get_send_ioctx(ch)) != NULL) {