* [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands
@ 2018-09-17 21:35 Bart Van Assche
2018-10-03 23:20 ` Bart Van Assche
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Bart Van Assche @ 2018-09-17 21:35 UTC (permalink / raw)
To: target-devel
Since the target core waits anyway until a target driver has
finished processing a command, remove similar waiting code from
the tcm_qla2xxx driver. With this change applied command abortion
works as follows:
* tcm_qla2xxx receives an ABTS and calls target_submit_tmr().
* The target core calls core_tmr_abort_task(). That function
sets the CMD_T_ABORTED flag and next calls transport_wait_for_tasks().
* If CMD_T_ACTIVE is still set, __transport_wait_for_tasks() sets
CMD_T_STOP and waits for t_transport_stop_comp.
* When tcm_qla2xxx_handle_data_work() gets called, it either invokes
transport_generic_request_failure() or target_execute_cmd().
* Both functions start with calling __transport_check_aborted_status()
and return 1 if CMD_T_ABORTED was set. Otherwise the command that
is being executed is completed and target_complete_cmd() completes
t_transport_stop_comp.
* Once transport_wait_for_tasks() returns the target core considers
the TMF as finished.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
Cc: Quinn Tran <quinn.tran@cavium.com>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index e03d12a5f986..2cedfd4f15a3 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -414,21 +414,14 @@ static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd)
{
unsigned long flags;
- /*
- * Check for WRITE_PENDING status to determine if we need to wait for
- * CTIO aborts to be posted via hardware in tcm_qla2xxx_handle_data().
- */
+ bool wp;
+
spin_lock_irqsave(&se_cmd->t_state_lock, flags);
- if (se_cmd->t_state = TRANSPORT_WRITE_PENDING ||
- se_cmd->t_state = TRANSPORT_COMPLETE_QF_WP) {
- spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
- wait_for_completion_timeout(&se_cmd->t_transport_stop_comp,
- 50);
- return 0;
- }
+ wp = se_cmd->t_state = TRANSPORT_WRITE_PENDING ||
+ se_cmd->t_state = TRANSPORT_COMPLETE_QF_WP;
spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
- return 0;
+ return wp;
}
static void tcm_qla2xxx_set_default_node_attrs(struct se_node_acl *nacl)
@@ -508,15 +501,6 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
cmd->qpair->tgt_counters.qla_core_ret_ctio++;
if (!cmd->write_data_transferred) {
- /*
- * Check if se_cmd has already been aborted via LUN_RESET, and
- * waiting upon completion in tcm_qla2xxx_write_pending_status()
- */
- if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
- complete(&cmd->se_cmd.t_transport_stop_comp);
- return;
- }
-
switch (cmd->dif_err_code) {
case DIF_ERR_GRD:
cmd->se_cmd.pi_err --
2.18.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands
2018-09-17 21:35 [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands Bart Van Assche
@ 2018-10-03 23:20 ` Bart Van Assche
2018-10-06 5:07 ` Madhani, Himanshu
2018-10-16 20:29 ` Madhani, Himanshu
2 siblings, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2018-10-03 23:20 UTC (permalink / raw)
To: target-devel
On Mon, 2018-09-17 at 14:35 -0700, Bart Van Assche wrote:
+AD4 Since the target core waits anyway until a target driver has
+AD4 finished processing a command, remove similar waiting code from
+AD4 the tcm+AF8-qla2xxx driver. With this change applied command abortion
+AD4 works as follows:
+AD4 +ACo tcm+AF8-qla2xxx receives an ABTS and calls target+AF8-submit+AF8-tmr().
+AD4 +ACo The target core calls core+AF8-tmr+AF8-abort+AF8-task(). That function
+AD4 sets the CMD+AF8-T+AF8-ABORTED flag and next calls transport+AF8-wait+AF8-for+AF8-tasks().
+AD4 +ACo If CMD+AF8-T+AF8-ACTIVE is still set, +AF8AXw-transport+AF8-wait+AF8-for+AF8-tasks() sets
+AD4 CMD+AF8-T+AF8-STOP and waits for t+AF8-transport+AF8-stop+AF8-comp.
+AD4 +ACo When tcm+AF8-qla2xxx+AF8-handle+AF8-data+AF8-work() gets called, it either invokes
+AD4 transport+AF8-generic+AF8-request+AF8-failure() or target+AF8-execute+AF8-cmd().
+AD4 +ACo Both functions start with calling +AF8AXw-transport+AF8-check+AF8-aborted+AF8-status()
+AD4 and return 1 if CMD+AF8-T+AF8-ABORTED was set. Otherwise the command that
+AD4 is being executed is completed and target+AF8-complete+AF8-cmd() completes
+AD4 t+AF8-transport+AF8-stop+AF8-comp.
+AD4 +ACo Once transport+AF8-wait+AF8-for+AF8-tasks() returns the target core considers
+AD4 the TMF as finished.
+AD4
+AD4 Signed-off-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4
+AD4 Cc: Himanshu Madhani +ADw-himanshu.madhani+AEA-cavium.com+AD4
+AD4 Cc: Quinn Tran +ADw-quinn.tran+AEA-cavium.com+AD4
+AD4 Cc: Nicholas Bellinger +ADw-nab+AEA-linux-iscsi.org+AD4
+AD4 Cc: Mike Christie +ADw-mchristi+AEA-redhat.com+AD4
+AD4 Cc: Christoph Hellwig +ADw-hch+AEA-lst.de+AD4
+AD4 Cc: Hannes Reinecke +ADw-hare+AEA-suse.de+AD4
Hello Himanshu and Quinn,
Can one of you review this patch?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands
2018-09-17 21:35 [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands Bart Van Assche
2018-10-03 23:20 ` Bart Van Assche
@ 2018-10-06 5:07 ` Madhani, Himanshu
2018-10-16 20:29 ` Madhani, Himanshu
2 siblings, 0 replies; 4+ messages in thread
From: Madhani, Himanshu @ 2018-10-06 5:07 UTC (permalink / raw)
To: target-devel
> On Oct 3, 2018, at 4:20 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> External Email
>
> On Mon, 2018-09-17 at 14:35 -0700, Bart Van Assche wrote:
>> Since the target core waits anyway until a target driver has
>> finished processing a command, remove similar waiting code from
>> the tcm_qla2xxx driver. With this change applied command abortion
>> works as follows:
>> * tcm_qla2xxx receives an ABTS and calls target_submit_tmr().
>> * The target core calls core_tmr_abort_task(). That function
>> sets the CMD_T_ABORTED flag and next calls transport_wait_for_tasks().
>> * If CMD_T_ACTIVE is still set, __transport_wait_for_tasks() sets
>> CMD_T_STOP and waits for t_transport_stop_comp.
>> * When tcm_qla2xxx_handle_data_work() gets called, it either invokes
>> transport_generic_request_failure() or target_execute_cmd().
>> * Both functions start with calling __transport_check_aborted_status()
>> and return 1 if CMD_T_ABORTED was set. Otherwise the command that
>> is being executed is completed and target_complete_cmd() completes
>> t_transport_stop_comp.
>> * Once transport_wait_for_tasks() returns the target core considers
>> the TMF as finished.
>>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
>> Cc: Quinn Tran <quinn.tran@cavium.com>
>> Cc: Nicholas Bellinger <nab@linux-iscsi.org>
>> Cc: Mike Christie <mchristi@redhat.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Hannes Reinecke <hare@suse.de>
>
> Hello Himanshu and Quinn,
>
> Can one of you review this patch?
>
> Thanks,
>
> Bart.
>
Apologies for delay. Still reviewing the patch
Thanks,
- Himanshu
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands
2018-09-17 21:35 [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands Bart Van Assche
2018-10-03 23:20 ` Bart Van Assche
2018-10-06 5:07 ` Madhani, Himanshu
@ 2018-10-16 20:29 ` Madhani, Himanshu
2 siblings, 0 replies; 4+ messages in thread
From: Madhani, Himanshu @ 2018-10-16 20:29 UTC (permalink / raw)
To: target-devel
Hi Bart,
Apologies for long delay
> On Sep 17, 2018, at 2:35 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> External Email
>
> Since the target core waits anyway until a target driver has
> finished processing a command, remove similar waiting code from
> the tcm_qla2xxx driver. With this change applied command abortion
> works as follows:
> * tcm_qla2xxx receives an ABTS and calls target_submit_tmr().
> * The target core calls core_tmr_abort_task(). That function
> sets the CMD_T_ABORTED flag and next calls transport_wait_for_tasks().
> * If CMD_T_ACTIVE is still set, __transport_wait_for_tasks() sets
> CMD_T_STOP and waits for t_transport_stop_comp.
> * When tcm_qla2xxx_handle_data_work() gets called, it either invokes
> transport_generic_request_failure() or target_execute_cmd().
> * Both functions start with calling __transport_check_aborted_status()
> and return 1 if CMD_T_ABORTED was set. Otherwise the command that
> is being executed is completed and target_complete_cmd() completes
> t_transport_stop_comp.
> * Once transport_wait_for_tasks() returns the target core considers
> the TMF as finished.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
> Cc: Quinn Tran <quinn.tran@cavium.com>
> Cc: Nicholas Bellinger <nab@linux-iscsi.org>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 26 +++++---------------------
> 1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index e03d12a5f986..2cedfd4f15a3 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -414,21 +414,14 @@ static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
> static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd)
> {
> unsigned long flags;
> - /*
> - * Check for WRITE_PENDING status to determine if we need to wait for
> - * CTIO aborts to be posted via hardware in tcm_qla2xxx_handle_data().
> - */
> + bool wp;
> +
> spin_lock_irqsave(&se_cmd->t_state_lock, flags);
> - if (se_cmd->t_state == TRANSPORT_WRITE_PENDING ||
> - se_cmd->t_state == TRANSPORT_COMPLETE_QF_WP) {
> - spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
> - wait_for_completion_timeout(&se_cmd->t_transport_stop_comp,
> - 50);
> - return 0;
> - }
> + wp = se_cmd->t_state == TRANSPORT_WRITE_PENDING ||
> + se_cmd->t_state == TRANSPORT_COMPLETE_QF_WP;
> spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
>
> - return 0;
> + return wp;
> }
>
> static void tcm_qla2xxx_set_default_node_attrs(struct se_node_acl *nacl)
> @@ -508,15 +501,6 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
>
> cmd->qpair->tgt_counters.qla_core_ret_ctio++;
> if (!cmd->write_data_transferred) {
> - /*
> - * Check if se_cmd has already been aborted via LUN_RESET, and
> - * waiting upon completion in tcm_qla2xxx_write_pending_status()
> - */
> - if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
> - complete(&cmd->se_cmd.t_transport_stop_comp);
> - return;
> - }
> -
> switch (cmd->dif_err_code) {
> case DIF_ERR_GRD:
> cmd->se_cmd.pi_err =
> --
> 2.18.0
>
Patch Looks good.
Acked-by: Himanshu Madhani <himanshu.madhani@cavium.com>
Thanks,
- Himanshu
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-16 20:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 21:35 [PATCH 14/17] target/qla2xxx: Simplify the code for handling aborted commands Bart Van Assche
2018-10-03 23:20 ` Bart Van Assche
2018-10-06 5:07 ` Madhani, Himanshu
2018-10-16 20:29 ` Madhani, Himanshu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.