On 04/14/15 21:41, Christoph Hellwig wrote:
> That being said the whole local ->aborted flag in tcm_fc looks like
> a bad hack to me.
Hello Nic and Christoph,
I have a patch ready that removes that hack but unfortunately I had to
introduce another hack to make that possible. Since there is no guarantee
that remaining FC frames will be received after an FC exchange has been
closed and since the LIO command abort implementation waits until a
command has finished, I had to introduce a call to ft_execute_work().
But calling that function is only safe after the CMD_T_ABORTED flag
has been set because otherwise uninitialized data could be written to
a local storage device. Hence the delay loop in the patch below. If
anyone would like to comment on this patch, feedback is welcome.
[PATCH] tcm_fc: Use target core to abort SCSI commands
Instead of implementing the functionality to abort a SCSI command
in the tcm_fc driver, use the existing functionality in the target
core to abort SCSI commands.
---
drivers/target/tcm_fc/tcm_fc.h | 1 -
drivers/target/tcm_fc/tfc_cmd.c | 80 +++++++++++++++++++++++++++++++++--------
drivers/target/tcm_fc/tfc_io.c | 10 ------
3 files changed, 66 insertions(+), 25 deletions(-)
diff --git a/drivers/target/tcm_fc/tcm_fc.h b/drivers/target/tcm_fc/tcm_fc.h
index b453d23..f0ee231 100644
--- a/drivers/target/tcm_fc/tcm_fc.h
+++ b/drivers/target/tcm_fc/tcm_fc.h
@@ -122,7 +122,6 @@ struct ft_cmd {
/* Local sense buffer */
unsigned char ft_sense_buffer[TRANSPORT_SENSE_BUFFER];
u32 was_ddp_setup:1; /* Set only if ddp is setup */
- u32 aborted:1; /* Set if aborted by reset or timeout */
struct scatterlist *sg; /* Set only if DDP is setup */
u32 sg_cnt; /* No. of item in scatterlist */
};
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 7775dbd..422bd83 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -96,13 +96,15 @@ static void ft_free_cmd(struct ft_cmd *cmd)
return;
sess = cmd->sess;
fp = cmd->req_frame;
- lport = fr_dev(fp);
- sp = fr_seq(fp);
- if (sp) {
- lport->tt.seq_set_resp(sp, NULL, NULL);
- lport->tt.seq_release(sp);
+ if (fp) {
+ lport = fr_dev(fp);
+ sp = fr_seq(fp);
+ if (sp) {
+ lport->tt.seq_set_resp(sp, NULL, NULL);
+ lport->tt.seq_release(sp);
+ }
+ fc_frame_free(fp);
}
- fc_frame_free(fp);
percpu_ida_free(&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag);
ft_sess_put(sess); /* undo get from lookup at recv */
}
@@ -134,8 +136,6 @@ int ft_queue_status(struct se_cmd *se_cmd)
size_t len;
int rc;
- if (cmd->aborted)
- return 0;
ft_dump_cmd(cmd, __func__);
ep = fc_seq_exch(cmd->seq);
lport = ep->lp;
@@ -214,8 +214,6 @@ int ft_write_pending(struct se_cmd *se_cmd)
ft_dump_cmd(cmd, __func__);
- if (cmd->aborted)
- return 0;
ep = fc_seq_exch(cmd->seq);
lport = ep->lp;
fp = fc_frame_alloc(lport, sizeof(*txrdy));
@@ -256,6 +254,59 @@ int ft_get_cmd_state(struct se_cmd *se_cmd)
return 0;
}
+static void ft_abort_cmd(struct ft_cmd *cmd)
+{
+ struct ft_sess *ft_sess = cmd->sess;
+ struct se_cmd *se_cmd = &cmd->se_cmd;
+ struct se_session *se_sess = se_cmd->se_sess;
+ struct se_lun *se_lun = se_cmd->se_lun;
+ int lun = se_lun ? se_lun->unpacked_lun : -1;
+ u16 rxid = se_cmd->tag;
+ struct ft_cmd *abort_tmr;
+ int abort_tmr_tag, rc;
+
+ pr_debug("Processing ABTS for LUN %d / RXID %#x\n", lun, rxid);
+
+ if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) {
+ pr_debug("Ignored an attempt to abort a TMF\n");
+ return;
+ }
+
+ kref_get(&se_cmd->cmd_kref);
+
+ abort_tmr_tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING);
+ if (abort_tmr_tag < 0) {
+ pr_err("Ignored ABTS for RXID %#x because out of tags\n", rxid);
+ return;
+ }
+
+ abort_tmr = &((struct ft_cmd *)se_sess->sess_cmd_map)[abort_tmr_tag];
+ memset(abort_tmr, 0, sizeof(*abort_tmr));
+ abort_tmr->se_cmd.map_tag = abort_tmr_tag;
+ abort_tmr->sess = ft_sess;
+
+ rc = target_submit_tmr(&abort_tmr->se_cmd, se_sess, NULL, lun,
+ abort_tmr, TMR_ABORT_TASK, GFP_KERNEL, rxid, 0);
+ if (rc < 0) {
+ pr_err("Failed to process ABTS for RXID %#x (%d)\n", rxid, rc);
+ percpu_ida_free(&se_sess->sess_tag_pool, abort_tmr_tag);
+ }
+
+ if (cmd->write_data_len < se_cmd->data_length) {
+ /*
+ * If the above target_submit_tmr() call will set the
+ * CMD_T_ABORTED flag, ensure that ft_execute_work() will see
+ * that flag.
+ */
+ while (!(se_cmd->transport_state & CMD_T_ABORTED) &&
+ se_cmd->t_state != TRANSPORT_COMPLETE)
+ mdelay(100);
+ ft_queue_execute_work(cmd);
+ }
+
+ target_put_sess_cmd(se_cmd);
+}
+
/*
* FC sequence response handler for follow-on sequences (data) and aborts.
*/
@@ -265,9 +316,8 @@ static void ft_recv_seq(struct fc_seq *sp, struct fc_frame *fp, void *arg)
struct fc_frame_header *fh;
if (unlikely(IS_ERR(fp))) {
- /* XXX need to find cmd if queued */
- cmd->seq = NULL;
- cmd->aborted = true;
+ WARN_ON_ONCE(cmd->se_cmd.tag != fc_seq_exch(sp)->rxid);
+ ft_abort_cmd(cmd);
return;
}
@@ -410,8 +460,10 @@ void ft_queue_tm_resp(struct se_cmd *se_cmd)
struct se_tmr_req *tmr = se_cmd->se_tmr_req;
enum fcp_resp_rsp_codes code;
- if (cmd->aborted)
+ /* fc_exch_recv_abts() already sent the ABTS response */
+ if (se_cmd->se_tmr_req->function == TMR_ABORT_TASK)
return;
+
switch (tmr->response) {
case TMR_FUNCTION_COMPLETE:
code = FCP_TMF_CMPL;
diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
index 7fb9fb3..d875bbd 100644
--- a/drivers/target/tcm_fc/tfc_io.c
+++ b/drivers/target/tcm_fc/tfc_io.c
@@ -79,9 +79,6 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
void *from;
void *to = NULL;
- if (cmd->aborted)
- return 0;
-
if (se_cmd->scsi_status == SAM_STAT_TASK_SET_FULL)
goto queue_status;
@@ -120,11 +117,6 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
while (remaining) {
struct fc_seq *seq = cmd->seq;
- if (!seq) {
- pr_debug("%s: Command aborted, xid 0x%x\n",
- __func__, ep->xid);
- break;
- }
if (!mem_len) {
sg = sg_next(sg);
mem_len = min((size_t)sg->length, remaining);
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
