RE: [PATCH 2/2] scsi: qla4xxx: remvoe unnecessary condition check for mempool_destroy()

[Rangankar, Manish]

> -Original Message-
> From: Chengguang Xu 
> Sent: Sunday, November 4, 2018 7:28 PM
> To: j...@linux.vnet.ibm.com; martin.peter...@oracle.com
> Cc: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; linux-scsi@vger.kernel.org; Chengguang Xu
> 
> Subject: [PATCH 2/2] scsi: qla4xxx: remvoe unnecessary condition check for
> mempool_destroy()
> 
> External Email
> 
> mempool_destroy() can handle NULL pointer correctly, so there is no need
> to check NULL pointer before calling mempool_destroy().
> 
> Signed-off-by: Chengguang Xu 
> ---
>  drivers/scsi/qla4xxx/ql4_os.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 039950ab1cbc..1c702cd22359 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -4160,9 +4160,7 @@ static void qla4xxx_mem_free(struct scsi_qla_host
> *ha)
> ha->fw_dump_size = 0;
> 
> /* Free srb pool. */
> -   if (ha->srb_mempool)
> -   mempool_destroy(ha->srb_mempool);
> -
> +   mempool_destroy(ha->srb_mempool);
> ha->srb_mempool = NULL;
> 
> dma_pool_destroy(ha->chap_dma_pool);
> --

Thanks
Acked-by: Manish Rangankar 

RE: [PATCH 1/2] scsi: qla4xxx: remove unnecessary condition check for dma_pool_destroy()

[Rangankar, Manish]

