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] 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

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 

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 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] 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] 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] 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: 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] 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: 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-s...@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: qla4xxx: add error handling for try_module_get

[Rangankar, Manish]

> -Original Message-
> From: Zhouyang Jia 
> Sent: Tuesday, June 12, 2018 10:18 AM
> Cc: Zhouyang Jia ; Dept-Eng QLogic Storage
> Upstream ; James E.J. Bottomley
> ; Martin K. Petersen ;
> linux-s...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] scsi: qla4xxx: add error handling for try_module_get
> 
> When try_module_get fails, the lack of error-handling code may cause
> unexpected results.
> 
> This patch adds error-handling code after calling try_module_get.
> 
> Signed-off-by: Zhouyang Jia 
> ---
>  drivers/scsi/qla4xxx/ql4_os.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c 
> index
> 0e13349..6b677ab 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -7687,7 +7687,10 @@ static int qla4xxx_sysfs_ddb_logout_sid(struct
> iscsi_cls_session *cls_sess)
>* to be seamless without actually destroying the
>* session
>**/
> - try_module_get(qla4xxx_iscsi_transport.owner);
> + if (!try_module_get(qla4xxx_iscsi_transport.owner))
> + ql4_printk(KERN_WARNING, ha,
> + "%s: cannot get module.\n", __func__);
> +
>   iscsi_destroy_endpoint(ddb_entry->conn->ep);
> 
>   spin_lock_irqsave(>hardware_lock, flags); @@ -8970,7 +8973,9
> @@ static void qla4xxx_destroy_fw_ddb_session(struct scsi_qla_host *ha)
>* to be seamless without actually destroying the
>* session
>**/
> - try_module_get(qla4xxx_iscsi_transport.owner);
> + if (!try_module_get(qla4xxx_iscsi_transport.owner))
> + ql4_printk(KERN_WARNING, ha,
> + "%s: cannot get module.\n",
> __func__);
>   iscsi_destroy_endpoint(ddb_entry->conn->ep);
>   qla4xxx_free_ddb(ha, ddb_entry);
>   iscsi_session_teardown(ddb_entry->sess);
> --
> 2.7.4

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: [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 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 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] scsi: qla4xxx: NULL check before some freeing functions is not needed.

[Rangankar, Manish]

> -Original Message-
> From: Thomas Meyer 
> Sent: Monday, December 3, 2018 2:22 AM
> To: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; j...@linux.vnet.ibm.com;
> martin.peter...@oracle.com; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] scsi: qla4xxx: NULL check before some freeing functions is 
> not
> needed.
> 
> External Email
> 
> NULL check before some freeing functions is not needed.
> 
> Signed-off-by: Thomas Meyer 
> ---
> 
> diff -u -p a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -4160,20 +4160,17 @@ static void qla4xxx_mem_free(struct scsi
> 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;
> 
> -   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] scsi: qedf: NULL check before some freeing functions is not needed.

[Rangankar, Manish]

