On Sun, 2014-03-23 at 11:44 -0700, Alex Leung wrote:
> Hi Nicholas,
>
> On 3/22/14, 3:50 PM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
> >
> > Now that TASK_ABORTED status is not generated for all cases by
> > TMR ABORT_TASK + LUN_RESET, a new TFO->abort_task() caller is
> > necessary in order to give fabric drivers a chance to unmap
> > hardware / software resources before the se_cmd descriptor is
> > released via the normal TFO->release_cmd() codepath.
> >
> > This patch adds TFO->aborted_task() in core_tmr_abort_task()
> > in place of the original transport_send_task_abort(), and
> > also updates all fabric drivers to implement this caller.
> >
> > The fabric drivers that include changes to perform cleanup
> > via ->aborted_task() are:
> >
> > - iscsi-target
> > - iser-target
> > - srpt
> > - tcm_qla2xxx
> >
> > The fabric drivers that currently set ->aborted_task() to
> > NOPs are:
> >
> > - loopback
> > - tcm_fc
> > - usb-gadget
> > - sbp-target
> > - vhost-scsi
> >
> > For the latter five, there appears to be no additional cleanup
> > required before invoking TFO->release_cmd() to release the
> > se_cmd descriptor.
> >
>
> Shouldn't this also be called for the LUN Reset path? i.e.
> core_tmr_lun_reset --> core_tmr_drain_state_list() -->
> core_tmr_handle_tas_abort(). Or better yet, just once from
> transport_cmd_finish_abort()?
>
Mmmm, good catch.
So calling ->aborted_task() from transport_cmd_finish_abort() makes
sense to me, but of course only when the se_cmd descriptor is being
released.
Here's an updated incremental patch that I'll fold into the original for
-v2.
Thanks again,
--nab
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index d355518..3f0338f 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -153,11 +153,6 @@ void core_tmr_abort_task(
cancel_work_sync(&se_cmd->work);
transport_wait_for_tasks(se_cmd);
- /*
- * Allow the fabric driver to unmap any resources before
- * releasing the descriptor via TFO->release_cmd()
- */
- se_cmd->se_tfo->aborted_task(se_cmd);
target_put_sess_cmd(se_sess, se_cmd);
transport_cmd_finish_abort(se_cmd, true);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 51a3754..a4a5179 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -605,6 +605,12 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int remove)
{
if (cmd->se_cmd_flags & SCF_SE_LUN_CMD)
transport_lun_remove_cmd(cmd);
+ /*
+ * Allow the fabric driver to unmap any resources before
+ * releasing the descriptor via TFO->release_cmd()
+ */
+ if (remove)
+ se_cmd->se_tfo->aborted_task(cmd);
if (transport_cmd_check_stop_to_fabric(cmd))
return;
--
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
