On Thu, 2012-02-09 at 11:21 -0800, Roland Dreier wrote:
> Hi Nick,
>
> So I'm looking through the flow in tcm_qla2xxx when we have a WRITE that
> fails to transfer data. In tcm_qla2xxx_handle_data() we end up in
>
> if (!cmd->write_data_transferred) {
>
> ...
>
> se_cmd->scsi_sense_reason = TCM_CHECK_CONDITION_ABORT_CMD;
> INIT_WORK(&cmd->work, tcm_qla2xxx_do_rsp);
> queue_work(tcm_qla2xxx_free_wq, &cmd->work);
> return 0;
> }
>
> and tcm_qla2xxx_do_rsp() is just:
>
> void tcm_qla2xxx_do_rsp(struct work_struct *work)
> {
> struct qla_tgt_cmd *cmd = container_of(work, struct qla_tgt_cmd, work);
> /*
> * Dispatch ->queue_status from workqueue process context
> */
> transport_send_check_condition_and_sense(&cmd->se_cmd,
> cmd->se_cmd.scsi_sense_reason, 0);
> }
>
> so we'll send the status back but it seems we never drop the
> second kref on the command?
>
Bingo.. Great catch here Roland.
> I have to admit I'm still a bit vague on exactly what we're supposed
> to do to finish the command here. Is it as simple as adding
>
> target_put_sess_cmd()
>
> ?
>
> Anyway I think this explains one reason we get stuck waiting
> for session commands on shutdown -- if any commands ever
> hit this path, I guess they'll never get freed and so we'll end up
> waiting forever for them... make sense?
>
So since tcm_qla2xxx_do_rsp() will always have a valid se_device
pointer, the transport_send_check_condition_and_sense() call needs to be
wrapped in transport_generic_request_failure() in order to perform the
final TFO->check_stop_free() -> tcm_qla2xxx_check_stop_free() ->
target_put_sess_cmd().
This is similar what was fixed recently for target_submit_cmd() ->
transport_generic_allocate_tasks()..
Please give the following a shot.
Thanks,
--nab
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 83e5df4..8f291cd 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -626,8 +626,7 @@ void tcm_qla2xxx_do_rsp(struct work_struct *work)
/*
* Dispatch ->queue_status from workqueue process context
*/
- transport_send_check_condition_and_sense(&cmd->se_cmd,
- cmd->se_cmd.scsi_sense_reason, 0);
+ transport_generic_request_failure(cmd);
}
/*
--
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