> -Original Message-
> From: Thomas Meyer 
> Sent: Monday, December 3, 2018 2:22 AM
> To: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; j...@linux.vnet.ibm.com;
> martin.peter...@oracle.com; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] scsi: qedf: NULL check before some freeing functions is not
> needed.
> 
> External Email
> 
> NULL check before some freeing functions is not needed.
> 
> Signed-off-by: Thomas Meyer 
> ---
> 
> diff -u -p a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
> --- a/drivers/scsi/qedf/qedf_main.c
> +++ b/drivers/scsi/qedf/qedf_main.c
> @@ -2935,8 +2935,7 @@ static void qedf_free_fcoe_pf_param(stru
> 
> qedf_free_global_queues(qedf);
> 
> -   if (qedf->global_queues)
> -   kfree(qedf->global_queues);
> +   kfree(qedf->global_queues);
>  }
> 
>  /*

Thanks
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;
>   struct qedi_endpoint *ep;
>@@ -1727,7 

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-s...@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: 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 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: [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 = dma_alloc_coherent(>pdev->dev, sizeof(*sys_info),
>-_info_dma, 

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 = -ENOMEM;
>   goto out_free_sq;
>   }
>-  

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] 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] 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] 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: qla4xxx: NULL check before some freeing functions is not needed.

[Rangankar, Manish]

> -Original Message-
> From: Thomas Meyer 
> Sent: Monday, December 3, 2018 2:22 AM
> To: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; j...@linux.vnet.ibm.com;
> martin.peter...@oracle.com; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] scsi: qla4xxx: NULL check before some freeing functions is 
> not
> needed.
> 
> External Email
> 
> NULL check before some freeing functions is not needed.
> 
> Signed-off-by: Thomas Meyer 
> ---
> 
> diff -u -p a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -4160,20 +4160,17 @@ static void qla4xxx_mem_free(struct scsi
> 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;
> 
> -   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] scsi: qedf: NULL check before some freeing functions is not needed.

[Rangankar, Manish]

> -Original Message-
> From: Thomas Meyer 
> Sent: Monday, December 3, 2018 2:22 AM
> To: Dept-Eng QLogic Storage Upstream  upstr...@cavium.com>; j...@linux.vnet.ibm.com;
> martin.peter...@oracle.com; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] scsi: qedf: NULL check before some freeing functions is not
> needed.
> 
> External Email
> 
> NULL check before some freeing functions is not needed.
> 
> Signed-off-by: Thomas Meyer 
> ---
> 
> diff -u -p a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
> --- a/drivers/scsi/qedf/qedf_main.c
> +++ b/drivers/scsi/qedf/qedf_main.c
> @@ -2935,8 +2935,7 @@ static void qedf_free_fcoe_pf_param(stru
> 
> qedf_free_global_queues(qedf);
> 
> -   if (qedf->global_queues)
> -   kfree(qedf->global_queues);
> +   kfree(qedf->global_queues);
>  }
> 
>  /*

Thanks
Acked-by: Manish Rangankar 

RE: [PATCH] scsi: qla4xxx: add error handling for try_module_get

[Rangankar, Manish]

> -Original Message-
> From: Zhouyang Jia 
> Sent: Tuesday, June 12, 2018 10:18 AM
> Cc: Zhouyang Jia ; Dept-Eng QLogic Storage
> Upstream ; James E.J. Bottomley
> ; Martin K. Petersen ;
> linux-s...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] scsi: qla4xxx: add error handling for try_module_get
> 
> When try_module_get fails, the lack of error-handling code may cause
> unexpected results.
> 
> This patch adds error-handling code after calling try_module_get.
> 
> Signed-off-by: Zhouyang Jia 
> ---
>  drivers/scsi/qla4xxx/ql4_os.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c 
> index
> 0e13349..6b677ab 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -7687,7 +7687,10 @@ static int qla4xxx_sysfs_ddb_logout_sid(struct
> iscsi_cls_session *cls_sess)
>* to be seamless without actually destroying the
>* session
>**/
> - try_module_get(qla4xxx_iscsi_transport.owner);
> + if (!try_module_get(qla4xxx_iscsi_transport.owner))
> + ql4_printk(KERN_WARNING, ha,
> + "%s: cannot get module.\n", __func__);
> +
>   iscsi_destroy_endpoint(ddb_entry->conn->ep);
> 
>   spin_lock_irqsave(>hardware_lock, flags); @@ -8970,7 +8973,9
> @@ static void qla4xxx_destroy_fw_ddb_session(struct scsi_qla_host *ha)
>* to be seamless without actually destroying the
>* session
>**/
> - try_module_get(qla4xxx_iscsi_transport.owner);
> + if (!try_module_get(qla4xxx_iscsi_transport.owner))
> + ql4_printk(KERN_WARNING, ha,
> + "%s: cannot get module.\n",
> __func__);
>   iscsi_destroy_endpoint(ddb_entry->conn->ep);
>   qla4xxx_free_ddb(ha, ddb_entry);
>   iscsi_session_teardown(ddb_entry->sess);
> --
> 2.7.4

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] 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] 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

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;
> 
>-  if (do_not_recover)
>+  if 

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