2015-07-21 22:08:21

by Spencer Baugh

[permalink] [raw]
Subject: [PATCH] qla2xxx: Return the fabric command state for non-task management requests

From: Dilip Kumar Uppugandla <[email protected]>

Invoking get_cmd_state for qla2xxx always returns 0. Instead change it
to return the actual fabric state from qla_tgt_cmd. This will help with
debugging.

Signed-off-by: Dilip Kumar Uppugandla <[email protected]>
Signed-off-by: Spencer Baugh <[email protected]>
---
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index d9a8c60..e859586 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -420,6 +420,12 @@ static void tcm_qla2xxx_set_default_node_attrs(struct se_node_acl *nacl)

static int tcm_qla2xxx_get_cmd_state(struct se_cmd *se_cmd)
{
+ if (!(se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) {
+ struct qla_tgt_cmd *cmd = container_of(se_cmd,
+ struct qla_tgt_cmd, se_cmd);
+ return cmd->state;
+ }
+
return 0;
}

--
2.4.3


2015-07-21 23:00:55

by Himanshu Madhani

[permalink] [raw]
Subject: Re: [PATCH] qla2xxx: Return the fabric command state for non-task management requests



On 7/21/15, 3:07 PM, "Spencer Baugh" <[email protected]> wrote:

>From: Dilip Kumar Uppugandla <[email protected]>
>
>Invoking get_cmd_state for qla2xxx always returns 0. Instead change it
>to return the actual fabric state from qla_tgt_cmd. This will help with
>debugging.
>
>Signed-off-by: Dilip Kumar Uppugandla <[email protected]>
>Signed-off-by: Spencer Baugh <[email protected]>
>---
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>index d9a8c60..e859586 100644
>--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
>@@ -420,6 +420,12 @@ static void
>tcm_qla2xxx_set_default_node_attrs(struct se_node_acl *nacl)
>
> static int tcm_qla2xxx_get_cmd_state(struct se_cmd *se_cmd)
> {
>+ if (!(se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) {
>+ struct qla_tgt_cmd *cmd = container_of(se_cmd,
>+ struct qla_tgt_cmd, se_cmd);
>+ return cmd->state;
>+ }
>+
> return 0;
> }
>
>--
>2.4.3
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/

Looks Good.

Acked-by: Himanshu Madhani <[email protected]>

>

2015-07-23 09:47:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] qla2xxx: Return the fabric command state for non-task management requests

On Tue, Jul 21, 2015 at 03:07:55PM -0700, Spencer Baugh wrote:
> From: Dilip Kumar Uppugandla <[email protected]>
>
> Invoking get_cmd_state for qla2xxx always returns 0. Instead change it
> to return the actual fabric state from qla_tgt_cmd. This will help with
> debugging.

I think the ->get_cmd_state callback should go away instead. Returning
values with zero meaning to the core in a callback doesn't make much
sense. I'd rather build some real structured debugging infrastructure,
so it would be useful if you could explain the use case for this.

2015-07-25 06:29:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] qla2xxx: Return the fabric command state for non-task management requests

Which debug printk do you care about? I'd much prefer having a trace
point inside the driver which could even pretty print it instead of the
hack where a driver defined binary value is printed by the core.

2015-07-27 23:26:21

by Quinn Tran

[permalink] [raw]
Subject: Re: [PATCH] qla2xxx: Return the fabric command state for non-task management requests

Christoph,

Currently, the command state within TCM (se_cmd.t_state) only track
command states from the point of new to the Back End and from Back End up
to QLA driver. From the debug perspective, that?s only half the picture.
The other half comes from qla2xxx?s private command state provided by this
patch.

Having another trace point that happens 5~10 seconds ago might be
difficult to back track. For task management debugging, dumping the
current states (se_cmd + qla_tgt_cmd) of each command when the TMR is
received provides some benefit.

If you feel uncomfortable with the "driver defined binary", one
alternative would be to translate QLA?s private command state into
se_cmd?s new field. This new file would be modify by the fabric layer
only. This would limit any regression with existing se_cmd field
modification.


Regards,
Quinn Tran




On 7/24/15, 11:29 PM, "[email protected] on behalf of
Christoph Hellwig" <[email protected] on behalf of
[email protected]> wrote:

>Which debug printk do you care about? I'd much prefer having a trace
>point inside the driver which could even pretty print it instead of the
>hack where a driver defined binary value is printed by the core.
>--
>To unsubscribe from this list: send the line "unsubscribe target-devel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html