Re: [PATCH v2 01/15] qla2xxx: Combine Active command arrays.

[Madhani, Himanshu]

Hi Bart,

> On Jul 11, 2017, at 5:39 PM, Nicholas A. Bellinger  
> wrote:
> 
> On Tue, 2017-07-11 at 23:43 +, Bart Van Assche wrote:
>> On Tue, 2017-06-13 at 20:47 -0700, Himanshu Madhani wrote:
>>> typedef struct srb {
>>> +   /*
>>> +* Do not move cmd_type field, it needs to
>>> +* line up with qla_tgt_cmd->cmd_type
>>> +*/
>>> +   uint8_t cmd_type;
>>> +   uint8_t pad[3];
>> 
>> Hello Himanshu,
>> 
>> Had I understood correctly that you had promised to rework the command
>> array merging such that a union is used instead of requiring certain
>> fields to be present at certain offsets (see also
>> https://www.spinics.net/lists/target-devel/msg15591.html)?
>> 
> 
> Yeah, as discussed previously it was not a blocker for merge.
> 

I had mentioned in my v2 submission cover letter that we will submit new patch 
to
address your review comment. 

Since we are in merge window right now i have not posted this patch. As soon as 
4.13.0-rc1 is out,
i will submit patch addressing your review comment. 

Thanks,
- Himanshu

Re: [PATCH v2 01/15] qla2xxx: Combine Active command arrays.

[Nicholas A. Bellinger]

On Tue, 2017-07-11 at 23:43 +, Bart Van Assche wrote:
> On Tue, 2017-06-13 at 20:47 -0700, Himanshu Madhani wrote:
> >  typedef struct srb {
> > +   /*
> > +* Do not move cmd_type field, it needs to
> > +* line up with qla_tgt_cmd->cmd_type
> > +*/
> > +   uint8_t cmd_type;
> > +   uint8_t pad[3];
> 
> Hello Himanshu,
> 
> Had I understood correctly that you had promised to rework the command
> array merging such that a union is used instead of requiring certain
> fields to be present at certain offsets (see also
> https://www.spinics.net/lists/target-devel/msg15591.html)?
> 

Yeah, as discussed previously it was not a blocker for merge.

Re: [PATCH v2 01/15] qla2xxx: Combine Active command arrays.

[Bart Van Assche]

On Tue, 2017-06-13 at 20:47 -0700, Himanshu Madhani wrote:
>  typedef struct srb {
> + /*
> +  * Do not move cmd_type field, it needs to
> +  * line up with qla_tgt_cmd->cmd_type
> +  */
> + uint8_t cmd_type;
> + uint8_t pad[3];

Hello Himanshu,

Had I understood correctly that you had promised to rework the command
array merging such that a union is used instead of requiring certain
fields to be present at certain offsets (see also
https://www.spinics.net/lists/target-devel/msg15591.html)?

Bart.

[PATCH v2 01/15] qla2xxx: Combine Active command arrays.

[Himanshu Madhani]

From: Quinn Tran 

Merge active/outstanding cmd arrays from target side
and initiator side together in prepration for Target
Multi Queue support.

Signed-off-by: Quinn Tran 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_def.h|  15 +++-
 drivers/scsi/qla2xxx/qla_gbl.h|   2 -
 drivers/scsi/qla2xxx/qla_inline.h |   1 +
 drivers/scsi/qla2xxx/qla_isr.c|  48 +
 drivers/scsi/qla2xxx/qla_os.c |  75 ++--
 drivers/scsi/qla2xxx/qla_target.c | 144 --
 drivers/scsi/qla2xxx/qla_target.h |  23 --
 7 files changed, 164 insertions(+), 144 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index ddf93efe3986..1b5049b1ef4a 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -437,7 +437,18 @@ struct srb_iocb {
 #define SRB_NACK_PRLI  17
 #define SRB_NACK_LOGO  18
 
+enum {
+   TYPE_SRB,
+   TYPE_TGT_CMD,
+};
+
 typedef struct srb {
+   /*
+* Do not move cmd_type field, it needs to
+* line up with qla_tgt_cmd->cmd_type
+*/
+   uint8_t cmd_type;
+   uint8_t pad[3];
atomic_t ref_count;
struct fc_port *fcport;
struct scsi_qla_host *vha;
@@ -3287,9 +3298,6 @@ struct qlt_hw_data {
uint32_t __iomem *atio_q_out;
 
struct qla_tgt_func_tmpl *tgt_ops;
-   struct qla_tgt_cmd *cmds[DEFAULT_OUTSTANDING_COMMANDS];
-   uint16_t current_handle;
-
struct qla_tgt_vp_map *tgt_vp_map;
 
int saved_set;
@@ -4258,6 +4266,7 @@ enum nexus_wait_type {
WAIT_LUN,
 };
 
+#include "qla_target.h"
 #include "qla_gbl.h"
 #include "qla_dbg.h"
 #include "qla_inline.h"
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index f8540f5c9e5d..63355f40ff2f 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -834,8 +834,6 @@ extern irqreturn_t qla8044_intr_handler(int, void *);
 extern void qla82xx_mbx_completion(scsi_qla_host_t *, uint16_t);
 extern int qla8044_abort_isp(scsi_qla_host_t *);
 extern int qla8044_check_fw_alive(struct scsi_qla_host *);
-
-extern void qlt_host_reset_handler(struct qla_hw_data *ha);
 extern int qla_get_exlogin_status(scsi_qla_host_t *, uint16_t *,
uint16_t *);
 extern int qla_set_exlogin_mem_cfg(scsi_qla_host_t *vha, dma_addr_t phys_addr);
diff --git a/drivers/scsi/qla2xxx/qla_inline.h 
b/drivers/scsi/qla2xxx/qla_inline.h
index 9996ec0daab1..99028d48c664 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -250,6 +250,7 @@ qla2x00_get_sp(scsi_qla_host_t *vha, fc_port_t *fcport, 
gfp_t flag)
 
memset(sp, 0, sizeof(*sp));
sp->fcport = fcport;
+   sp->cmd_type = TYPE_SRB;
sp->iocbs = 1;
sp->vha = vha;
 done:
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 2984abcc29e7..8aaddb75f964 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -17,7 +17,7 @@
 static void qla2x00_mbx_completion(scsi_qla_host_t *, uint16_t);
 static void qla2x00_status_entry(scsi_qla_host_t *, struct rsp_que *, void *);
 static void qla2x00_status_cont_entry(struct rsp_que *, sts_cont_entry_t *);
-static void qla2x00_error_entry(scsi_qla_host_t *, struct rsp_que *,
+static int qla2x00_error_entry(scsi_qla_host_t *, struct rsp_que *,
sts_entry_t *);
 
 /**
@@ -2280,6 +2280,14 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct 
rsp_que *rsp, void *pkt)
return;
}
 
+   if (sp->cmd_type != TYPE_SRB) {
+   req->outstanding_cmds[handle] = NULL;
+   ql_dbg(ql_dbg_io, vha, 0x3015,
+   "Unknown sp->cmd_type %x %p).\n",
+   sp->cmd_type, sp);
+   return;
+   }
+
if (unlikely((state_flags & BIT_1) && (sp->type == SRB_BIDI_CMD))) {
qla25xx_process_bidir_status_iocb(vha, pkt, req, handle);
return;
@@ -2632,8 +2640,9 @@ qla2x00_status_cont_entry(struct rsp_que *rsp, 
sts_cont_entry_t *pkt)
  * qla2x00_error_entry() - Process an error entry.
  * @ha: SCSI driver HA context
  * @pkt: Entry pointer
+ * return : 1=allow further error analysis. 0=no additional error analysis.
  */
-static void
+static int
 qla2x00_error_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, sts_entry_t 
*pkt)
 {
srb_t *sp;
@@ -2654,18 +2663,35 @@ qla2x00_error_entry(scsi_qla_host_t *vha, struct 
rsp_que *rsp, sts_entry_t *pkt)
if (pkt->entry_status & RF_BUSY)
res = DID_BUS_BUSY << 16;
 
-   if (pkt->entry_type == NOTIFY_ACK_TYPE &&
-   pkt->handle == QLA_TGT_SKIP_HANDLE)
-   return;
+   if ((pkt->handle & ~QLA_TGT_HANDLE_MASK) == QLA_TGT_SKIP_HANDLE)
+   return 0;
 
-   sp = qla2x00_get_sp_from_handle(vha, func, req, pkt);
-