> -Original Message-
> From: Chengguang Xu 
> Sent: Sunday, November 4, 2018 7:28 PM
> To: j...@linux.vnet.ibm.com; martin.peter...@oracle.com
> Cc: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; linux-scsi@vger.kernel.org; Chengguang Xu
> 
> Subject: [PATCH 1/2] scsi: qla4xxx: remove unnecessary condition check for
> dma_pool_destroy()
> 
> External Email
> 
> dma_pool_destroy() can handle NULL pointer correctly, so there is no need
> to check NULL pointer before calling dma_pool_destroy().
> 
> Signed-off-by: Chengguang Xu 
> ---
>  drivers/scsi/qla4xxx/ql4_os.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 051164f755a4..039950ab1cbc 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -4165,15 +4165,13 @@ static void qla4xxx_mem_free(struct
> scsi_qla_host *ha)
> 
> ha->srb_mempool = NULL;
> 
> -   if (ha->chap_dma_pool)
> -   dma_pool_destroy(ha->chap_dma_pool);
> +   dma_pool_destroy(ha->chap_dma_pool);
> 
> if (ha->chap_list)
> vfree(ha->chap_list);
> ha->chap_list = NULL;
> 
> -   if (ha->fw_ddb_dma_pool)
> -   dma_pool_destroy(ha->fw_ddb_dma_pool);
> +   dma_pool_destroy(ha->fw_ddb_dma_pool);
> 
> /* release io space registers  */
> if (is_qla8022(ha)) {
> --

Thanks
Acked-by: Manish Rangankar 

RE: [PATCH 20/28] qedi: fully convert to the generic DMA API

[Rangankar, Manish]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org  On
> Behalf Of Christoph Hellwig
> Sent: Sunday, October 14, 2018 9:30 PM
> To: linux-scsi@vger.kernel.org
> Subject: [PATCH 20/28] qedi: fully convert to the generic DMA API
> 
> External Email
> 
> The driver is currently using an odd mix of legacy PCI DMA API and generic DMA
> API calls, switch it over to the generic API entirely.
> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Johannes Thumshirn 
> ---
>  drivers/scsi/qedi/qedi_main.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c 
> index
> aa96bccb5a96..95d3fce994f6 100644
> --- a/drivers/scsi/qedi/qedi_main.c
> +++ b/drivers/scsi/qedi/qedi_main.c
> @@ -806,11 +806,11 @@ static int qedi_set_iscsi_pf_param(struct qedi_ctx
> *qedi)
> memset(>pf_params.iscsi_pf_params, 0,
>sizeof(qedi->pf_params.iscsi_pf_params));
> 
> -   qedi->p_cpuq = pci_alloc_consistent(qedi->pdev,
> +   qedi->p_cpuq = dma_alloc_coherent(>pdev->dev,
> qedi->num_queues * sizeof(struct qedi_glbl_q_params),
> -   >hw_p_cpuq);
> +   >hw_p_cpuq, GFP_KERNEL);
> if (!qedi->p_cpuq) {
> -   QEDI_ERR(>dbg_ctx, "pci_alloc_consistent fail\n");
> +   QEDI_ERR(>dbg_ctx, "pci_alloc_coherent fail\n");

Trivial change, please update the QEDI_ERR message with correct function name.

> rval = -1;
> goto err_alloc_mem;
> }
> @@ -871,7 +871,7 @@ static void qedi_free_iscsi_pf_param(struct qedi_ctx
> *qedi)
> 
> if (qedi->p_cpuq) {
> size = qedi->num_queues * sizeof(struct qedi_glbl_q_params);
> -   pci_free_consistent(qedi->pdev, size, qedi->p_cpuq,
> +   dma_free_coherent(>pdev->dev, size, qedi->p_cpuq,
> qedi->hw_p_cpuq);
> }
> 
> --
> 2.19.1

Rest of the patch looks good to me.

Acked-by: Manish Rangankar 

RE: [PATCH] scsi: qla4xxx: remove redundant check on drvr_wait

[Rangankar, Manish]

> -Original Message-
> From: Colin King 
> Sent: Wednesday, September 26, 2018 6:39 PM
> To: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; James E . J . Bottomley ;
> Martin K . Petersen ; linux-scsi@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH] scsi: qla4xxx: remove redundant check on drvr_wait
> 
> External Email
> 
> From: Colin Ian King 
> 
> The check for a non-zero drvr_wait is redundant as the same check is performed
> earlier in the outer while loop, the inner check will always be true if we 
> reached
> this point inside the while loop.
> Remove the redundant if check.
> 
> Detected by cppcheck:
> (warning) Identical inner 'if' condition is always true.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/qla4xxx/ql4_init.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/qla4xxx/ql4_init.c 
> b/drivers/scsi/qla4xxx/ql4_init.c index
> 52b1a0bc93c9..1ef74aa2d00a 100644
> --- a/drivers/scsi/qla4xxx/ql4_init.c
> +++ b/drivers/scsi/qla4xxx/ql4_init.c
> @@ -766,12 +766,10 @@ int ql4xxx_lock_drvr_wait(struct scsi_qla_host *a)
> while (drvr_wait) {
> if (ql4xxx_lock_drvr(a) == 0) {
> ssleep(QL4_LOCK_DRVR_SLEEP);
> -   if (drvr_wait) {
> -   DEBUG2(printk("scsi%ld: %s: Waiting for "
> - "Global Init 
> Semaphore(%d)...\n",
> - a->host_no,
> - __func__, drvr_wait));
> -   }
> +   DEBUG2(printk("scsi%ld: %s: Waiting for "
> + "Global Init Semaphore(%d)...\n",
> + a->host_no,
> + __func__, drvr_wait));
> drvr_wait -= QL4_LOCK_DRVR_SLEEP;
> } else {
> DEBUG2(printk("scsi%ld: %s: Global Init Semaphore "
> --
> 2.17.1

Thanks,

Acked-by: Manish Rangankar 

RE: [PATCH] scsi: qedi: tidy up a size caculation

[Rangankar, Manish]

> -Original Message-
> From: Dan Carpenter 
> Sent: Thursday, June 28, 2018 2:53 PM
> To: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; Rangankar, Manish
> 
> Cc: James E.J. Bottomley ; Martin K. Petersen
> ; linux-scsi@vger.kernel.org; kernel-
> janit...@vger.kernel.org
> Subject: [PATCH] scsi: qedi: tidy up a size caculation
> 
> External Email
> 
> The id_tbl->table pointer points to unsigned long so static checkers complain
> that instead of 4 we should be allocating sizeof(long) bytes.
> 
> We're trying to allocate enough bits for the bitmap.  The size variable is 
> always
> 1024.  (1024 / 32 * 4) is the same as (1024 / 64 * 8) so this doesn't change
> runtime, but this is the more idiomatic way to do it and makes the static 
> checker
> happy.
> 
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c 
> index
> cf274a79e77a..682f3ce31014 100644
> --- a/drivers/scsi/qedi/qedi_main.c
> +++ b/drivers/scsi/qedi/qedi_main.c
> @@ -524,7 +524,7 @@ static int qedi_init_id_tbl(struct qedi_portid_tbl 
> *id_tbl,
> u16 size,
> id_tbl->max = size;
> id_tbl->next = next;
> spin_lock_init(_tbl->lock);
> -   id_tbl->table = kcalloc(DIV_ROUND_UP(size, 32), 4, GFP_KERNEL);
> +   id_tbl->table = kcalloc(BITS_TO_LONGS(size), sizeof(long),
> + GFP_KERNEL);
> if (!id_tbl->table)
> return -ENOMEM;

Thanks,

Acked-by: Manish Rangankar 

Re: [PATCH] scsi: qla4xxx: Use dma_pool_zalloc()

[Rangankar, Manish]

On 15/02/18 12:30 AM, "Souptick Joarder"  wrote:

>Use dma_pool_zalloc() instead of dma_pool_alloc + memset
>
>Signed-off-by: Souptick Joarder 
>---
> drivers/scsi/qla4xxx/ql4_mbx.c | 6 ++
> drivers/scsi/qla4xxx/ql4_os.c  | 4 +---
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c
>b/drivers/scsi/qla4xxx/ql4_mbx.c
>index bda2e64..5d56904 100644
>--- a/drivers/scsi/qla4xxx/ql4_mbx.c
>+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
>@@ -1584,12 +1584,11 @@ int qla4xxx_get_chap(struct scsi_qla_host *ha,
>char *username, char *password,
>   struct ql4_chap_table *chap_table;
>   dma_addr_t chap_dma;
>
>-  chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL, _dma);
>+  chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL, _dma);
>   if (chap_table == NULL)
>   return -ENOMEM;
>
>   chap_size = sizeof(struct ql4_chap_table);
>-  memset(chap_table, 0, chap_size);
>
>   if (is_qla40XX(ha))
>   offset = FLASH_CHAP_OFFSET | (idx * chap_size);
>@@ -1648,13 +1647,12 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha,
>char *username, char *password,
>   uint32_t chap_size = 0;
>   dma_addr_t chap_dma;
>
>-  chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL, _dma);
>+  chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL, _dma);
>   if (chap_table == NULL) {
>   ret =  -ENOMEM;
>   goto exit_set_chap;
>   }
>
>-  memset(chap_table, 0, sizeof(struct ql4_chap_table));
>   if (bidi)
>   chap_table->flags |= BIT_6; /* peer */
>   else
>diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
>index 82e889b..5896343 100644
>--- a/drivers/scsi/qla4xxx/ql4_os.c
>+++ b/drivers/scsi/qla4xxx/ql4_os.c
>@@ -825,12 +825,10 @@ static int qla4xxx_delete_chap(struct Scsi_Host
>*shost, uint16_t chap_tbl_idx)
>   uint32_t chap_size;
>   int ret = 0;
>
>-  chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL, _dma);
>+  chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL, _dma);
>   if (chap_table == NULL)
>   return -ENOMEM;
>
>-  memset(chap_table, 0, sizeof(struct ql4_chap_table));
>-
>   if (is_qla80XX(ha))
>   max_chap_entries = (ha->hw.flt_chap_size / 2) /
>  sizeof(struct ql4_chap_table);
>--
>1.9.1

Thanks,

Acked-by: Manish Rangankar 

Re: [PATCH 5/6] scsi: qedi: fix building with LTO

[Rangankar, Manish]

On 02/02/18 6:42 PM, "Arnd Bergmann"  wrote:

>When link-time optimizations are enabled, qedi fails to build because
>of mismatched prototypes:
>
>drivers/scsi/qedi/qedi_gbl.h:27:37: error: type of 'qedi_dbg_fops' does
>not match original declaration [-Werror=lto-type-mismatch]
> extern const struct file_operations qedi_dbg_fops;
> ^
>drivers/scsi/qedi/qedi_debugfs.c:239:30: note: 'qedi_dbg_fops' was
>previously declared here
> const struct file_operations qedi_dbg_fops[] = {
>  ^
>drivers/scsi/qedi/qedi_gbl.h:26:32: error: type of 'qedi_debugfs_ops'
>does not match original declaration [-Werror=lto-type-mismatch]
> extern struct qedi_debugfs_ops qedi_debugfs_ops;
>^
>drivers/scsi/qedi/qedi_debugfs.c:102:25: note: 'qedi_debugfs_ops' was
>previously declared here
> struct qedi_debugfs_ops qedi_debugfs_ops[] = {
>
>This changes the declaration to match the definition, and adapts the
>users as necessary. Since both array can be constant here, I'm adding
>the 'const' everywhere for consistency.
>
>Signed-off-by: Arnd Bergmann 
>---
> drivers/scsi/qedi/qedi_dbg.h | 2 +-
> drivers/scsi/qedi/qedi_debugfs.c | 4 ++--
> drivers/scsi/qedi/qedi_gbl.h | 4 ++--
> drivers/scsi/qedi/qedi_main.c| 4 ++--
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_dbg.h b/drivers/scsi/qedi/qedi_dbg.h
>index c55572badfb0..358f40567849 100644
>--- a/drivers/scsi/qedi/qedi_dbg.h
>+++ b/drivers/scsi/qedi/qedi_dbg.h
>@@ -134,7 +134,7 @@ struct qedi_debugfs_ops {
> }
> 
> void qedi_dbg_host_init(struct qedi_dbg_ctx *qedi,
>-  struct qedi_debugfs_ops *dops,
>+  const struct qedi_debugfs_ops *dops,
>   const struct file_operations *fops);
> void qedi_dbg_host_exit(struct qedi_dbg_ctx *qedi);
> void qedi_dbg_init(char *drv_name);
>diff --git a/drivers/scsi/qedi/qedi_debugfs.c
>b/drivers/scsi/qedi/qedi_debugfs.c
>index fd8a1eea3163..fd914ca4149a 100644
>--- a/drivers/scsi/qedi/qedi_debugfs.c
>+++ b/drivers/scsi/qedi/qedi_debugfs.c
>@@ -19,7 +19,7 @@ static struct dentry *qedi_dbg_root;
> 
> void
> qedi_dbg_host_init(struct qedi_dbg_ctx *qedi,
>- struct qedi_debugfs_ops *dops,
>+ const struct qedi_debugfs_ops *dops,
>  const struct file_operations *fops)
> {
>   char host_dirname[32];
>@@ -99,7 +99,7 @@ static struct qedi_list_of_funcs
>qedi_dbg_do_not_recover_ops[] = {
>   { NULL, NULL }
> };
> 
>-struct qedi_debugfs_ops qedi_debugfs_ops[] = {
>+const struct qedi_debugfs_ops qedi_debugfs_ops[] = {
>   { "gbl_ctx", NULL },
>   { "do_not_recover", qedi_dbg_do_not_recover_ops},
>   { "io_trace", NULL },
>diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h
>index f5b5a31999aa..a2aa06ed1620 100644
>--- a/drivers/scsi/qedi/qedi_gbl.h
>+++ b/drivers/scsi/qedi/qedi_gbl.h
>@@ -23,8 +23,8 @@ extern uint qedi_io_tracing;
> extern struct scsi_host_template qedi_host_template;
> extern struct iscsi_transport qedi_iscsi_transport;
> extern const struct qed_iscsi_ops *qedi_ops;
>-extern struct qedi_debugfs_ops qedi_debugfs_ops;
>-extern const struct file_operations qedi_dbg_fops;
>+extern const struct qedi_debugfs_ops qedi_debugfs_ops[];
>+extern const struct file_operations qedi_dbg_fops[];
> extern struct device_attribute *qedi_shost_attrs[];
> 
> int qedi_alloc_sq(struct qedi_ctx *qedi, struct qedi_endpoint *ep);
>diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
>index 029e2e69b29f..e992f9d3ef00 100644
>--- a/drivers/scsi/qedi/qedi_main.c
>+++ b/drivers/scsi/qedi/qedi_main.c
>@@ -2303,8 +2303,8 @@ static int __qedi_probe(struct pci_dev *pdev, int
>mode)
>   }
> 
> #ifdef CONFIG_DEBUG_FS
>-  qedi_dbg_host_init(>dbg_ctx, _debugfs_ops,
>- _dbg_fops);
>+  qedi_dbg_host_init(>dbg_ctx, qedi_debugfs_ops,
>+ qedi_dbg_fops);
> #endif
>   QEDI_INFO(>dbg_ctx, QEDI_LOG_INFO,
> "QLogic FastLinQ iSCSI Module qedi %s, FW %d.%d.%d.%d\n",
>-- 
>2.9.0

Thanks

Acked-by: Manish Rangankar 


>

Re: [RESEND PATCH 2/2] scsi: qedi: Use zeroing allocator instead of allocator/memset

[Rangankar, Manish]

On 09/01/18 2:36 PM, "Himanshu Jha"  wrote:

>Use dma_zalloc_coherent instead of dma_alloc_coherent followed by memset
>0.
>
>Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
>Suggested-by: Luis R. Rodriguez 
>Signed-off-by: Himanshu Jha 
>---
> drivers/scsi/qedi/qedi_main.c | 42
>+++---
> 1 file changed, 15 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
>index 34a..5ef0b36 100644
>--- a/drivers/scsi/qedi/qedi_main.c
>+++ b/drivers/scsi/qedi/qedi_main.c
>@@ -1268,16 +1268,14 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
>   }
> 
>   /* Allocate list of PBL pages */
>-  qedi->bdq_pbl_list = dma_alloc_coherent(>pdev->dev,
>-  PAGE_SIZE,
>-  >bdq_pbl_list_dma,
>-  GFP_KERNEL);
>+  qedi->bdq_pbl_list = dma_zalloc_coherent(>pdev->dev, PAGE_SIZE,
>+   >bdq_pbl_list_dma,
>+   GFP_KERNEL);
>   if (!qedi->bdq_pbl_list) {
>   QEDI_ERR(>dbg_ctx,
>"Could not allocate list of PBL pages.\n");
>   return -ENOMEM;
>   }
>-  memset(qedi->bdq_pbl_list, 0, PAGE_SIZE);
> 
>   /*
>* Now populate PBL list with pages that contain pointers to the
>@@ -1367,11 +1365,10 @@ static int qedi_alloc_global_queues(struct
>qedi_ctx *qedi)
>   (qedi->global_queues[i]->cq_pbl_size +
>   (QEDI_PAGE_SIZE - 1));
> 
>-  qedi->global_queues[i]->cq =
>-  dma_alloc_coherent(>pdev->dev,
>- qedi->global_queues[i]->cq_mem_size,
>- >global_queues[i]->cq_dma,
>- GFP_KERNEL);
>+  qedi->global_queues[i]->cq = 
>dma_zalloc_coherent(>pdev->dev,
>+   
>qedi->global_queues[i]->cq_mem_size,
>+   
>>global_queues[i]->cq_dma,
>+   GFP_KERNEL);
> 
>   if (!qedi->global_queues[i]->cq) {
>   QEDI_WARN(>dbg_ctx,
>@@ -1379,14 +1376,10 @@ static int qedi_alloc_global_queues(struct
>qedi_ctx *qedi)
>   status = -ENOMEM;
>   goto mem_alloc_failure;
>   }
>-  memset(qedi->global_queues[i]->cq, 0,
>- qedi->global_queues[i]->cq_mem_size);
>-
>-  qedi->global_queues[i]->cq_pbl =
>-  dma_alloc_coherent(>pdev->dev,
>- qedi->global_queues[i]->cq_pbl_size,
>- >global_queues[i]->cq_pbl_dma,
>- GFP_KERNEL);
>+  qedi->global_queues[i]->cq_pbl = 
>dma_zalloc_coherent(>pdev->dev,
>+   
>qedi->global_queues[i]->cq_pbl_size,
>+   
>>global_queues[i]->cq_pbl_dma,
>+   
>GFP_KERNEL);
> 
>   if (!qedi->global_queues[i]->cq_pbl) {
>   QEDI_WARN(>dbg_ctx,
>@@ -1394,8 +1387,6 @@ static int qedi_alloc_global_queues(struct qedi_ctx
>*qedi)
>   status = -ENOMEM;
>   goto mem_alloc_failure;
>   }
>-  memset(qedi->global_queues[i]->cq_pbl, 0,
>- qedi->global_queues[i]->cq_pbl_size);
> 
>   /* Create PBL */
>   num_pages = qedi->global_queues[i]->cq_mem_size /
>@@ -1456,25 +1447,22 @@ int qedi_alloc_sq(struct qedi_ctx *qedi, struct
>qedi_endpoint *ep)
>   ep->sq_pbl_size = (ep->sq_mem_size / QEDI_PAGE_SIZE) * sizeof(void *);
>   ep->sq_pbl_size = ep->sq_pbl_size + QEDI_PAGE_SIZE;
> 
>-  ep->sq = dma_alloc_coherent(>pdev->dev, ep->sq_mem_size,
>-  >sq_dma, GFP_KERNEL);
>+  ep->sq = dma_zalloc_coherent(>pdev->dev, ep->sq_mem_size,
>+   >sq_dma, GFP_KERNEL);
>   if (!ep->sq) {
>   QEDI_WARN(>dbg_ctx,
> "Could not allocate send queue.\n");
>   rval = -ENOMEM;
>   goto out;
>   }
>-  memset(ep->sq, 0, ep->sq_mem_size);
>-
>-  ep->sq_pbl = dma_alloc_coherent(>pdev->dev, ep->sq_pbl_size,
>-  >sq_pbl_dma, GFP_KERNEL);
>+  ep->sq_pbl = dma_zalloc_coherent(>pdev->dev, ep->sq_pbl_size,
>+   >sq_pbl_dma, GFP_KERNEL);
>   if 

Re: [PATCH 9/9] scsi: bnx2i: Use zeroing allocator rather than allocator/memset

[Rangankar, Manish]

On 30/12/17 8:58 PM, "Himanshu Jha"  wrote:

>Use dma_zalloc_coherent instead of dma_alloc_coherent followed by
>memset 0.
>
>Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
>Suggested-by: Luis R. Rodriguez 
>Signed-off-by: Himanshu Jha 
>---
> drivers/scsi/bnx2i/bnx2i_hwi.c | 14 ++
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c
>b/drivers/scsi/bnx2i/bnx2i_hwi.c
>index 9e3bf53..c6a0bd6 100644
>--- a/drivers/scsi/bnx2i/bnx2i_hwi.c
>+++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
>@@ -1069,16 +1069,15 @@ int bnx2i_alloc_qp_resc(struct bnx2i_hba *hba,
>struct bnx2i_endpoint *ep)
>   }
> 
>   /* Allocate memory area for actual SQ element */
>-  ep->qp.sq_virt =
>-  dma_alloc_coherent(>pcidev->dev, ep->qp.sq_mem_size,
>- >qp.sq_phys, GFP_KERNEL);
>+  ep->qp.sq_virt =
>+  dma_zalloc_coherent(>pcidev->dev, ep->qp.sq_mem_size,
>+  >qp.sq_phys, GFP_KERNEL);
>   if (!ep->qp.sq_virt) {
>   printk(KERN_ALERT "bnx2i: unable to alloc SQ BD memory %d\n",
> ep->qp.sq_mem_size);
>   goto mem_alloc_err;
>   }
> 
>-  memset(ep->qp.sq_virt, 0x00, ep->qp.sq_mem_size);
>   ep->qp.sq_first_qe = ep->qp.sq_virt;
>   ep->qp.sq_prod_qe = ep->qp.sq_first_qe;
>   ep->qp.sq_cons_qe = ep->qp.sq_first_qe;
>@@ -1106,15 +1105,14 @@ int bnx2i_alloc_qp_resc(struct bnx2i_hba *hba,
>struct bnx2i_endpoint *ep)
>   }
> 
>   /* Allocate memory area for actual CQ element */
>-  ep->qp.cq_virt =
>-  dma_alloc_coherent(>pcidev->dev, ep->qp.cq_mem_size,
>- >qp.cq_phys, GFP_KERNEL);
>+  ep->qp.cq_virt =
>+  dma_zalloc_coherent(>pcidev->dev, ep->qp.cq_mem_size,
>+  >qp.cq_phys, GFP_KERNEL);
>   if (!ep->qp.cq_virt) {
>   printk(KERN_ALERT "bnx2i: unable to alloc CQ BD memory %d\n",
> ep->qp.cq_mem_size);
>   goto mem_alloc_err;
>   }
>-  memset(ep->qp.cq_virt, 0x00, ep->qp.cq_mem_size);
> 
>   ep->qp.cq_first_qe = ep->qp.cq_virt;
>   ep->qp.cq_prod_qe = ep->qp.cq_first_qe;
>-- 
>2.7.4

Acked-by: Manish Rangankar 


>

Re: [PATCH 3/9] scsi: qedi: Use zeroing allocator instead of allocator/memset

[Rangankar, Manish]

On 30/12/17 8:58 PM, "Himanshu Jha"  wrote:

>Use dma_zalloc_coherent instead of dma_alloc_coherent followed by memset
>0.
>
>Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
>Suggested-by: Luis R. Rodriguez 
>Signed-off-by: Himanshu Jha 
>---
> drivers/scsi/qedi/qedi_main.c | 42
>+++---
> 1 file changed, 15 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
>index 34a..5ef0b36 100644
>--- a/drivers/scsi/qedi/qedi_main.c
>+++ b/drivers/scsi/qedi/qedi_main.c
>@@ -1268,16 +1268,14 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
>   }
> 
>   /* Allocate list of PBL pages */
>-  qedi->bdq_pbl_list = dma_alloc_coherent(>pdev->dev,
>-  PAGE_SIZE,
>-  >bdq_pbl_list_dma,
>-  GFP_KERNEL);
>+  qedi->bdq_pbl_list = dma_zalloc_coherent(>pdev->dev, PAGE_SIZE,
>+   >bdq_pbl_list_dma,
>+   GFP_KERNEL);
>   if (!qedi->bdq_pbl_list) {
>   QEDI_ERR(>dbg_ctx,
>"Could not allocate list of PBL pages.\n");
>   return -ENOMEM;
>   }
>-  memset(qedi->bdq_pbl_list, 0, PAGE_SIZE);
> 
>   /*
>* Now populate PBL list with pages that contain pointers to the
>@@ -1367,11 +1365,10 @@ static int qedi_alloc_global_queues(struct
>qedi_ctx *qedi)
>   (qedi->global_queues[i]->cq_pbl_size +
>   (QEDI_PAGE_SIZE - 1));
> 
>-  qedi->global_queues[i]->cq =
>-  dma_alloc_coherent(>pdev->dev,
>- qedi->global_queues[i]->cq_mem_size,
>- >global_queues[i]->cq_dma,
>- GFP_KERNEL);
>+  qedi->global_queues[i]->cq =
>+  dma_zalloc_coherent(>pdev->dev,
>+  qedi->global_queues[i]->cq_mem_size,
>+  >global_queues[i]->cq_dma,
>+  GFP_KERNEL);
> 
>   if (!qedi->global_queues[i]->cq) {
>   QEDI_WARN(>dbg_ctx,
>@@ -1379,14 +1376,10 @@ static int qedi_alloc_global_queues(struct
>qedi_ctx *qedi)
>   status = -ENOMEM;
>   goto mem_alloc_failure;
>   }
>-  memset(qedi->global_queues[i]->cq, 0,
>- qedi->global_queues[i]->cq_mem_size);
>-
>-  qedi->global_queues[i]->cq_pbl =
>-  dma_alloc_coherent(>pdev->dev,
>- qedi->global_queues[i]->cq_pbl_size,
>- >global_queues[i]->cq_pbl_dma,
>- GFP_KERNEL);
>+  qedi->global_queues[i]->cq_pbl =
>+  dma_zalloc_coherent(>pdev->dev,
>+  qedi->global_queues[i]->cq_pbl_size,
>+  >global_queues[i]->cq_pbl_dma,
>+  GFP_KERNEL);
> 
>   if (!qedi->global_queues[i]->cq_pbl) {
>   QEDI_WARN(>dbg_ctx,
>@@ -1394,8 +1387,6 @@ static int qedi_alloc_global_queues(struct qedi_ctx
>*qedi)
>   status = -ENOMEM;
>   goto mem_alloc_failure;
>   }
>-  memset(qedi->global_queues[i]->cq_pbl, 0,
>- qedi->global_queues[i]->cq_pbl_size);
> 
>   /* Create PBL */
>   num_pages = qedi->global_queues[i]->cq_mem_size /
>@@ -1456,25 +1447,22 @@ int qedi_alloc_sq(struct qedi_ctx *qedi, struct
>qedi_endpoint *ep)
>   ep->sq_pbl_size = (ep->sq_mem_size / QEDI_PAGE_SIZE) * sizeof(void *);
>   ep->sq_pbl_size = ep->sq_pbl_size + QEDI_PAGE_SIZE;
> 
>-  ep->sq = dma_alloc_coherent(>pdev->dev, ep->sq_mem_size,
>-  >sq_dma, GFP_KERNEL);
>+  ep->sq = dma_zalloc_coherent(>pdev->dev, ep->sq_mem_size,
>+   >sq_dma, GFP_KERNEL);
>   if (!ep->sq) {
>   QEDI_WARN(>dbg_ctx,
> "Could not allocate send queue.\n");
>   rval = -ENOMEM;
>   goto out;
>   }
>-  memset(ep->sq, 0, ep->sq_mem_size);
>-
>-  ep->sq_pbl = dma_alloc_coherent(>pdev->dev, ep->sq_pbl_size,
>-  >sq_pbl_dma, GFP_KERNEL);
>+  ep->sq_pbl = dma_zalloc_coherent(>pdev->dev, ep->sq_pbl_size,
>+   >sq_pbl_dma, GFP_KERNEL);
>   if (!ep->sq_pbl) {
>   QEDI_WARN(>dbg_ctx,
> "Could not allocate send queue PBL.\n");
>   rval 

Re: [PATCH 1/9] scsi: qla4xxx: Use zeroing allocator rather than allocator/memset

[Rangankar, Manish]

On 30/12/17 8:58 PM, "Himanshu Jha"  wrote:

>Use dma_zalloc_coherent instead of dma_alloc_coherent followed by memset
>0.
>
>Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
>Suggested-by: Luis R. Rodriguez 
>Signed-off-by: Himanshu Jha 
>---
> drivers/scsi/qla4xxx/ql4_init.c |  5 ++---
> drivers/scsi/qla4xxx/ql4_mbx.c  | 21 +
> drivers/scsi/qla4xxx/ql4_nx.c   |  5 ++---
> drivers/scsi/qla4xxx/ql4_os.c   | 12 +---
> 4 files changed, 18 insertions(+), 25 deletions(-)
>
>diff --git a/drivers/scsi/qla4xxx/ql4_init.c
>b/drivers/scsi/qla4xxx/ql4_init.c
>index 5d6d158..52b1a0b 100644
>--- a/drivers/scsi/qla4xxx/ql4_init.c
>+++ b/drivers/scsi/qla4xxx/ql4_init.c
>@@ -153,15 +153,14 @@ int qla4xxx_get_sys_info(struct scsi_qla_host *ha)
>   dma_addr_t sys_info_dma;
>   int status = QLA_ERROR;
> 
>-  sys_info = dma_alloc_coherent(>pdev->dev, sizeof(*sys_info),
>-_info_dma, GFP_KERNEL);
>+  sys_info = dma_zalloc_coherent(>pdev->dev, sizeof(*sys_info),
>+ _info_dma, GFP_KERNEL);
>   if (sys_info == NULL) {
>   DEBUG2(printk("scsi%ld: %s: Unable to allocate dma buffer.\n",
> ha->host_no, __func__));
> 
>   goto exit_get_sys_info_no_free;
>   }
>-  memset(sys_info, 0, sizeof(*sys_info));
> 
>   /* Get flash sys info */
>   if (qla4xxx_get_flash(ha, sys_info_dma, FLASH_OFFSET_SYS_INFO,
>diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c
>b/drivers/scsi/qla4xxx/ql4_mbx.c
>index 1da04f3..bda2e64 100644
>--- a/drivers/scsi/qla4xxx/ql4_mbx.c
>+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
>@@ -625,15 +625,14 @@ int qla4xxx_initialize_fw_cb(struct scsi_qla_host *
>ha)
>   uint32_t mbox_sts[MBOX_REG_COUNT];
>   int status = QLA_ERROR;
> 
>-  init_fw_cb = dma_alloc_coherent(>pdev->dev,
>-  sizeof(struct addr_ctrl_blk),
>-  _fw_cb_dma, GFP_KERNEL);
>+  init_fw_cb = dma_zalloc_coherent(>pdev->dev,
>+   sizeof(struct addr_ctrl_blk),
>+   _fw_cb_dma, GFP_KERNEL);
>   if (init_fw_cb == NULL) {
>   DEBUG2(printk("scsi%ld: %s: Unable to alloc init_cb\n",
> ha->host_no, __func__));
>   goto exit_init_fw_cb_no_free;
>   }
>-  memset(init_fw_cb, 0, sizeof(struct addr_ctrl_blk));
> 
>   /* Get Initialize Firmware Control Block. */
>   memset(_cmd, 0, sizeof(mbox_cmd));
>@@ -710,9 +709,9 @@ int qla4xxx_get_dhcp_ip_address(struct scsi_qla_host
>* ha)
>   uint32_t mbox_cmd[MBOX_REG_COUNT];
>   uint32_t mbox_sts[MBOX_REG_COUNT];
> 
>-  init_fw_cb = dma_alloc_coherent(>pdev->dev,
>-  sizeof(struct addr_ctrl_blk),
>-  _fw_cb_dma, GFP_KERNEL);
>+  init_fw_cb = dma_zalloc_coherent(>pdev->dev,
>+   sizeof(struct addr_ctrl_blk),
>+   _fw_cb_dma, GFP_KERNEL);
>   if (init_fw_cb == NULL) {
>   printk("scsi%ld: %s: Unable to alloc init_cb\n", ha->host_no,
>  __func__);
>@@ -720,7 +719,6 @@ int qla4xxx_get_dhcp_ip_address(struct scsi_qla_host
>* ha)
>   }
> 
>   /* Get Initialize Firmware Control Block. */
>-  memset(init_fw_cb, 0, sizeof(struct addr_ctrl_blk));
>   if (qla4xxx_get_ifcb(ha, _cmd[0], _sts[0], init_fw_cb_dma) !=
>   QLA_SUCCESS) {
>   DEBUG2(printk("scsi%ld: %s: Failed to get init_fw_ctrl_blk\n",
>@@ -1342,16 +1340,15 @@ int qla4xxx_about_firmware(struct scsi_qla_host
>*ha)
>   uint32_t mbox_sts[MBOX_REG_COUNT];
>   int status = QLA_ERROR;
> 
>-  about_fw = dma_alloc_coherent(>pdev->dev,
>-sizeof(struct about_fw_info),
>-_fw_dma, GFP_KERNEL);
>+  about_fw = dma_zalloc_coherent(>pdev->dev,
>+ sizeof(struct about_fw_info),
>+ _fw_dma, GFP_KERNEL);
>   if (!about_fw) {
>   DEBUG2(ql4_printk(KERN_ERR, ha, "%s: Unable to alloc memory "
> "for about_fw\n", __func__));
>   return status;
>   }
> 
>-  memset(about_fw, 0, sizeof(struct about_fw_info));
>   memset(_cmd, 0, sizeof(mbox_cmd));
>   memset(_sts, 0, sizeof(mbox_sts));
> 
>diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c
>index e91abb3..968bd85 100644
>--- a/drivers/scsi/qla4xxx/ql4_nx.c
>+++ b/drivers/scsi/qla4xxx/ql4_nx.c
>@@ -4050,15 +4050,14 @@ int qla4_8xxx_get_sys_info(struct scsi_qla_host
>*ha)
>   dma_addr_t sys_info_dma;
>   int status = QLA_ERROR;
> 
>-  sys_info = 

Re: [PATCH] qedi: Fix a possible sleep-in-atomic bug in qedi_process_tmf_resp

[Rangankar, Manish]

On 13/12/17 2:41 PM, "Jia-Ju Bai"  wrote:

>The driver may sleep under a spinlock.
>The function call path is:
>qedi_cpu_offline (acquire the spinlock)
>  qedi_fp_process_cqes
>qedi_mtask_completion
>  qedi_process_tmf_resp
>kzalloc(GFP_KERNEL) --> may sleep
>
>To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
>
>This bug is found by my static analysis tool(DSAC) and checked by my code
>review.
>
>Signed-off-by: Jia-Ju Bai 
>---
> drivers/scsi/qedi/qedi_fw.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>index bd302d3..20a9259 100644
>--- a/drivers/scsi/qedi/qedi_fw.c
>+++ b/drivers/scsi/qedi/qedi_fw.c
>@@ -198,7 +198,7 @@ static void qedi_process_tmf_resp(struct qedi_ctx
>*qedi,
>   cqe_tmp_response = >cqe_common.iscsi_hdr.tmf_response;
> 
>   qedi_cmd = task->dd_data;
>-  qedi_cmd->tmf_resp_buf = kzalloc(sizeof(*resp_hdr_ptr), GFP_KERNEL);
>+  qedi_cmd->tmf_resp_buf = kzalloc(sizeof(*resp_hdr_ptr), GFP_ATOMIC);
>   if (!qedi_cmd->tmf_resp_buf) {
>   QEDI_ERR(>dbg_ctx,
>"Failed to allocate resp buf, cid=0x%x\n",
>-- 
>1.7.9.5

Thanks,

Acked-by: Manish Rangankar 

>

Re: [PATCH] scsi: bnx2i: bnx2i_hwi: use swap macro in bnx2i_send_iscsi_nopout

[Rangankar, Manish]

On 04/11/17 1:28 AM, "Gustavo A. R. Silva"  wrote:

>Make use of the swap macro and remove unnecessary variable tmp.
>This makes the code easier to read and maintain.
>
>This code was detected with the help of Coccinelle.
>
>Signed-off-by: Gustavo A. R. Silva 
>---
> drivers/scsi/bnx2i/bnx2i_hwi.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c
>b/drivers/scsi/bnx2i/bnx2i_hwi.c
>index e0640e0..9e3bf53 100644
>--- a/drivers/scsi/bnx2i/bnx2i_hwi.c
>+++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
>@@ -547,12 +547,9 @@ int bnx2i_send_iscsi_nopout(struct bnx2i_conn
>*bnx2i_conn,
>   nopout_wqe->op_attr = ISCSI_FLAG_CMD_FINAL;
>   memcpy(nopout_wqe->lun, _hdr->lun, 8);
> 
>-  if (test_bit(BNX2I_NX2_DEV_57710, >hba->cnic_dev_type)) {
>-  u32 tmp = nopout_wqe->lun[0];
>-  /* 57710 requires LUN field to be swapped */
>-  nopout_wqe->lun[0] = nopout_wqe->lun[1];
>-  nopout_wqe->lun[1] = tmp;
>-  }
>+  /* 57710 requires LUN field to be swapped */
>+  if (test_bit(BNX2I_NX2_DEV_57710, >hba->cnic_dev_type))
>+  swap(nopout_wqe->lun[0], nopout_wqe->lun[1]);
> 
>   nopout_wqe->itt = ((u16)task->itt |
>  (ISCSI_TASK_TYPE_MPATH <<
>-- 
>2.7.4

Thanks,

Acked-by: Manish Rangankar 

>

Re: [PATCH] scsi: qla4xxx: Convert timers to use timer_setup()

[Rangankar, Manish]

On 25/10/17 3:37 PM, "Kees Cook"  wrote:

>In preparation for unconditionally passing the struct timer_list pointer
>to
>all timer callbacks, switch to using the new timer_setup() and
>from_timer()
>to pass the timer pointer explicitly.
>
>Cc: qlogic-storage-upstr...@qlogic.com
>Cc: "James E.J. Bottomley" 
>Cc: "Martin K. Petersen" 
>Cc: linux-scsi@vger.kernel.org
>Signed-off-by: Kees Cook 
>---
> drivers/scsi/qla4xxx/ql4_os.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
>index 64c6fa563fdb..2b8a8ce2a431 100644
>--- a/drivers/scsi/qla4xxx/ql4_os.c
>+++ b/drivers/scsi/qla4xxx/ql4_os.c
>@@ -3955,16 +3955,15 @@ void qla4xxx_update_session_conn_param(struct
>scsi_qla_host *ha,
> /*
>  * Timer routines
>  */
>+static void qla4xxx_timer(struct timer_list *t);
> 
>-static void qla4xxx_start_timer(struct scsi_qla_host *ha, void *func,
>+static void qla4xxx_start_timer(struct scsi_qla_host *ha,
>   unsigned long interval)
> {
>   DEBUG(printk("scsi: %s: Starting timer thread for adapter %d\n",
>__func__, ha->host->host_no));
>-  init_timer(>timer);
>+  timer_setup(>timer, qla4xxx_timer, 0);
>   ha->timer.expires = jiffies + interval * HZ;
>-  ha->timer.data = (unsigned long)ha;
>-  ha->timer.function = (void (*)(unsigned long))func;
>   add_timer(>timer);
>   ha->timer_active = 1;
> }
>@@ -4508,8 +4507,9 @@ static void qla4xxx_check_relogin_flash_ddb(struct
>iscsi_cls_session *cls_sess)
>  * qla4xxx_timer - checks every second for work to do.
>  * @ha: Pointer to host adapter structure.
>  **/
>-static void qla4xxx_timer(struct scsi_qla_host *ha)
>+static void qla4xxx_timer(struct timer_list *t)
> {
>+  struct scsi_qla_host *ha = from_timer(ha, t, timer);
>   int start_dpc = 0;
>   uint16_t w;
> 
>@@ -8805,7 +8805,7 @@ static int qla4xxx_probe_adapter(struct pci_dev
>*pdev,
>   ha->isp_ops->enable_intrs(ha);
> 
>   /* Start timer thread. */
>-  qla4xxx_start_timer(ha, qla4xxx_timer, 1);
>+  qla4xxx_start_timer(ha, 1);
> 
>   set_bit(AF_INIT_DONE, >flags);
> 
>-- 
>2.7.4


Acked-by: Manish Rangankar 

Re: [PATCH] scsi: qedi: Delete redundant variables

[Rangankar, Manish]

On 14/10/17 5:47 PM, "Christos Gkekas"  wrote:

>Remove redundant variables in quedi_fw.c that are set but not used.
>
>Signed-off-by: Christos Gkekas 
>---
> drivers/scsi/qedi/qedi_fw.c | 17 +
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>index 93d54ac..bd302d3 100644
>--- a/drivers/scsi/qedi/qedi_fw.c
>+++ b/drivers/scsi/qedi/qedi_fw.c
>@@ -92,7 +92,6 @@ static void qedi_process_text_resp(struct qedi_ctx
>*qedi,
>   struct iscsi_text_response_hdr *cqe_text_response;
>   struct qedi_cmd *cmd;
>   int pld_len;
>-  u32 *tmp;
> 
>   cmd = (struct qedi_cmd *)task->dd_data;
>   task_ctx = qedi_get_task_mem(>tasks, cmd->task_id);
>@@ -108,7 +107,6 @@ static void qedi_process_text_resp(struct qedi_ctx
>*qedi,
>   hton24(resp_hdr_ptr->dlength,
>  (cqe_text_response->hdr_second_dword &
>   ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK));
>-  tmp = (u32 *)resp_hdr_ptr->dlength;
> 
>   resp_hdr_ptr->itt = build_itt(cqe->cqe_solicited.itid,
> conn->session->age);
>@@ -196,7 +194,6 @@ static void qedi_process_tmf_resp(struct qedi_ctx
>*qedi,
>   struct iscsi_tm_rsp *resp_hdr_ptr;
>   struct iscsi_tm *tmf_hdr;
>   struct qedi_cmd *qedi_cmd = NULL;
>-  u32 *tmp;
> 
>   cqe_tmp_response = >cqe_common.iscsi_hdr.tmf_response;
> 
>@@ -222,7 +219,6 @@ static void qedi_process_tmf_resp(struct qedi_ctx
>*qedi,
>   hton24(resp_hdr_ptr->dlength,
>  (cqe_tmp_response->hdr_second_dword &
>   ISCSI_TMF_RESPONSE_HDR_DATA_SEG_LEN_MASK));
>-  tmp = (u32 *)resp_hdr_ptr->dlength;
>   resp_hdr_ptr->itt = build_itt(cqe->cqe_solicited.itid,
> conn->session->age);
>   resp_hdr_ptr->statsn = cpu_to_be32(cqe_tmp_response->stat_sn);
>@@ -269,7 +265,6 @@ static void qedi_process_login_resp(struct qedi_ctx
>*qedi,
>   struct iscsi_login_response_hdr *cqe_login_response;
>   struct qedi_cmd *cmd;
>   int pld_len;
>-  u32 *tmp;
> 
>   cmd = (struct qedi_cmd *)task->dd_data;
> 
>@@ -286,7 +281,6 @@ static void qedi_process_login_resp(struct qedi_ctx
>*qedi,
>   hton24(resp_hdr_ptr->dlength,
>  (cqe_login_response->hdr_second_dword &
>   ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK));
>-  tmp = (u32 *)resp_hdr_ptr->dlength;
>   resp_hdr_ptr->itt = build_itt(cqe->cqe_solicited.itid,
> conn->session->age);
>   resp_hdr_ptr->tsih = cqe_login_response->tsih;
>@@ -590,7 +584,6 @@ static void qedi_scsi_completion(struct qedi_ctx
>*qedi,
>   int datalen = 0;
>   struct qedi_conn *qedi_conn;
>   u32 iscsi_cid;
>-  bool mark_cmd_node_deleted = false;
>   u8 cqe_err_bits = 0;
> 
>   iscsi_cid  = cqe->cqe_common.conn_id;
>@@ -674,7 +667,6 @@ static void qedi_scsi_completion(struct qedi_ctx
>*qedi,
>   cmd->io_cmd_in_list = false;
>   list_del_init(>io_cmd);
>   qedi_conn->active_cmd_count--;
>-  mark_cmd_node_deleted = true;
>   }
>   spin_unlock(_conn->list_lock);
> 
>@@ -763,7 +755,7 @@ static void qedi_process_cmd_cleanup_resp(struct
>qedi_ctx *qedi,
>   u32 rtid = 0;
>   u32 iscsi_cid;
>   struct qedi_conn *qedi_conn;
>-  struct qedi_cmd *cmd_new, *dbg_cmd;
>+  struct qedi_cmd *dbg_cmd;
>   struct iscsi_task *mtask;
>   struct iscsi_tm *tmf_hdr = NULL;
> 
>@@ -856,7 +848,6 @@ static void qedi_process_cmd_cleanup_resp(struct
>qedi_ctx *qedi,
>   }
>   qedi_conn->cmd_cleanup_cmpl++;
>   wake_up(_conn->wait_queue);
>-  cmd_new = task->dd_data;
> 
>   QEDI_INFO(>dbg_ctx, QEDI_LOG_TID,
> "Freeing tid=0x%x for cid=0x%x\n",
>@@ -1029,7 +1020,6 @@ int qedi_send_iscsi_login(struct qedi_conn
>*qedi_conn,
>   struct iscsi_task_context *fw_task_ctx;
>   struct qedi_ctx *qedi = qedi_conn->qedi;
>   struct iscsi_login_req *login_hdr;
>-  struct scsi_sge *req_sge = NULL;
>   struct scsi_sge *resp_sge = NULL;
>   struct qedi_cmd *qedi_cmd;
>   struct qedi_endpoint *ep;
>@@ -1037,7 +1027,6 @@ int qedi_send_iscsi_login(struct qedi_conn
>*qedi_conn,
>   u16 sq_idx = 0;
>   int rval = 0;
> 
>-  req_sge = (struct scsi_sge *)qedi_conn->gen_pdu.req_bd_tbl;
>   resp_sge = (struct scsi_sge *)qedi_conn->gen_pdu.resp_bd_tbl;
>   qedi_cmd = (struct qedi_cmd *)task->dd_data;
>   ep = qedi_conn->ep;
>@@ -1718,7 +1707,6 @@ int qedi_send_iscsi_nopout(struct qedi_conn
>*qedi_conn,
>   struct qedi_ctx *qedi = qedi_conn->qedi;
>   struct iscsi_task_context *fw_task_ctx;
>   struct iscsi_nopout *nopout_hdr;
>-  struct scsi_sge *req_sge = NULL;
>   struct scsi_sge *resp_sge = NULL;
>   struct qedi_cmd *qedi_cmd;

Re: [PATCH] bnx2i: Clean up unused pointers in bnx2i_hwi

[Rangankar, Manish]

On 10/09/17 5:48 PM, "Christos Gkekas"  wrote:

>Pointers bnx2i_cmd are set but never used, so they can be removed.
>
>Signed-off-by: Christos Gkekas 
>---
> drivers/scsi/bnx2i/bnx2i_hwi.c | 10 --
> 1 file changed, 10 deletions(-)
>
>diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c
>b/drivers/scsi/bnx2i/bnx2i_hwi.c
>index 42921db..e3f22cb 100644
>--- a/drivers/scsi/bnx2i/bnx2i_hwi.c
>+++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
>@@ -332,12 +332,10 @@ static void
>bnx2i_ring_dbell_update_sq_params(struct bnx2i_conn *bnx2i_conn,
> int bnx2i_send_iscsi_login(struct bnx2i_conn *bnx2i_conn,
>  struct iscsi_task *task)
> {
>-  struct bnx2i_cmd *bnx2i_cmd;
>   struct bnx2i_login_request *login_wqe;
>   struct iscsi_login_req *login_hdr;
>   u32 dword;
> 
>-  bnx2i_cmd = (struct bnx2i_cmd *)task->dd_data;
>   login_hdr = (struct iscsi_login_req *)task->hdr;
>   login_wqe = (struct bnx2i_login_request *)
>   bnx2i_conn->ep->qp.sq_prod_qe;
>@@ -391,12 +389,10 @@ int bnx2i_send_iscsi_tmf(struct bnx2i_conn
>*bnx2i_conn,
>   struct iscsi_tm *tmfabort_hdr;
>   struct scsi_cmnd *ref_sc;
>   struct iscsi_task *ctask;
>-  struct bnx2i_cmd *bnx2i_cmd;
>   struct bnx2i_tmf_request *tmfabort_wqe;
>   u32 dword;
>   u32 scsi_lun[2];
> 
>-  bnx2i_cmd = (struct bnx2i_cmd *)mtask->dd_data;
>   tmfabort_hdr = (struct iscsi_tm *)mtask->hdr;
>   tmfabort_wqe = (struct bnx2i_tmf_request *)
>   bnx2i_conn->ep->qp.sq_prod_qe;
>@@ -463,12 +459,10 @@ int bnx2i_send_iscsi_tmf(struct bnx2i_conn
>*bnx2i_conn,
> int bnx2i_send_iscsi_text(struct bnx2i_conn *bnx2i_conn,
> struct iscsi_task *mtask)
> {
>-  struct bnx2i_cmd *bnx2i_cmd;
>   struct bnx2i_text_request *text_wqe;
>   struct iscsi_text *text_hdr;
>   u32 dword;
> 
>-  bnx2i_cmd = (struct bnx2i_cmd *)mtask->dd_data;
>   text_hdr = (struct iscsi_text *)mtask->hdr;
>   text_wqe = (struct bnx2i_text_request *) bnx2i_conn->ep->qp.sq_prod_qe;
> 
>@@ -541,11 +535,9 @@ int bnx2i_send_iscsi_nopout(struct bnx2i_conn
>*bnx2i_conn,
>   char *datap, int data_len, int unsol)
> {
>   struct bnx2i_endpoint *ep = bnx2i_conn->ep;
>-  struct bnx2i_cmd *bnx2i_cmd;
>   struct bnx2i_nop_out_request *nopout_wqe;
>   struct iscsi_nopout *nopout_hdr;
> 
>-  bnx2i_cmd = (struct bnx2i_cmd *)task->dd_data;
>   nopout_hdr = (struct iscsi_nopout *)task->hdr;
>   nopout_wqe = (struct bnx2i_nop_out_request *)ep->qp.sq_prod_qe;
> 
>@@ -602,11 +594,9 @@ int bnx2i_send_iscsi_nopout(struct bnx2i_conn
>*bnx2i_conn,
> int bnx2i_send_iscsi_logout(struct bnx2i_conn *bnx2i_conn,
>   struct iscsi_task *task)
> {
>-  struct bnx2i_cmd *bnx2i_cmd;
>   struct bnx2i_logout_request *logout_wqe;
>   struct iscsi_logout *logout_hdr;
> 
>-  bnx2i_cmd = (struct bnx2i_cmd *)task->dd_data;
>   logout_hdr = (struct iscsi_logout *)task->hdr;
> 
>   logout_wqe = (struct bnx2i_logout_request *)
>-- 
>2.7.4

Thanks,

Acked-by: Manish Rangankar 

Re: [PATCH] scsi: qedi: off by one in qedi_get_cmd_from_tid()

[Rangankar, Manish]

On 25/08/17 4:06 PM, "Dan Carpenter"  wrote:

>The > here should be >= or we end up reading one element beyond the end
>of the qedi->itt_map[] array.  The qedi->itt_map[] array is allocated in
>qedi_alloc_itt().
>
>Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI
>driver framework.")
>Signed-off-by: Dan Carpenter 
>
>diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
>index c4a470bab4dd..34adc0e0 100644
>--- a/drivers/scsi/qedi/qedi_main.c
>+++ b/drivers/scsi/qedi/qedi_main.c
>@@ -1576,7 +1576,7 @@ struct qedi_cmd *qedi_get_cmd_from_tid(struct
>qedi_ctx *qedi, u32 tid)
> {
>   struct qedi_cmd *cmd = NULL;
> 
>-  if (tid > MAX_ISCSI_TASK_ENTRIES)
>+  if (tid >= MAX_ISCSI_TASK_ENTRIES)
>   return NULL;
> 
>   cmd = qedi->itt_map[tid].p_cmd;

Thanks for the patch.

Acked-by: Manish Rangankar 


>

Re: [PATCH] scsi: qedi: fix another spelling mistake: "alloction" -> "allocation"

[Rangankar, Manish]

On 03/07/17 3:54 PM, "Colin King"  wrote:

>From: Colin Ian King 
>
>Trivial fix to spelling mistake in QEDF_ERR message.  I should have
>also included this in a previous fix, but I only just spotted this one.
>
>Signed-off-by: Colin Ian King 
>---
> drivers/scsi/qedi/qedi_fw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>index 19254bd739d9..93d54acd4a22 100644
>--- a/drivers/scsi/qedi/qedi_fw.c
>+++ b/drivers/scsi/qedi/qedi_fw.c
>@@ -1411,7 +1411,7 @@ static void qedi_tmf_work(struct work_struct *work)
> 
>   list_work = kzalloc(sizeof(*list_work), GFP_ATOMIC);
>   if (!list_work) {
>-  QEDI_ERR(>dbg_ctx, "Memory alloction failed\n");
>+  QEDI_ERR(>dbg_ctx, "Memory allocation failed\n");
>   goto abort_ret;
>   }
> 
>-- 
>2.11.0
>
Thanks

Acked-by: Manish Rangankar 

Re: [PATCH] scsi: qedi: Remove comparison of u16 idx with zero.

[Rangankar, Manish]

On 24/06/17 9:54 PM, "Christos Gkekas"  wrote:

>Variable idx is defined as u16 thus statement (idx < 0) is
>always false and should be removed.
>
>Signed-off-by: Christos Gkekas 
>---
> drivers/scsi/qedi/qedi_fw.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>index e937490..19254bd 100644
>--- a/drivers/scsi/qedi/qedi_fw.c
>+++ b/drivers/scsi/qedi/qedi_fw.c
>@@ -333,7 +333,7 @@ static void qedi_get_rq_bdq_buf(struct qedi_ctx *qedi,
> 
>   /* Obtain buffer address from rqe_opaque */
>   idx = cqe->rqe_opaque.lo;
>-  if ((idx < 0) || (idx > (QEDI_BDQ_NUM - 1))) {
>+  if (idx > (QEDI_BDQ_NUM - 1)) {
>   QEDI_INFO(>dbg_ctx, QEDI_LOG_CONN,
> "wrong idx %d returned by FW, dropping the 
> unsolicited pkt\n",
> idx);
>@@ -370,7 +370,7 @@ static void qedi_put_rq_bdq_buf(struct qedi_ctx *qedi,
> 
>   /* Obtain buffer address from rqe_opaque */
>   idx = cqe->rqe_opaque.lo;
>-  if ((idx < 0) || (idx > (QEDI_BDQ_NUM - 1))) {
>+  if (idx > (QEDI_BDQ_NUM - 1)) {
>   QEDI_INFO(>dbg_ctx, QEDI_LOG_CONN,
> "wrong idx %d returned by FW, dropping the 
> unsolicited pkt\n",
> idx);
>-- 
>2.7.4

Acked-by: Manish Rangankar 

Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.

[Rangankar, Manish]

On 13/06/17 10:19 PM, "Robert LeBlanc"  wrote:

>On Wed, Jun 7, 2017 at 12:30 PM, Robert LeBlanc 
>wrote:
>> On Wed, Jun 7, 2017 at 10:28 AM, Chris Leech  wrote:
>>> On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote:
 This patchset enables iSCSI offload drivers to have access to the
iface
 information provided by iscsid. This allows users to have more control
 of how the driver connects to the iSCSI target. iSER is updated to use
 iface.ipaddress to set the source IP address if configured. This
allows
 iSER to use multiple ports on the same network or in more complicated
 routed configurations.

 Since there is already a change to the function parameters, dst_addr
 is upgraded to sockaddr_storage so that it is more future proof and
makes
 the size of the struct static and not dependent on checking the
SA_FAMILY.

 This is dependent on updates to Open-iSCSI.
>>>
>>> Hi Robert,
>>>
>>> I don't think that passing the iface_rec structure directly from the
>>> iscsid internals into a netlink message is a good way to go about this.
>>> It's really big, there's an embedded list_head with user address
>>> pointers that needs to be left out, and there are 32/64-bit layout
>>> differences.
>>>
>>> Let me take a look at how you're proposing using this info for iSER, if
>>> it makes sense I think we should come up with a better designed
>>> structure for passing the information.
>>>
>>> Thanks,
>>> Chris
>>>
>>
>> Chris,
>>
>> Thank you for your feedback. I agree that the entire iface is probably
>> overkill, it was more of a proof of concept. We are only using the
>> ipaddress in the iface for iSER (in my patch), but I could see other
>> drivers benefiting from some of the other data in the iface (mac,
>> interface_name, vlan, etc) so I didn't want to be too restrictive so
>> that it wouldn't have to be extended later. I've not worked on
>> userspace/kernel interaction before so I need some guidance to make
>> the transition between userspace and kernel versions smoother.
>>
>> This patchset works for what we need and it is very important for us
>> (and I'm sure others once the feature is available) and I'm happy to
>> put in the time to get it accepted upstream, I'm just new to kernel
>> development and need some guidance.
>
>Are there other comments/ideas/suggestions specifically from the
>iSCSI/iSER guys? I'd like to keep this patch moving.

Considering partial iSCSI offload solution (like bnx2i and qedi) point of
view, we liked the idea from Hannes to create TAP interface to associate
with each iSCSI offload interface, which will allow us to use userspace
tools for configuration. I haven't dig into its details yet, but at higher
level it looks like this will help us to move away from our dependency
over iscsiuio. 


Thanks,
Manish R.

Re: [PATCH 1/5] scsi: bnx2i: convert to workqueue

[Rangankar, Manish]

On 10/04/17 10:42 PM, "Sebastian Andrzej Siewior" 
wrote:

>The driver creates its own per-CPU threads which are updated based on CPU
>hotplug events. It is also possible to use kworkers and remove some of the
>infrastructure get the same job done while saving a few lines of code.
>
>The DECLARE_PER_CPU() definition is moved into the header file where it
>belongs. bnx2i_percpu_io_thread() becomes bnx2i_percpu_io_work() which is
>mostly the same code. The outer loop (kthread_should_stop()) gets removed
>and
>the remaining code is shifted to the left.
>bnx2i_queue_scsi_cmd_resp() is mostly the same. The code checked
>->iothread to
>decide if there is an active per-CPU thread. With the kworkers this is no
>longer possible nor required.
>The allocation of struct bnx2i_work does not happen with ->p_work_lock
>held
>which is not required. I am unsure about the call-stack so I can't say
>if this qualifies it for the allocation with GFP_KERNEL instead of
>GFP_ATOMIC (it is not _bh lock but as I said, I don't know the context).
>The allocation case has been reversed so the inner if case is called on
>!bnx2i_work and is just the invocation one function since the lock is not
>held during allocation. The init of the new bnx2i_work struct is now
>done also without the ->p_work_lock held: it is a new object, nobody
>knows about it yet. It should be enough to hold the lock while adding
>this item to the list. I am unsure about that atomic_inc() so I keep
>things as they were.
>
>The remaining part is the removal CPU hotplug notifier since it is taken
>care by the workqueue code.
>
>This patch was only compile-tested due to -ENODEV.
>
>Cc: qlogic-storage-upstr...@qlogic.com
>Cc: Christoph Hellwig 
>Signed-off-by: Sebastian Andrzej Siewior 
>---

Didn't seen any issue with regression testing. Thanks Sebastian.

Acked-by: Manish Rangankar 

Re: [PATCH] qla4xxx: fix spelling mistake: "Tempalate" -> "Template"

[Rangankar, Manish]

On 26/04/17 3:07 AM, "Colin King"  wrote:

>From: Colin Ian King 
>
>trivial fix to spelling mistake in DEBUG2 debug message
>
>Signed-off-by: Colin Ian King 
>---
> drivers/scsi/qla4xxx/ql4_init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/qla4xxx/ql4_init.c
>b/drivers/scsi/qla4xxx/ql4_init.c
>index 4180d6d9fe78..5d6d158bbfd6 100644
>--- a/drivers/scsi/qla4xxx/ql4_init.c
>+++ b/drivers/scsi/qla4xxx/ql4_init.c
>@@ -389,7 +389,7 @@ void qla4xxx_alloc_fw_dump(struct scsi_qla_host *ha)
>   goto alloc_cleanup;
> 
>   DEBUG2(ql4_printk(KERN_INFO, ha,
>-"Minidump Tempalate Size = 0x%x KB\n",
>+"Minidump Template Size = 0x%x KB\n",
> ha->fw_dump_tmplt_size));
>   DEBUG2(ql4_printk(KERN_INFO, ha,
> "Total Minidump size = 0x%x KB\n", ha->fw_dump_size));
>-- 
>2.11.0

Acked-by: Manish Rangankar 

Re: [PATCH] [v2] scsi: qedi: fix build error without DEBUG_FS

[Rangankar, Manish]

On 02/03/17 8:28 PM, "Arnd Bergmann"  wrote:

>Without CONFIG_DEBUG_FS, we run into a link error:
>
>drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_poll':
>qedi_iscsi.c:(.text.qedi_ep_poll+0x134): undefined reference to
>`do_not_recover'
>drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_disconnect':
>qedi_iscsi.c:(.text.qedi_ep_disconnect+0x36c): undefined reference to
>`do_not_recover'
>drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_connect':
>qedi_iscsi.c:(.text.qedi_ep_connect+0x350): undefined reference to
>`do_not_recover'
>drivers/scsi/qedi/qedi_fw.o: In function `qedi_tmf_work':
>qedi_fw.c:(.text.qedi_tmf_work+0x3b4): undefined reference to
>`do_not_recover'
>
>This defines the symbol as a constant in this case, as there is no way to
>set it to anything other than zero without DEBUG_FS. In addition, I'm
>renaming
>it to qedi_do_not_recover in order to put it into a driver specific
>namespace,
>as "do_not_recover" is a really bad name for a kernel-wide global
>identifier
>when it is used only in one driver.
>
>Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI
>driver framework.")
>Reviewed-by: Johannes Thumshirn 
>Signed-off-by: Arnd Bergmann 
>---
>v2: don't rename references to do_not_recover in string literals
>---
> drivers/scsi/qedi/qedi_debugfs.c | 16 
> drivers/scsi/qedi/qedi_fw.c  |  4 ++--
> drivers/scsi/qedi/qedi_gbl.h |  8 +++-
> drivers/scsi/qedi/qedi_iscsi.c   |  8 
> 4 files changed, 21 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_debugfs.c
>b/drivers/scsi/qedi/qedi_debugfs.c
>index 955936274241..59417199bf36 100644
>--- a/drivers/scsi/qedi/qedi_debugfs.c
>+++ b/drivers/scsi/qedi/qedi_debugfs.c
>@@ -14,7 +14,7 @@
> #include 
> #include 
> 
>-int do_not_recover;
>+int qedi_do_not_recover;
> static struct dentry *qedi_dbg_root;
> 
> void
>@@ -74,22 +74,22 @@ qedi_dbg_exit(void)
> static ssize_t
> qedi_dbg_do_not_recover_enable(struct qedi_dbg_ctx *qedi_dbg)
> {
>-  if (!do_not_recover)
>-  do_not_recover = 1;
>+  if (!qedi_do_not_recover)
>+  qedi_do_not_recover = 1;
> 
>   QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
>-do_not_recover);
>+qedi_do_not_recover);
>   return 0;
> }
> 
> static ssize_t
> qedi_dbg_do_not_recover_disable(struct qedi_dbg_ctx *qedi_dbg)
> {
>-  if (do_not_recover)
>-  do_not_recover = 0;
>+  if (qedi_do_not_recover)
>+  qedi_do_not_recover = 0;
> 
>   QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
>-do_not_recover);
>+qedi_do_not_recover);
>   return 0;
> }
> 
>@@ -141,7 +141,7 @@ qedi_dbg_do_not_recover_cmd_read(struct file *filp,
>char __user *buffer,
>   if (*ppos)
>   return 0;
> 
>-  cnt = sprintf(buffer, "do_not_recover=%d\n", do_not_recover);
>+  cnt = sprintf(buffer, "do_not_recover=%d\n", qedi_do_not_recover);
>   cnt = min_t(int, count, cnt - *ppos);
>   *ppos += cnt;
>   return cnt;
>diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>index c9f0ef4e11b3..2bce3efc66a4 100644
>--- a/drivers/scsi/qedi/qedi_fw.c
>+++ b/drivers/scsi/qedi/qedi_fw.c
>@@ -1461,9 +1461,9 @@ static void qedi_tmf_work(struct work_struct *work)
> get_itt(tmf_hdr->rtt), get_itt(ctask->itt), cmd->task_id,
> qedi_conn->iscsi_conn_id);
> 
>-  if (do_not_recover) {
>+  if (qedi_do_not_recover) {
>   QEDI_ERR(>dbg_ctx, "DONT SEND CLEANUP/ABORT %d\n",
>-   do_not_recover);
>+   qedi_do_not_recover);
>   goto abort_ret;
>   }
> 
>diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h
>index 8e488de88ece..63d793f46064 100644
>--- a/drivers/scsi/qedi/qedi_gbl.h
>+++ b/drivers/scsi/qedi/qedi_gbl.h
>@@ -12,8 +12,14 @@
> 
> #include "qedi_iscsi.h"
> 
>+#ifdef CONFIG_DEBUG_FS
>+extern int qedi_do_not_recover;
>+#else
>+#define qedi_do_not_recover (0)
>+#endif
>+
> extern uint qedi_io_tracing;
>-extern int do_not_recover;
>+
> extern struct scsi_host_template qedi_host_template;
> extern struct iscsi_transport qedi_iscsi_transport;
> extern const struct qed_iscsi_ops *qedi_ops;
>diff --git a/drivers/scsi/qedi/qedi_iscsi.c
>b/drivers/scsi/qedi/qedi_iscsi.c
>index b9f79d36142d..4cc474364c50 100644
>--- a/drivers/scsi/qedi/qedi_iscsi.c
>+++ b/drivers/scsi/qedi/qedi_iscsi.c
>@@ -833,7 +833,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct
>sockaddr *dst_addr,
>   return ERR_PTR(ret);
>   }
> 
>-  if (do_not_recover) {
>+  if (qedi_do_not_recover) {
>   ret = -ENOMEM;
>   return ERR_PTR(ret);
>   }
>@@ -957,7 +957,7 @@ static int qedi_ep_poll(struct iscsi_endpoint *ep,
>int timeout_ms)
>   struct qedi_endpoint *qedi_ep;
>   int ret = 0;
> 
>- 

Re: [PATCH] scsi: qedi: fix missing return error code check on call to qedi_setup_int

[Rangankar, Manish]

On 28/02/17 4:32 PM, "Colin King"  wrote:

>From: Colin Ian King 
>
>The call to qedi_setup_int is not updating the return code rc yet rc
>is being checked for an error. Fix this by assigning rc to the return
>code from the call to qedi_setup_int.
>
>Signed-off-by: Colin Ian King 
>---
> drivers/scsi/qedi/qedi_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
>index 5eda21d..8e3d928 100644
>--- a/drivers/scsi/qedi/qedi_main.c
>+++ b/drivers/scsi/qedi/qedi_main.c
>@@ -1805,7 +1805,7 @@ static int __qedi_probe(struct pci_dev *pdev, int
>mode)
>*/
>   qedi_ops->common->update_pf_params(qedi->cdev, >pf_params);
> 
>-  qedi_setup_int(qedi);
>+  rc = qedi_setup_int(qedi);
>   if (rc)
>   goto stop_iscsi_func;
> 
>-- 
>2.10.2
>

Acked-by: Manish Rangankar 

Re: [PATCH -next] scsi: qedi: Fix possible memory leak in qedi_iscsi_update_conn()

[Rangankar, Manish]

On 07/02/17 8:22 PM, "Wei Yongjun"  wrote:

>From: Wei Yongjun 
>
>'conn_info' is malloced in qedi_iscsi_update_conn() and should be
>freed before leaving from the error handling cases, otherwise it
>will cause memory leak.
>
>Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI
>driver framework.")
>Signed-off-by: Wei Yongjun 
>---
> drivers/scsi/qedi/qedi_iscsi.c | 4 
> 1 file changed, 4 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_iscsi.c
>b/drivers/scsi/qedi/qedi_iscsi.c
>index d6a2054..eb64469 100644
>--- a/drivers/scsi/qedi/qedi_iscsi.c
>+++ b/drivers/scsi/qedi/qedi_iscsi.c
>@@ -453,13 +453,9 @@ static int qedi_iscsi_update_conn(struct qedi_ctx
>*qedi,
>   if (rval) {
>   rval = -ENXIO;
>   QEDI_ERR(>dbg_ctx, "Could not update connection\n");
>-  goto update_conn_err;
>   }
> 
>   kfree(conn_info);
>-  rval = 0;
>-
>-update_conn_err:
>   return rval;
> }
>

Acked-by: Manish Rangankar 

Re: [PATCH] scsi: qedi: select UIO

[Rangankar, Manish]

On 11/01/17 9:40 AM, "Martin K. Petersen" 
wrote:

>> "Ewan" == Ewan D Milne  writes:
>
>Ewan> Randy posted a similar patch back in December but I don't think
>Ewan> there was ever a reply to Christoph's question about why qedi
>Ewan> depends on uio.
>
>I did queue up Randy's patch to shut up the build warnings. But we're
>still looking for a long term fix or an explanation as to why UIO is
>needed in the first place.

Similar to bnx2i driver, qedi driver also has a dependency over iscsiuio
to provide ARP and DHCP functionality for iscsi offload, and the
communication to the
driver is done via uio interface.

https://github.com/open-iscsi/open-iscsi/blob/master/iscsiuio/README


Thanks,
Manish

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: qedi: return via va_end to match corresponding va_start

[Rangankar, Manish]

On 16/12/16 7:40 PM, "Colin King"  wrote:

>From: Colin Ian King 
>
>Although on most systems va_end is a no-op, it is good practice
>to use va_end on the function return path, especially since the
>va_start documenation states:
>
>  "Each invocation of va_start() must be matched by a corresponding
>   invocation of va_end() in the same function."
>
>Found with static analysis by CoverityScan, CIDs 1389477-1389479
>
>Signed-off-by: Colin Ian King 
>---
> drivers/scsi/qedi/qedi_dbg.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_dbg.c b/drivers/scsi/qedi/qedi_dbg.c
>index 2bdedb9..8fd28b0 100644
>--- a/drivers/scsi/qedi/qedi_dbg.c
>+++ b/drivers/scsi/qedi/qedi_dbg.c
>@@ -52,7 +52,7 @@ qedi_dbg_warn(struct qedi_dbg_ctx *qedi, const char
>*func, u32 line,
>   vaf.va = 
> 
>   if (!(qedi_dbg_log & QEDI_LOG_WARN))
>-  return;
>+  goto ret;
> 
>   if (likely(qedi) && likely(qedi->pdev))
>   pr_warn("[%s]:[%s:%d]:%d: %pV", dev_name(>pdev->dev),
>@@ -60,6 +60,7 @@ qedi_dbg_warn(struct qedi_dbg_ctx *qedi, const char
>*func, u32 line,
>   else
>   pr_warn("[:00:00.0]:[%s:%d]: %pV", nfunc, line, );
> 
>+ret:
>   va_end(va);
> }
> 
>@@ -80,7 +81,7 @@ qedi_dbg_notice(struct qedi_dbg_ctx *qedi, const char
>*func, u32 line,
>   vaf.va = 
> 
>   if (!(qedi_dbg_log & QEDI_LOG_NOTICE))
>-  return;
>+  goto ret;
> 
>   if (likely(qedi) && likely(qedi->pdev))
>   pr_notice("[%s]:[%s:%d]:%d: %pV",
>@@ -89,6 +90,7 @@ qedi_dbg_notice(struct qedi_dbg_ctx *qedi, const char
>*func, u32 line,
>   else
>   pr_notice("[:00:00.0]:[%s:%d]: %pV", nfunc, line, );
> 
>+ret:
>   va_end(va);
> }
> 
>@@ -109,7 +111,7 @@ qedi_dbg_info(struct qedi_dbg_ctx *qedi, const char
>*func, u32 line,
>   vaf.va = 
> 
>   if (!(qedi_dbg_log & level))
>-  return;
>+  goto ret;
> 
>   if (likely(qedi) && likely(qedi->pdev))
>   pr_info("[%s]:[%s:%d]:%d: %pV", dev_name(>pdev->dev),
>@@ -117,6 +119,7 @@ qedi_dbg_info(struct qedi_dbg_ctx *qedi, const char
>*func, u32 line,
>   else
>   pr_info("[:00:00.0]:[%s:%d]: %pV", nfunc, line, );
> 
>+ret:
>   va_end(va);
> }
> 
>-- 
>2.10.2


Acked-by: Manish Rangankar 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/3] Add QLogic FastLinQ iSCSI (qedi) driver.

[Rangankar, Manish]

Hi Dave,

Please consider applying the qed patches 1 & 2 to net-next.

Patch 3, qedi, goes to SCSI. Martin P. is ok to have you take qedi
(already Acked) through your tree. Please let us know which way you
prefer.


Thanks,
Manish

On 01/12/16 1:51 PM, "Rangankar, Manish" <manish.rangan...@cavium.com>
wrote:

>This series introduces hardware offload iSCSI initiator driver for the
>41000 Series Converged Network Adapters (579xx chip) by Qlogic. The
>overall
>driver design includes a common module ('qed') and protocol specific
>dependent modules ('qedi' for iSCSI).
>
>This is an open iSCSI driver, modifications to open iSCSI user components
>'iscsid', 'iscsiuio', etc. are required for the solution to work. The user
>space changes are also in the process of being submitted.
>
>https://groups.google.com/forum/#!forum/open-iscsi
>
>The 'qed' common module, under drivers/net/ethernet/qlogic/qed/, is
>enhanced with functionality required for the iSCSI support. This series
>is based on:
>
>net tree base: Merge of net and net-next as of 11/29/2016
>
>Changes from RFC v2:
>
>  1. qedi patches are squashed into single patch to prevent krobot
> warning.
>  2. Fixed 'hw_p_cpuq' incompatible pointer type.
>  3. Fixed sparse incompatible types in comparison expression.
>  4. Misc fixes with latest 'checkpatch --strict' option.
>  5. Remove int_mode option from MODULE_PARAM.
>  6. Prefix all MODULE_PARAM params with qedi_*.
>  7. Use CONFIG_QED_ISCSI instead of CONFIG_QEDI
>  8. Added bad task mem access fix.
>
>Manish Rangankar (1):
>  qedi: Add QLogic FastLinQ offload iSCSI driver framework.
>
>Yuval Mintz (2):
>  qed: Add support for hardware offloaded iSCSI.
>  qed: Add iSCSI out of order packet handling.
>
> MAINTAINERS|6 +
> drivers/net/ethernet/qlogic/Kconfig|3 +
> drivers/net/ethernet/qlogic/qed/Makefile   |1 +
> drivers/net/ethernet/qlogic/qed/qed.h  |8 +-
> drivers/net/ethernet/qlogic/qed/qed_dev.c  |   22 +
> drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 1277 +
> drivers/net/ethernet/qlogic/qed/qed_iscsi.h|   52 +
> drivers/net/ethernet/qlogic/qed/qed_ll2.c  |  555 +-
> drivers/net/ethernet/qlogic/qed/qed_ll2.h  |9 +
> drivers/net/ethernet/qlogic/qed/qed_ooo.c  |  501 +
> drivers/net/ethernet/qlogic/qed/qed_ooo.h  |  176 ++
> drivers/net/ethernet/qlogic/qed/qed_reg_addr.h |2 +
> drivers/net/ethernet/qlogic/qed/qed_roce.c |1 +
> drivers/net/ethernet/qlogic/qed/qed_spq.c  |   24 +
> drivers/scsi/Kconfig   |1 +
> drivers/scsi/Makefile  |1 +
> drivers/scsi/qedi/Kconfig  |   10 +
> drivers/scsi/qedi/Makefile |5 +
> drivers/scsi/qedi/qedi.h   |  364 
> drivers/scsi/qedi/qedi_dbg.c   |  143 ++
> drivers/scsi/qedi/qedi_dbg.h   |  144 ++
> drivers/scsi/qedi/qedi_debugfs.c   |  244 +++
> drivers/scsi/qedi/qedi_fw.c| 2378
>
> drivers/scsi/qedi/qedi_gbl.h   |   73 +
> drivers/scsi/qedi/qedi_hsi.h   |   52 +
> drivers/scsi/qedi/qedi_iscsi.c | 1624 
> drivers/scsi/qedi/qedi_iscsi.h |  232 +++
> drivers/scsi/qedi/qedi_main.c  | 2127
>+
> drivers/scsi/qedi/qedi_sysfs.c |   52 +
> drivers/scsi/qedi/qedi_version.h   |   14 +
> include/linux/qed/qed_if.h |2 +
> include/linux/qed/qed_iscsi_if.h   |  229 +++
> 32 files changed, 10303 insertions(+), 29 deletions(-)
> create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.h
> create mode 100644 drivers/net/ethernet/qlogic/qed/qed_ooo.c
> create mode 100644 drivers/net/ethernet/qlogic/qed/qed_ooo.h
> create mode 100644 drivers/scsi/qedi/Kconfig
> create mode 100644 drivers/scsi/qedi/Makefile
> create mode 100644 drivers/scsi/qedi/qedi.h
> create mode 100644 drivers/scsi/qedi/qedi_dbg.c
> create mode 100644 drivers/scsi/qedi/qedi_dbg.h
> create mode 100644 drivers/scsi/qedi/qedi_debugfs.c
> create mode 100644 drivers/scsi/qedi/qedi_fw.c
> create mode 100644 drivers/scsi/qedi/qedi_gbl.h
> create mode 100644 drivers/scsi/qedi/qedi_hsi.h
> create mode 100644 drivers/scsi/qedi/qedi_iscsi.c
> create mode 100644 drivers/scsi/qedi/qedi_iscsi.h
> create mode 100644 drivers/scsi/qedi/qedi_main.c
> create mode 100644 drivers/scsi/qedi/qedi_sysfs.c
> create mode 100644 drivers/scsi/qedi/qedi_version.h
> create mode 100644 include/linux/qed/qed_iscsi_if.h
>
>-- 
>1.8.3.1
>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Rangankar, Manish]

Hi Hannes,

Please find the response below,

On 11/11/16 10:13 PM, "Hannes Reinecke"  wrote:

>On 11/08/2016 07:57 AM, Manish Rangankar wrote:
>> The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module
>> for 41000 Series Converged Network Adapters by QLogic.
>>
>> This patch consists of following changes:
>>   - MAINTAINERS Makefile and Kconfig changes for qedi,
>>   - PCI driver registration,
>>   - iSCSI host level initialization,
>>   - Debugfs and log level infrastructure.
>>
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>>  MAINTAINERS |6 +
>>  drivers/net/ethernet/qlogic/Kconfig |   12 -
>>  drivers/scsi/Kconfig|1 +
>>  drivers/scsi/Makefile   |1 +
>>  drivers/scsi/qedi/Kconfig   |   10 +
>>  drivers/scsi/qedi/Makefile  |5 +
>>  drivers/scsi/qedi/qedi.h|  291 +++
>>  drivers/scsi/qedi/qedi_dbg.c|  143 
>>  drivers/scsi/qedi/qedi_dbg.h|  144 
>>  drivers/scsi/qedi/qedi_debugfs.c|  244 ++
>>  drivers/scsi/qedi/qedi_hsi.h|   52 ++
>>  drivers/scsi/qedi/qedi_main.c   | 1616
>>+++
>>  drivers/scsi/qedi/qedi_sysfs.c  |   52 ++
>>  drivers/scsi/qedi/qedi_version.h|   14 +
>>  14 files changed, 2579 insertions(+), 12 deletions(-)
>>  create mode 100644 drivers/scsi/qedi/Kconfig
>>  create mode 100644 drivers/scsi/qedi/Makefile
>>  create mode 100644 drivers/scsi/qedi/qedi.h
>>  create mode 100644 drivers/scsi/qedi/qedi_dbg.c
>>  create mode 100644 drivers/scsi/qedi/qedi_dbg.h
>>  create mode 100644 drivers/scsi/qedi/qedi_debugfs.c
>>  create mode 100644 drivers/scsi/qedi/qedi_hsi.h
>>  create mode 100644 drivers/scsi/qedi/qedi_main.c
>>  create mode 100644 drivers/scsi/qedi/qedi_sysfs.c
>>  create mode 100644 drivers/scsi/qedi/qedi_version.h
>>
[...]
>>
>> +static enum qed_int_mode qedi_int_mode_to_enum(void)
>> +{
>> +switch (int_mode) {
>> +case 0: return QED_INT_MODE_MSIX;
>> +case 1: return QED_INT_MODE_INTA;
>> +case 2: return QED_INT_MODE_MSI;
>> +default:
>> +QEDI_ERR(NULL, "Unknown qede_int_mode=%08x; "
>> + "Defaulting to MSI-x\n", int_mode);
>> +return QED_INT_MODE_MSIX;
>> +}
>> +}
>Errm. A per-driver interrupt mode?
>How very curious.
>You surely want to make that per-HBA, right?

This was added for testing purpose, we will remove this code.
 

[...]
>> +static int qedi_request_msix_irq(struct qedi_ctx *qedi)
>> +{
>> +int i, rc, cpu;
>> +
>> +cpu = cpumask_first(cpu_online_mask);
>> +for (i = 0; i < MIN_NUM_CPUS_MSIX(qedi); i++) {
>> +rc = request_irq(qedi->int_info.msix[i].vector,
>> + qedi_msix_handler, 0, "qedi",
>> + >fp_array[i]);
>> +
>> +if (rc) {
>> +QEDI_WARN(>dbg_ctx, "request_irq failed.\n");
>> +qedi_sync_free_irqs(qedi);
>> +return rc;
>> +}
>> +qedi->int_info.used_cnt++;
>> +rc = irq_set_affinity_hint(qedi->int_info.msix[i].vector,
>> +   get_cpu_mask(cpu));
>> +cpu = cpumask_next(cpu, cpu_online_mask);
>> +}
>> +
>> +return 0;
>> +}
>Please use the irq-affinity rework from Christoph here; that'll save you
>the additional msix vectors allocation.

The existing qed* driver(s) and common module (qed) framework is built on
top of the older pci_enable_msix_*() API. The new framework requires
re-work on the existing qed common module API. That would need
co-ordination among other dependent drivers (e.g.: qede network driver,
which is already in the tree). We would prefer to add this as a follow on
(to the initial submission) effort, with additional testing done and
submission co-ordinated across protocol drivers.



Thanks,
Manish

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Rangankar, Manish]

On 19/10/16 3:32 PM, "Johannes Thumshirn"  wrote:

>On Wed, Oct 19, 2016 at 01:01:10AM -0400, manish.rangan...@cavium.com
>wrote:
>> From: Manish Rangankar 
>> 
>> The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module
>> for 41000 Series Converged Network Adapters by QLogic.
>> 
>> This patch consists of following changes:
>>   - MAINTAINERS Makefile and Kconfig changes for qedi,
>>   - PCI driver registration,
>>   - iSCSI host level initialization,
>>   - Debugfs and log level infrastructure.
>> 
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>
>[...]
>
>> +/* MSI-X fastpath handler code */
>> +static irqreturn_t qedi_msix_handler(int irq, void *dev_id)
>> +{
>> +struct qedi_fastpath *fp = dev_id;
>> +struct qedi_ctx *qedi = fp->qedi;
>> +bool wake_io_thread = true;
>> +
>> +qed_sb_ack(fp->sb_info, IGU_INT_DISABLE, 0);
>> +
>> +process_again:
>> +wake_io_thread = qedi_process_completions(fp);
>> +if (wake_io_thread) {
>> +QEDI_INFO(>dbg_ctx, QEDI_LOG_DISC,
>> +  "process already running\n");
>> +}
>> +
>> +if (qedi_fp_has_work(fp) == 0)
>> +qed_sb_update_sb_idx(fp->sb_info);
>> +
>> +/* Check for more work */
>> +rmb();
>> +
>> +if (qedi_fp_has_work(fp) == 0)
>> +qed_sb_ack(fp->sb_info, IGU_INT_ENABLE, 1);
>> +else
>> +goto process_again;
>> +
>> +return IRQ_HANDLED;
>> +}
>
>You might want to consider workqueues here.

If there is no serious objection with current per-cpu threads
implementation
then we will like to do workqueue changes just after first submission.
This is because, 
for this change we have go through complete validation cycle on our part.


Thanks,
Manish R.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 6/6] qedi: Add support for data path.

[Rangankar, Manish]

On 19/10/16 3:54 PM, "Hannes Reinecke"  wrote:

>On 10/19/2016 07:01 AM, manish.rangan...@cavium.com wrote:
>> From: Manish Rangankar 
>> 
>> This patch adds support for data path and TMF handling.
>> 
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>>  drivers/scsi/qedi/qedi_fw.c| 1282
>>
>>  drivers/scsi/qedi/qedi_gbl.h   |6 +
>>  drivers/scsi/qedi/qedi_iscsi.c |6 +
>>  drivers/scsi/qedi/qedi_main.c  |4 +
>>  4 files changed, 1298 insertions(+)
>> 
>> diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>> index a820785..af1e14d 100644
>> --- a/drivers/scsi/qedi/qedi_fw.c
>> +++ b/drivers/scsi/qedi/qedi_fw.c
>> @@ -147,6 +147,114 @@ static void qedi_process_text_resp(struct
>>qedi_ctx *qedi,
>>  spin_unlock(>back_lock);
>>  }

--snipped--
>> +void qedi_trace_io(struct qedi_ctx *qedi, struct iscsi_task *task,
>> +   u16 tid, int8_t direction)
>> +{
>> +struct qedi_io_log *io_log;
>> +struct iscsi_conn *conn = task->conn;
>> +struct qedi_conn *qedi_conn = conn->dd_data;
>> +struct scsi_cmnd *sc_cmd = task->sc;
>> +unsigned long flags;
>> +u8 op;
>> +
>> +spin_lock_irqsave(>io_trace_lock, flags);
>> +
>> +io_log = >io_trace_buf[qedi->io_trace_idx];
>> +io_log->direction = direction;
>> +io_log->task_id = tid;
>> +io_log->cid = qedi_conn->iscsi_conn_id;
>> +io_log->lun = sc_cmd->device->lun;
>> +io_log->op = sc_cmd->cmnd[0];
>> +op = sc_cmd->cmnd[0];
>> +
>> +if (op == READ_10 || op == WRITE_10) {
>> +io_log->lba[0] = sc_cmd->cmnd[2];
>> +io_log->lba[1] = sc_cmd->cmnd[3];
>> +io_log->lba[2] = sc_cmd->cmnd[4];
>> +io_log->lba[3] = sc_cmd->cmnd[5];
>> +} else {
>> +io_log->lba[0] = 0;
>> +io_log->lba[1] = 0;
>> +io_log->lba[2] = 0;
>> +io_log->lba[3] = 0;
>> +}
>Only for READ_10 and WRITE_10? What about the other read or write
>commands?

We will add support for other scsi commands in the next revision.

>
>> +io_log->bufflen = scsi_bufflen(sc_cmd);
>> +io_log->sg_count = scsi_sg_count(sc_cmd);
>> +io_log->fast_sgs = qedi->fast_sgls;
>> +io_log->cached_sgs = qedi->cached_sgls;
>> +io_log->slow_sgs = qedi->slow_sgls;
>> +io_log->cached_sge = qedi->use_cached_sge;
>> +io_log->slow_sge = qedi->use_slow_sge;
>> +io_log->fast_sge = qedi->use_fast_sge;
>> +io_log->result = sc_cmd->result;
>> +io_log->jiffies = jiffies;
>> +io_log->blk_req_cpu = smp_processor_id();
>> +
>> +if (direction == QEDI_IO_TRACE_REQ) {
>> +/* For requests we only care about the submission CPU */
>> +io_log->req_cpu = smp_processor_id() % qedi->num_queues;
>> +io_log->intr_cpu = 0;
>> +io_log->blk_rsp_cpu = 0;
>> +} else if (direction == QEDI_IO_TRACE_RSP) {
>> +io_log->req_cpu = smp_processor_id() % qedi->num_queues;
>> +io_log->intr_cpu = qedi->intr_cpu;
>> +io_log->blk_rsp_cpu = smp_processor_id();
>> +}
>> +
>> +qedi->io_trace_idx++;
>> +if (qedi->io_trace_idx == QEDI_IO_TRACE_SIZE)
>> +qedi->io_trace_idx = 0;
>> +
>> +qedi->use_cached_sge = false;
>> +qedi->use_slow_sge = false;
>> +qedi->use_fast_sge = false;
>> +
>> +spin_unlock_irqrestore(>io_trace_lock, flags);
>> +}
>> +
>> +int qedi_iscsi_send_ioreq(struct iscsi_task *task)
>> +{
>> +struct iscsi_conn *conn = task->conn;
>> +struct iscsi_session *session = conn->session;
>> +struct Scsi_Host *shost =
>>iscsi_session_to_shost(session->cls_session);
>> +struct qedi_ctx *qedi = iscsi_host_priv(shost);
>> +struct qedi_conn *qedi_conn = conn->dd_data;
>> +struct qedi_cmd *cmd = task->dd_data;
>> +struct scsi_cmnd *sc = task->sc;
>> +struct iscsi_task_context *fw_task_ctx;
>> +struct iscsi_cached_sge_ctx *cached_sge;
>> +struct iscsi_phys_sgl_ctx *phys_sgl;
>> +struct iscsi_virt_sgl_ctx *virt_sgl;
>> +struct ystorm_iscsi_task_st_ctx *yst_cxt;
>> +struct mstorm_iscsi_task_st_ctx *mst_cxt;
>> +struct iscsi_sgl *sgl_struct;
>> +struct iscsi_sge *single_sge;
>> +struct iscsi_scsi_req *hdr = (struct iscsi_scsi_req *)task->hdr;
>> +struct iscsi_sge *bd = cmd->io_tbl.sge_tbl;
>> +enum iscsi_task_type task_type;
>> +struct iscsi_cmd_hdr *fw_cmd;
>> +u32 scsi_lun[2];
>> +u16 cq_idx = smp_processor_id() % qedi->num_queues;
>> +s16 ptu_invalidate = 0;
>> +s16 tid = 0;
>> +u8 num_fast_sgs;
>> +
>> +tid = 

Re: [RFC 5/6] qedi: Add support for iSCSI session management.

[Rangankar, Manish]

On 19/10/16 1:33 PM, "Hannes Reinecke"  wrote:

>On 10/19/2016 07:01 AM, manish.rangan...@cavium.com wrote:
>> From: Manish Rangankar 
>> 
>> This patch adds support for iscsi_transport LLD Login,
>> Logout, NOP-IN/NOP-OUT, Async, Reject PDU processing
>> and Firmware async event handling support.
>> 
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>>  drivers/scsi/qedi/qedi_fw.c| 1123 
>>  drivers/scsi/qedi/qedi_gbl.h   |   67 ++
>>  drivers/scsi/qedi/qedi_iscsi.c | 1604
>>
>>  drivers/scsi/qedi/qedi_iscsi.h |  228 ++
>>  drivers/scsi/qedi/qedi_main.c  |  164 
>>  5 files changed, 3186 insertions(+)
>>  create mode 100644 drivers/scsi/qedi/qedi_fw.c
>>  create mode 100644 drivers/scsi/qedi/qedi_gbl.h
>>  create mode 100644 drivers/scsi/qedi/qedi_iscsi.c
>>  create mode 100644 drivers/scsi/qedi/qedi_iscsi.h
>> 

--snipped--
>>
>> +static void qedi_process_async_mesg(struct qedi_ctx *qedi,
>> +union iscsi_cqe *cqe,
>> +struct iscsi_task *task,
>> +struct qedi_conn *qedi_conn,
>> +u16 que_idx)
>> +{
>> +struct iscsi_conn *conn = qedi_conn->cls_conn->dd_data;
>> +struct iscsi_session *session = conn->session;
>> +struct iscsi_async_msg_hdr *cqe_async_msg;
>> +struct iscsi_async *resp_hdr;
>> +u32 scsi_lun[2];
>> +u32 pdu_len, num_bdqs;
>> +char bdq_data[QEDI_BDQ_BUF_SIZE];
>> +unsigned long flags;
>> +
>> +spin_lock_bh(>back_lock);
>> +
>> +cqe_async_msg = >cqe_common.iscsi_hdr.async_msg;
>> +pdu_len = cqe_async_msg->hdr_second_dword &
>> +ISCSI_ASYNC_MSG_HDR_DATA_SEG_LEN_MASK;
>> +num_bdqs = pdu_len / QEDI_BDQ_BUF_SIZE;
>> +
>> +if (cqe->cqe_common.cqe_type == ISCSI_CQE_TYPE_UNSOLICITED) {
>> +spin_lock_irqsave(>hba_lock, flags);
>> +qedi_unsol_pdu_adjust_bdq(qedi, >cqe_unsolicited,
>> +  pdu_len, num_bdqs, bdq_data);
>> +spin_unlock_irqrestore(>hba_lock, flags);
>> +}
>> +
>> +resp_hdr = (struct iscsi_async *)_conn->gen_pdu.resp_hdr;
>> +memset(resp_hdr, 0, sizeof(struct iscsi_hdr));
>> +resp_hdr->opcode = cqe_async_msg->opcode;
>> +resp_hdr->flags = 0x80;
>> +
>> +scsi_lun[0] = cpu_to_be32(cqe_async_msg->lun.lo);
>> +scsi_lun[1] = cpu_to_be32(cqe_async_msg->lun.hi);
>I _think_ we have a SCSI LUN structure ...

Will do.

--snipped--
>> +void qedi_process_iscsi_error(struct qedi_endpoint *ep, struct
>>async_data *data)
>> +{
>> +struct qedi_conn *qedi_conn;
>> +struct qedi_ctx *qedi;
>> +char warn_notice[] = "iscsi_warning";
>> +char error_notice[] = "iscsi_error";
>> +char *message;
>> +int need_recovery = 0;
>> +u32 err_mask = 0;
>> +char msg[64];
>> +
>> +if (!ep)
>> +return;
>> +
>> +qedi_conn = ep->conn;
>> +if (!qedi_conn)
>> +return;
>> +
>> +qedi = ep->qedi;
>> +
>> +QEDI_ERR(>dbg_ctx, "async event iscsi error:0x%x\n",
>> + data->error_code);
>> +
>> +if (err_mask) {
>> +need_recovery = 0;
>> +message = warn_notice;
>> +} else {
>> +need_recovery = 1;
>> +message = error_notice;
>> +}
>> +
>> +switch (data->error_code) {
>> +case ISCSI_STATUS_NONE:
>> +strcpy(msg, "tcp_error none");
>> +break;
>> +case ISCSI_CONN_ERROR_TASK_CID_MISMATCH:
>> +strcpy(msg, "task cid mismatch");
>> +break;
>> +case ISCSI_CONN_ERROR_TASK_NOT_VALID:
>> +strcpy(msg, "invalid task");
>> +break;
>> +case ISCSI_CONN_ERROR_RQ_RING_IS_FULL:
>> +strcpy(msg, "rq ring full");
>> +break;
>> +case ISCSI_CONN_ERROR_CMDQ_RING_IS_FULL:
>> +strcpy(msg, "cmdq ring full");
>> +break;
>> +case ISCSI_CONN_ERROR_HQE_CACHING_FAILED:
>> +strcpy(msg, "sge caching failed");
>> +break;
>> +case ISCSI_CONN_ERROR_HEADER_DIGEST_ERROR:
>> +strcpy(msg, "hdr digest error");
>> +break;
>> +case ISCSI_CONN_ERROR_LOCAL_COMPLETION_ERROR:
>> +strcpy(msg, "local cmpl error");
>> +break;
>> +case ISCSI_CONN_ERROR_DATA_OVERRUN:
>> +strcpy(msg, "invalid task");
>> +break;
>> +case ISCSI_CONN_ERROR_OUT_OF_SGES_ERROR:
>> +strcpy(msg, "out of sge error");
>> +break;
>> +case 

Re: [RFC 5/6] qedi: Add support for iSCSI session management.

[Rangankar, Manish]

On 19/10/16 6:58 PM, "Johannes Thumshirn"  wrote:

>On Wed, Oct 19, 2016 at 01:01:12AM -0400, manish.rangan...@cavium.com
>wrote:
>> From: Manish Rangankar 
>> 
>> This patch adds support for iscsi_transport LLD Login,
>> Logout, NOP-IN/NOP-OUT, Async, Reject PDU processing
>> and Firmware async event handling support.
>> 
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>
>[...]
>
>> +void qedi_iscsi_unmap_sg_list(struct qedi_cmd *cmd)
>> +{
>> +struct scsi_cmnd *sc = cmd->scsi_cmd;
>> +
>> +if (cmd->io_tbl.sge_valid && sc) {
>> +scsi_dma_unmap(sc);
>> +cmd->io_tbl.sge_valid = 0;
>> +}
>> +}
>
>Maybe set sge_valid to 0 and then call scsi_dma_unmap(). I don't know if
>it's
>really racy but it looks like it is.
>
>[...]
>
>> +static void qedi_process_text_resp(struct qedi_ctx *qedi,
>> +   union iscsi_cqe *cqe,
>> +   struct iscsi_task *task,
>> +   struct qedi_conn *qedi_conn)
>> +{
>> +struct iscsi_conn *conn = qedi_conn->cls_conn->dd_data;
>> +struct iscsi_session *session = conn->session;
>> +struct iscsi_task_context *task_ctx;
>> +struct iscsi_text_rsp *resp_hdr_ptr;
>> +struct iscsi_text_response_hdr *cqe_text_response;
>> +struct qedi_cmd *cmd;
>> +int pld_len;
>> +u32 *tmp;
>> +
>> +cmd = (struct qedi_cmd *)task->dd_data;
>> +task_ctx = (struct iscsi_task_context
>>*)qedi_get_task_mem(>tasks,
>> +  cmd->task_id);
>
>No need to cast here, qedi_get_task_mem() returns void *.
>
>[...]
>
>> +cqe_login_response = >cqe_common.iscsi_hdr.login_response;
>> +task_ctx = (struct iscsi_task_context
>>*)qedi_get_task_mem(>tasks,
>> +  cmd->task_id);
>
>Same here.
>
>[...]
>
>> +}
>> +
>> +pbl = (struct scsi_bd *)qedi->bdq_pbl;
>> +pbl += (qedi->bdq_prod_idx % qedi->rq_num_entries);
>> +pbl->address.hi =
>> +  cpu_to_le32((u32)(((u64)(qedi->bdq[idx].buf_dma)) >> 32));
>> +pbl->address.lo =
>> +cpu_to_le32(((u32)(((u64)(qedi->bdq[idx].buf_dma)) &
>> +0x)));
>
>Is this LISP or C?
>
>> +QEDI_INFO(>dbg_ctx, QEDI_LOG_CONN,
>> +  "pbl [0x%p] pbl->address hi [0x%llx] lo [0x%llx] idx [%d]\n",
>> +  pbl, pbl->address.hi, pbl->address.lo, idx);
>> +pbl->opaque.hi = cpu_to_le32((u32)(((u64)0) >> 32));
>
>Isn't this plain pbl->opaque.hi = 0; ?
>
>> +pbl->opaque.lo = cpu_to_le32(((u32)(((u64)idx) & 0x)));
>> +
>
>[...]
>
>> +switch (comp_type) {
>> +case ISCSI_CQE_TYPE_SOLICITED:
>> +case ISCSI_CQE_TYPE_SOLICITED_WITH_SENSE:
>> +fw_task_ctx =
>> +  (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
>> +  cqe->cqe_solicited.itid);
>
>Again, no cast needed.
>
>[...]
>
>> +writel(*(u32 *), qedi_conn->ep->p_doorbell);
>> +/* Make sure fw idx is coherent */
>> +wmb();
>> +mmiowb();
>
>Isn't either wmb() or mmiowb() enough?
>
>[..]
>
>> +
>> +fw_task_ctx =
>> + (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
>>tid);
>
>Cast again.
>
>[...]
>
>> +fw_task_ctx =
>> + (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
>>tid);
>
>^^
>
>[...]
>
>> +fw_task_ctx =
>> +(struct iscsi_task_context *)qedi_get_task_mem(>tasks, tid);
>
>
>[...]
>
>> +fw_task_ctx =
>> +  (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
>>tid);
>> +
>
>[...]
>
>> +
>> +qedi = (struct qedi_ctx *)iscsi_host_priv(shost);
>
>Same goes for iscsi_host_priv();
>
>[...]
>
>> +ret = wait_event_interruptible_timeout(qedi_ep->ofld_wait,
>> +   ((qedi_ep->state ==
>> +EP_STATE_OFLDCONN_FAILED) ||
>> +(qedi_ep->state ==
>> +EP_STATE_OFLDCONN_COMPL)),
>> +msecs_to_jiffies(timeout_ms));
>
>Maybe:
>#define QEDI_OLDCON_STATE(q) ((q)->state == EP_STATE_OFLDCONN_FAILED || \
>   (q)->state == EP_STATE_OFLDCONN_COMPL)
>
>ret = wait_event_interruptible_timeout(qedi_ep->ofld_wait,
>   QEDI_OLDCON_STATE(qedi_ep),
>   msec_to_jiffies(timeout_ms));
>
>But that could be just me hating linewraps.
>
>[...]

We will address 

Re: [RFC 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Rangankar, Manish]

Thanks Hannes for the review, please see my comments below,

On 19/10/16 1:15 PM, "Hannes Reinecke"  wrote:

>On 10/19/2016 07:01 AM, manish.rangan...@cavium.com wrote:
>> From: Manish Rangankar 
>> 
>> The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module
>> for 41000 Series Converged Network Adapters by QLogic.
>> 
>> This patch consists of following changes:
>>   - MAINTAINERS Makefile and Kconfig changes for qedi,
>>   - PCI driver registration,
>>   - iSCSI host level initialization,
>>   - Debugfs and log level infrastructure.
>> 
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>>  MAINTAINERS |6 +
>>  drivers/net/ethernet/qlogic/Kconfig |   12 -
>>  drivers/scsi/Kconfig|1 +
>>  drivers/scsi/Makefile   |1 +
>>  drivers/scsi/qedi/Kconfig   |   10 +
>>  drivers/scsi/qedi/Makefile  |5 +
>>  drivers/scsi/qedi/qedi.h|  286 +++
>>  drivers/scsi/qedi/qedi_dbg.c|  143 
>>  drivers/scsi/qedi/qedi_dbg.h|  144 
>>  drivers/scsi/qedi/qedi_debugfs.c|  244 ++
>>  drivers/scsi/qedi/qedi_hsi.h|   52 ++
>>  drivers/scsi/qedi/qedi_main.c   | 1550
>>+++
>>  drivers/scsi/qedi/qedi_sysfs.c  |   52 ++
>>  drivers/scsi/qedi/qedi_version.h|   14 +
>>  14 files changed, 2508 insertions(+), 12 deletions(-)
>>  create mode 100644 drivers/scsi/qedi/Kconfig
>>  create mode 100644 drivers/scsi/qedi/Makefile
>>  create mode 100644 drivers/scsi/qedi/qedi.h
>>  create mode 100644 drivers/scsi/qedi/qedi_dbg.c
>>  create mode 100644 drivers/scsi/qedi/qedi_dbg.h
>>  create mode 100644 drivers/scsi/qedi/qedi_debugfs.c
>>  create mode 100644 drivers/scsi/qedi/qedi_hsi.h
>>  create mode 100644 drivers/scsi/qedi/qedi_main.c
>>  create mode 100644 drivers/scsi/qedi/qedi_sysfs.c
>>  create mode 100644 drivers/scsi/qedi/qedi_version.h
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5e925a2..906d05f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9909,6 +9909,12 @@ F:drivers/net/ethernet/qlogic/qed/
>>  F:  include/linux/qed/
>>  F:  drivers/net/ethernet/qlogic/qede/
>>  
>> +QLOGIC QL41xxx ISCSI DRIVER
>> +M:  qlogic-storage-upstr...@cavium.com
>> +L:  linux-scsi@vger.kernel.org
>> +S:  Supported
>> +F:  drivers/scsi/qedi/
>> +
>>  QNX4 FILESYSTEM
>>  M:  Anders Larsen 
>>  W:  http://www.alarsen.net/linux/qnx4fs/
>> diff --git a/drivers/net/ethernet/qlogic/Kconfig
>>b/drivers/net/ethernet/qlogic/Kconfig
>> index bad4fae..28b4366 100644
>> --- a/drivers/net/ethernet/qlogic/Kconfig
>> +++ b/drivers/net/ethernet/qlogic/Kconfig
>> @@ -121,16 +121,4 @@ config INFINIBAND_QEDR
>>  config QED_ISCSI
>>  bool
>>  
>> -config QEDI
>> -tristate "QLogic QED 25/40/100Gb iSCSI driver"
>> -depends on QED
>> -select QED_LL2
>> -select QED_ISCSI
>> -default n
>> ----help---
>> -  This provides a temporary node that allows the compilation
>> -  and logical testing of the hardware offload iSCSI support
>> -  for QLogic QED. This would be replaced by the 'real' option
>> -  once the QEDI driver is added [+relocated].
>> -
>>  endif # NET_VENDOR_QLOGIC
>Huh? You just introduce this one in patch 1/6.
>Please fold them together so that this can be omitted.

Yes, we will remove this in the next revision.

-- snipped --


>> @@ -0,0 +1,52 @@
>> +/*
>> + * QLogic iSCSI Offload Driver
>> + * Copyright (c) 2016 Cavium Inc.
>> + *
>> + * This software is available under the terms of the GNU General
>>Public License
>> + * (GPL) Version 2, available from the file COPYING in the main
>>directory of
>> + * this source tree.
>> + */
>> +#ifndef __QEDI_HSI__
>> +#define __QEDI_HSI__
>> +//
>> +/* Add include to common target */
>> +//
>> +#include 
>> +
>Please use kernel-doc style for comments

Will do.

--snipped--
>> +static void qedi_int_fp(struct qedi_ctx *qedi)
>> +{
>> +struct qedi_fastpath *fp;
>> +int id;
>> +
>> +memset((void *)qedi->fp_array, 0, MIN_NUM_CPUS_MSIX(qedi) *
>> +   sizeof(*qedi->fp_array));
>> +memset((void *)qedi->sb_array, 0, MIN_NUM_CPUS_MSIX(qedi) *
>> +   sizeof(*qedi->sb_array));
>> +
>> +for (id = 0; id < MIN_NUM_CPUS_MSIX(qedi); id++) {
>> +fp = >fp_array[id];
>> +fp->sb_info = >sb_array[id];
>> +fp->sb_id = id;
>> +fp->qedi = qedi;
>> +snprintf(fp->name, sizeof(fp->name), "%s-fp-%d",
>> + "qedi", id);
>> +
>> +/* 

Re: [RFC 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Rangankar, Manish]

Thanks Johannes for the review, please see comments below,


On 19/10/16 3:32 PM, "Johannes Thumshirn"  wrote:

>On Wed, Oct 19, 2016 at 01:01:10AM -0400, manish.rangan...@cavium.com
>wrote:
>> From: Manish Rangankar 
>> 
>> The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module
>> for 41000 Series Converged Network Adapters by QLogic.
>> 
>> This patch consists of following changes:
>>   - MAINTAINERS Makefile and Kconfig changes for qedi,
>>   - PCI driver registration,
>>   - iSCSI host level initialization,
>>   - Debugfs and log level infrastructure.
>> 
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>
>[...]
>
>> +static inline void *qedi_get_task_mem(struct qed_iscsi_tid *info, u32
>>tid)
>> +{
>> +return (void *)(info->blocks[tid / info->num_tids_per_block] +
>> +(tid % info->num_tids_per_block) * info->size);
>> +}
>
>Unnecessary cast here.

Noted

>
>
>[...]
>
>> +void
>> +qedi_dbg_err(struct qedi_dbg_ctx *qedi, const char *func, u32 line,
>> + const char *fmt, ...)
>> +{
>> +va_list va;
>> +struct va_format vaf;
>> +char nfunc[32];
>> +
>> +memset(nfunc, 0, sizeof(nfunc));
>> +memcpy(nfunc, func, sizeof(nfunc) - 1);
>> +
>> +va_start(va, fmt);
>> +
>> +vaf.fmt = fmt;
>> +vaf.va = 
>> +
>> +if (likely(qedi) && likely(qedi->pdev))
>> +pr_crit("[%s]:[%s:%d]:%d: %pV", dev_name(>pdev->dev),
>> +nfunc, line, qedi->host_no, );
>> +else
>> +pr_crit("[:00:00.0]:[%s:%d]: %pV", nfunc, line, );
>
>pr_crit, seriously?

We will change it to pr_err.

>
>[...]
>
>> +static void qedi_int_fp(struct qedi_ctx *qedi)
>> +{
>> +struct qedi_fastpath *fp;
>> +int id;
>> +
>> +memset((void *)qedi->fp_array, 0, MIN_NUM_CPUS_MSIX(qedi) *
>> +   sizeof(*qedi->fp_array));
>> +memset((void *)qedi->sb_array, 0, MIN_NUM_CPUS_MSIX(qedi) *
>> +   sizeof(*qedi->sb_array));
>
>I don't think the cast is necessary here.

Noted


>
>[...]
>
>> +static int qedi_setup_cid_que(struct qedi_ctx *qedi)
>> +{
>> +int i;
>> +
>> +qedi->cid_que.cid_que_base = kmalloc((qedi->max_active_conns *
>> +  sizeof(u32)), GFP_KERNEL);
>> +if (!qedi->cid_que.cid_que_base)
>> +return -ENOMEM;
>> +
>> +qedi->cid_que.conn_cid_tbl = kmalloc((qedi->max_active_conns *
>> +  sizeof(struct qedi_conn *)),
>> + GFP_KERNEL);
>
>Please use kmalloc_array() here.

Will do.

>
>[...]
>
>> +/* MSI-X fastpath handler code */
>> +static irqreturn_t qedi_msix_handler(int irq, void *dev_id)
>> +{
>> +struct qedi_fastpath *fp = dev_id;
>> +struct qedi_ctx *qedi = fp->qedi;
>> +bool wake_io_thread = true;
>> +
>> +qed_sb_ack(fp->sb_info, IGU_INT_DISABLE, 0);
>> +
>> +process_again:
>> +wake_io_thread = qedi_process_completions(fp);
>> +if (wake_io_thread) {
>> +QEDI_INFO(>dbg_ctx, QEDI_LOG_DISC,
>> +  "process already running\n");
>> +}
>> +
>> +if (qedi_fp_has_work(fp) == 0)
>> +qed_sb_update_sb_idx(fp->sb_info);
>> +
>> +/* Check for more work */
>> +rmb();
>> +
>> +if (qedi_fp_has_work(fp) == 0)
>> +qed_sb_ack(fp->sb_info, IGU_INT_ENABLE, 1);
>> +else
>> +goto process_again;
>> +
>> +return IRQ_HANDLED;
>> +}
>
>You might want to consider workqueues here.

We will revisit this code.

>
>[...]
>
>> +static int qedi_alloc_itt(struct qedi_ctx *qedi)
>> +{
>> +qedi->itt_map = kzalloc((sizeof(struct qedi_itt_map) *
>> +MAX_ISCSI_TASK_ENTRIES), GFP_KERNEL);
>
>that screams for kcalloc()
>
>> +if (!qedi->itt_map) {
>> +QEDI_ERR(>dbg_ctx,
>> + "Unable to allocate itt map array memory\n");
>> +return -ENOMEM;
>> +}
>> +return 0;
>> +}
>> +
>> +static void qedi_free_itt(struct qedi_ctx *qedi)
>> +{
>> +kfree(qedi->itt_map);
>> +}
>> +
>> +static struct qed_ll2_cb_ops qedi_ll2_cb_ops = {
>> +.rx_cb = qedi_ll2_rx,
>> +.tx_cb = NULL,
>> +};
>> +
>> +static int qedi_percpu_io_thread(void *arg)
>> +{
>> +struct qedi_percpu_s *p = arg;
>> +struct qedi_work *work, *tmp;
>> +unsigned long flags;
>> +LIST_HEAD(work_list);
>> +
>> +set_user_nice(current, -20);
>> +
>> +while (!kthread_should_stop()) {
>> +spin_lock_irqsave(>p_work_lock, flags);
>> +while (!list_empty(>work_list)) {
>> +list_splice_init(>work_list, _list);
>> +