Message ID | 1437516477-30554-3-git-send-email-sbaugh@catern.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 7/21/15, 3:07 PM, "Spencer Baugh" <sbaugh@catern.com> wrote: >From: Dilip Kumar Uppugandla <dilip@purestorage.com> > >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 <dilip@purestorage.com> >Signed-off-by: Spencer Baugh <sbaugh@catern.com> >--- > 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 majordomo@vger.kernel.org >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 <himanshu.madhani@qlogic.com> >
On Tue, Jul 21, 2015 at 03:07:55PM -0700, Spencer Baugh wrote: > From: Dilip Kumar Uppugandla <dilip@purestorage.com> > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, "target-devel-owner@vger.kernel.org on behalf of Christoph Hellwig" <target-devel-owner@vger.kernel.org on behalf of hch@infradead.org> 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 majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }