[PATCH net-next 2/4] qed: Share additional information with qedf

[Yuval Mintz]

Share several new tidbits with qedf:
 - wwpn & wwnn
 - Absolute pf-id [this one is actually meant for qedi as well]
 - Number of available CQs

While we're at it, now that qedf will be aware of the available CQs
we can add some validation on the inputs it provides.

Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qed/qed_dev.c  |  8 +++-
 drivers/net/ethernet/qlogic/qed/qed_fcoe.c | 14 ++
 drivers/net/ethernet/qlogic/qed/qed_main.c |  2 ++
 include/linux/qed/qed_fcoe_if.h|  5 +
 include/linux/qed/qed_if.h |  2 ++
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c 
b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 7649f35..2d88d48 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -2071,16 +2071,22 @@ static void qed_hw_set_feat(struct qed_hwfn *p_hwfn)
 QED_VF_L2_QUE));
}
 
+   if (p_hwfn->hw_info.personality == QED_PCI_FCOE)
+   feat_num[QED_FCOE_CQ] =  min_t(u32, sb_cnt.cnt,
+  RESC_NUM(p_hwfn,
+   QED_CMDQS_CQS));
+
if (p_hwfn->hw_info.personality == QED_PCI_ISCSI)
feat_num[QED_ISCSI_CQ] = min_t(u32, sb_cnt.cnt,
   RESC_NUM(p_hwfn,
QED_CMDQS_CQS));
DP_VERBOSE(p_hwfn,
   NETIF_MSG_PROBE,
-  "#PF_L2_QUEUES=%d VF_L2_QUEUES=%d #ROCE_CNQ=%d ISCSI_CQ=%d 
#SBS=%d\n",
+  "#PF_L2_QUEUES=%d VF_L2_QUEUES=%d #ROCE_CNQ=%d FCOE_CQ=%d 
ISCSI_CQ=%d #SBS=%d\n",
   (int)FEAT_NUM(p_hwfn, QED_PF_L2_QUE),
   (int)FEAT_NUM(p_hwfn, QED_VF_L2_QUE),
   (int)FEAT_NUM(p_hwfn, QED_RDMA_CNQ),
+  (int)FEAT_NUM(p_hwfn, QED_FCOE_CQ),
   (int)FEAT_NUM(p_hwfn, QED_ISCSI_CQ),
   (int)sb_cnt.cnt);
 }
diff --git a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c 
b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
index 3fc4ff2..df195c0 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
@@ -141,6 +141,15 @@ qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
p_data = _ramrod->init_ramrod_data;
fcoe_pf_params = _hwfn->pf_params.fcoe_pf_params;
 
+   /* Sanity */
+   if (fcoe_pf_params->num_cqs > p_hwfn->hw_info.feat_num[QED_FCOE_CQ]) {
+   DP_ERR(p_hwfn,
+  "Cannot satisfy CQ amount. CQs requested %d, CQs 
available %d. Aborting function start\n",
+  fcoe_pf_params->num_cqs,
+  p_hwfn->hw_info.feat_num[QED_FCOE_CQ]);
+   return -EINVAL;
+   }
+
p_data->mtu = cpu_to_le16(fcoe_pf_params->mtu);
tmp = cpu_to_le16(fcoe_pf_params->sq_num_pbl_pages);
p_data->sq_num_pages_in_pbl = tmp;
@@ -739,6 +748,11 @@ static int qed_fill_fcoe_dev_info(struct qed_dev *cdev,
info->secondary_bdq_rq_addr =
qed_fcoe_get_secondary_bdq_prod(hwfn, BDQ_ID_RQ);
 
+   info->wwpn = hwfn->mcp_info->func_info.wwn_port;
+   info->wwnn = hwfn->mcp_info->func_info.wwn_node;
+
+   info->num_cqs = FEAT_NUM(hwfn, QED_FCOE_CQ);
+
return rc;
 }
 
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c 
b/drivers/net/ethernet/qlogic/qed/qed_main.c
index ac3bdcd..baebd59 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -269,6 +269,8 @@ int qed_fill_dev_info(struct qed_dev *cdev,
if (QED_LEADING_HWFN(cdev)->hw_info.b_wol_support ==
QED_WOL_SUPPORT_PME)
dev_info->wol_support = true;
+
+   dev_info->abs_pf_id = QED_LEADING_HWFN(cdev)->abs_pf_id;
} else {
qed_vf_get_fw_version(>hwfns[0], _info->fw_major,
  _info->fw_minor, _info->fw_rev,
diff --git a/include/linux/qed/qed_fcoe_if.h b/include/linux/qed/qed_fcoe_if.h
index bd6bcb8..1e015c5 100644
--- a/include/linux/qed/qed_fcoe_if.h
+++ b/include/linux/qed/qed_fcoe_if.h
@@ -24,6 +24,11 @@ struct qed_dev_fcoe_info {
 
void __iomem *primary_dbq_rq_addr;
void __iomem *secondary_bdq_rq_addr;
+
+   u64 wwpn;
+   u64 wwnn;
+
+   u8 num_cqs;
 };
 
 struct qed_fcoe_params_offload {
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 607e1c5..e29c6f7 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -360,6 +360,8 @@ struct qed_dev_info {
boolvxlan_enable;
boolgre_enable;
boolgeneve_enable;
+
+   u8  abs_pf_id;
 };
 
 enum qed_sb_type {
-- 
2.9.4

[PATCH net-next 1/4] qed: Correct order of wwnn and wwpn

[Yuval Mintz]

Driver reads values via HSI splitting this 8-byte into 2 32-bit
values and builds a single u64 field - but it does so by shifting
the lower field instead of the higher.
Luckily, we still don't use these fields for anything - but we're about
to start.

Signed-off-by: Yuval Mintz 
---
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c 
b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 31c88e1..e82f329 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -1736,10 +1736,10 @@ int qed_mcp_fill_shmem_func_info(struct qed_hwfn 
*p_hwfn,
DP_NOTICE(p_hwfn, "MAC is 0 in shmem\n");
}
 
-   info->wwn_port = (u64)shmem_info.fcoe_wwn_port_name_upper |
-(((u64)shmem_info.fcoe_wwn_port_name_lower) << 32);
-   info->wwn_node = (u64)shmem_info.fcoe_wwn_node_name_upper |
-(((u64)shmem_info.fcoe_wwn_node_name_lower) << 32);
+   info->wwn_port = (u64)shmem_info.fcoe_wwn_port_name_lower |
+(((u64)shmem_info.fcoe_wwn_port_name_upper) << 32);
+   info->wwn_node = (u64)shmem_info.fcoe_wwn_node_name_lower |
+(((u64)shmem_info.fcoe_wwn_node_name_upper) << 32);
 
info->ovlan = (u16)(shmem_info.ovlan_stag & FUNC_MF_CFG_OV_STAG_MASK);
 
-- 
2.9.4

[PATCH net-next 0/4] qed: Enhance storage APIs

[Yuval Mintz]

This series is intended to add additional information and features
to the API between qed and its storage protocol drivers [qedi, qedf].

Patch #2 adds some information stored on device such as wwpn & wwnn
to allow qedf utilize it; #1 fixes an issue with the reading of those
values [which were unused until now].

Patch #3 would allow the protocol drivers access to images on persistent
storage which is a prerequirement for adding boot from SAN support.

Patch #4 adds infrastrucutre to a future feature for qedi.

Dave,

Please consider applying this series to 'net-next'.

Thanks,
Yuval

Yuval Mintz (4):
  qed: Correct order of wwnn and wwpn
  qed: Share additional information with qedf
  qed: Support NVM-image reading API
  qed: Add support for changing iSCSI mac

 drivers/net/ethernet/qlogic/qed/qed_dev.c   |  8 ++-
 drivers/net/ethernet/qlogic/qed/qed_fcoe.c  | 14 +
 drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 66 
 drivers/net/ethernet/qlogic/qed/qed_main.c  | 18 ++
 drivers/net/ethernet/qlogic/qed/qed_mcp.c   | 97 +++--
 drivers/net/ethernet/qlogic/qed/qed_mcp.h   | 21 +++
 drivers/net/ethernet/qlogic/qed/qed_sp.h|  1 +
 include/linux/qed/qed_fcoe_if.h |  5 ++
 include/linux/qed/qed_if.h  | 20 ++
 include/linux/qed/qed_iscsi_if.h|  7 +++
 10 files changed, 252 insertions(+), 5 deletions(-)

-- 
2.9.4

Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Julia Lawall]

On Fri, 2 Jun 2017, Milan P. Gandhi wrote:

> On 06/01/2017 08:32 PM, Dan Carpenter wrote:
> > On Thu, Jun 01, 2017 at 05:41:06PM +0530, Milan P. Gandhi wrote:
> >> Simplify the check for return code of fcoe_if_init routine
> >> in fcoe_init function such that we could eliminate need for
> >> extra 'out_free' label.
> >>
> >> Signed-off-by: Milan P. Gandhi 
> >> ---
> >>  drivers/scsi/fcoe/fcoe.c | 10 --
> >>  1 file changed, 4 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> >> index ea21e7b..fb2a4c9 100644
> >> --- a/drivers/scsi/fcoe/fcoe.c
> >> +++ b/drivers/scsi/fcoe/fcoe.c
> >> @@ -2523,13 +2523,11 @@ static int __init fcoe_init(void)
> >>fcoe_dev_setup();
> >>
> >>rc = fcoe_if_init();
> >> -  if (rc)
> >> -  goto out_free;
> >> -
> >> -  mutex_unlock(_config_mutex);
> >> -  return 0;
> >> +  if (rc == 0) {
> >> +  mutex_unlock(_config_mutex);
> >> +  return 0;
> >> +  }
> >>
> >> -out_free:
> >>mutex_unlock(_config_mutex);
> >
> > Gar...  Stop!  No1  Don't do this.
> >
> > Do failure handling, not success handling.
> >
> > People always think they should get creative with the last if statement
> > in a function.  This leads to spaghetti code and it's confusing.  Please
> > never do this again.
> >
> > The original is correct and the new code is bad rubbish code.
> >
> > regards,
> > dan carpenter
> >
> >
>
> Oops, my bad sir. Will keep this in mind.

Still, does the mutex_unlock really need to be duplicated?

julia

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

[PATCH v2 11/17] lpfc: Fix System panic after loading the driver

[James Smart]

System panic with general protection fault during driver load

The driver uses a static array sli4_hba.handler_name
to store the irq handler names. If the io_channel_irqs
exceeds the pre-allocated size (32+1), then the driver
will overwrite other fields of sli4_hba.

Fix: Dynamically allocate handler_name.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_init.c | 11 ++-
 drivers/scsi/lpfc/lpfc_sli4.h |  4 ++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 3064f0768033..a825806036c3 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -9665,6 +9665,7 @@ static int
 lpfc_sli4_enable_msix(struct lpfc_hba *phba)
 {
int vectors, rc, index;
+   char *name;
 
/* Set up MSI-X multi-message vectors */
vectors = phba->io_channel_irqs;
@@ -9683,9 +9684,9 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba)
 
/* Assign MSI-X vectors to interrupt handlers */
for (index = 0; index < vectors; index++) {
-   memset(>sli4_hba.handler_name[index], 0, 16);
-   snprintf((char *)>sli4_hba.handler_name[index],
-LPFC_SLI4_HANDLER_NAME_SZ,
+   name = phba->sli4_hba.hba_eq_hdl[index].handler_name;
+   memset(name, 0, LPFC_SLI4_HANDLER_NAME_SZ);
+   snprintf(name, LPFC_SLI4_HANDLER_NAME_SZ,
 LPFC_DRIVER_HANDLER_NAME"%d", index);
 
phba->sli4_hba.hba_eq_hdl[index].idx = index;
@@ -9694,12 +9695,12 @@ lpfc_sli4_enable_msix(struct lpfc_hba *phba)
if (phba->cfg_fof && (index == (vectors - 1)))
rc = request_irq(pci_irq_vector(phba->pcidev, index),
 _sli4_fof_intr_handler, 0,
-(char *)>sli4_hba.handler_name[index],
+name,
 >sli4_hba.hba_eq_hdl[index]);
else
rc = request_irq(pci_irq_vector(phba->pcidev, index),
 _sli4_hba_intr_handler, 0,
-(char *)>sli4_hba.handler_name[index],
+name,
 >sli4_hba.hba_eq_hdl[index]);
if (rc) {
lpfc_printf_log(phba, KERN_WARNING, LOG_INIT,
diff --git a/drivers/scsi/lpfc/lpfc_sli4.h b/drivers/scsi/lpfc/lpfc_sli4.h
index cf863db27700..28b75e08e044 100644
--- a/drivers/scsi/lpfc/lpfc_sli4.h
+++ b/drivers/scsi/lpfc/lpfc_sli4.h
@@ -407,8 +407,10 @@ struct lpfc_max_cfg_param {
 
 struct lpfc_hba;
 /* SLI4 HBA multi-fcp queue handler struct */
+#define LPFC_SLI4_HANDLER_NAME_SZ  16
 struct lpfc_hba_eq_hdl {
uint32_t idx;
+   char handler_name[LPFC_SLI4_HANDLER_NAME_SZ];
struct lpfc_hba *phba;
atomic_t hba_eq_in_use;
struct cpumask *cpumask;
@@ -480,7 +482,6 @@ struct lpfc_sli4_lnk_info {
 
 #define LPFC_SLI4_HANDLER_CNT  (LPFC_HBA_IO_CHAN_MAX+ \
 LPFC_FOF_IO_CHAN_NUM)
-#define LPFC_SLI4_HANDLER_NAME_SZ  16
 
 /* Used for IRQ vector to CPU mapping */
 struct lpfc_vector_map_info {
@@ -548,7 +549,6 @@ struct lpfc_sli4_hba {
uint32_t ue_to_rp;
struct lpfc_register sli_intf;
struct lpfc_pc_sli4_params pc_sli4_params;
-   uint8_t handler_name[LPFC_SLI4_HANDLER_CNT][LPFC_SLI4_HANDLER_NAME_SZ];
struct lpfc_hba_eq_hdl *hba_eq_hdl; /* HBA per-WQ handle */
 
/* Pointers to the constructed SLI4 queues */
-- 
2.11.0

[PATCH v2 17/17] lpfc: update to revision to 11.4.0.0

[James Smart]

Set lpfc driver revision to 11.4.0.0

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_version.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_version.h b/drivers/scsi/lpfc/lpfc_version.h
index c2653244221c..067c9e8a4b2d 100644
--- a/drivers/scsi/lpfc/lpfc_version.h
+++ b/drivers/scsi/lpfc/lpfc_version.h
@@ -20,7 +20,7 @@
  * included with this package. *
  ***/
 
-#define LPFC_DRIVER_VERSION "11.2.0.14"
+#define LPFC_DRIVER_VERSION "11.4.0.0"
 #define LPFC_DRIVER_NAME   "lpfc"
 
 /* Used for SLI 2/3 */
-- 
2.11.0

[PATCH v2 15/17] lpfc: Fix defects reported by Coverity Scan

[James Smart]

Addressed the following reported defects:

** CID 1411552:  Control flow issues  (MISSING_BREAK)
/drivers/scsi/lpfc/lpfc_sli.c: 13259 in lpfc_sli4_nvmet_handle_rcqe()

** CID 1411553:  Memory - illegal accesses  (OVERRUN)
/drivers/scsi/lpfc/lpfc_sli.c: 16218 in lpfc_fc_frame_check()

** CID 1411553:  Memory - illegal accesses  (OVERRUN)
   Overrunning array "lpfc_rctl_names" of 202 8-byte elements at element
   index 244 (byte offset 1952) using index "fc_hdr->fh_r_ctl" (which
   evaluates to 244).

** CID 1411554:  Null pointer dereferences  (REVERSE_INULL)
/drivers/scsi/lpfc/lpfc_nvmet.c: 2131 in lpfc_nvmet_unsol_fcp_abort_cmp()

** CID 1411555:  Memory - illegal accesses  (UNINIT)
/drivers/scsi/lpfc/lpfc_nvmet.c: 180 in lpfc_nvmet_ctxbuf_post()

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_nvmet.c | 10 --
 drivers/scsi/lpfc/lpfc_sli.c   | 24 +---
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index 9d8c4f07a2be..64908bef6f11 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -170,7 +170,6 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct 
lpfc_nvmet_ctxbuf *ctx_buf)
struct lpfc_nvmet_tgtport *tgtp;
struct fc_frame_header *fc_hdr;
struct rqb_dmabuf *nvmebuf;
-   struct lpfc_dmabuf *hbufp;
uint32_t *payload;
uint32_t size, oxid, sid, rc;
unsigned long iflag;
@@ -191,7 +190,6 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct 
lpfc_nvmet_ctxbuf *ctx_buf)
 
spin_lock_irqsave(>sli4_hba.nvmet_io_wait_lock, iflag);
if (phba->sli4_hba.nvmet_io_wait_cnt) {
-   hbufp = >hbuf;
list_remove_head(>sli4_hba.lpfc_nvmet_io_wait_list,
 nvmebuf, struct rqb_dmabuf,
 hbuf.list);
@@ -2165,10 +2163,6 @@ lpfc_nvmet_unsol_fcp_abort_cmp(struct lpfc_hba *phba, 
struct lpfc_iocbq *cmdwqe,
status = bf_get(lpfc_wcqe_c_status, wcqe);
result = wcqe->parameter;
 
-   tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private;
-   if (ctxp->flag & LPFC_NVMET_ABORT_OP)
-   atomic_inc(>xmt_fcp_abort_cmpl);
-
if (!ctxp) {
/* if context is clear, related io alrady complete */
lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS,
@@ -2178,6 +2172,10 @@ lpfc_nvmet_unsol_fcp_abort_cmp(struct lpfc_hba *phba, 
struct lpfc_iocbq *cmdwqe,
return;
}
 
+   tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private;
+   if (ctxp->flag & LPFC_NVMET_ABORT_OP)
+   atomic_inc(>xmt_fcp_abort_cmpl);
+
/* Sanity check */
if (ctxp->state != LPFC_NVMET_STE_ABORT) {
lpfc_printf_log(phba, KERN_ERR, LOG_NVME_ABTS,
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index fb4c708ae747..f60c9e3e37d7 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -13267,6 +13267,7 @@ lpfc_sli4_nvmet_handle_rcqe(struct lpfc_hba *phba, 
struct lpfc_queue *cq,
case FC_STATUS_RQ_BUF_LEN_EXCEEDED:
lpfc_printf_log(phba, KERN_ERR, LOG_SLI,
"6126 Receive Frame Truncated!!\n");
+   /* Drop thru */
case FC_STATUS_RQ_SUCCESS:
lpfc_sli4_rq_release(hrq, drq);
spin_lock_irqsave(>hbalock, iflags);
@@ -16137,9 +16138,6 @@ lpfc_sli4_post_scsi_sgl_block(struct lpfc_hba *phba,
return rc;
 }
 
-static char *lpfc_rctl_names[] = FC_RCTL_NAMES_INIT;
-static char *lpfc_type_names[] = FC_TYPE_NAMES_INIT;
-
 /**
  * lpfc_fc_frame_check - Check that this frame is a valid frame to handle
  * @phba: pointer to lpfc_hba struct that the frame was received on
@@ -16214,22 +16212,18 @@ lpfc_fc_frame_check(struct lpfc_hba *phba, struct 
fc_frame_header *fc_hdr)
}
 
lpfc_printf_log(phba, KERN_INFO, LOG_ELS,
-   "2538 Received frame rctl:%s (x%x), type:%s (x%x), "
+   "2538 Received frame rctl:x%x, type:x%x, "
"frame Data:%08x %08x %08x %08x %08x %08x %08x\n",
-   (fc_hdr->fh_r_ctl == FC_RCTL_MDS_DIAGS) ? "MDS Diags" :
-   lpfc_rctl_names[fc_hdr->fh_r_ctl], fc_hdr->fh_r_ctl,
-   (fc_hdr->fh_type == FC_TYPE_VENDOR_UNIQUE) ?
-   "Vendor Unique" : lpfc_type_names[fc_hdr->fh_type],
-   fc_hdr->fh_type, be32_to_cpu(header[0]),
-   be32_to_cpu(header[1]), be32_to_cpu(header[2]),
-   be32_to_cpu(header[3]), be32_to_cpu(header[4]),
-   be32_to_cpu(header[5]), be32_to_cpu(header[6]));
+   fc_hdr->fh_r_ctl, fc_hdr->fh_type,
+   

[PATCH v2 09/17] lpfc: Fix return value of board_mode store routine in case of online failure

[James Smart]

On hbacmd reset failure, observing wrong string "nline" in
kernel log.

On failure, non negative value (1) is returned from sysfs store
routine. It is interpreted as count by kernel and store routine
is called again with the remaining characters as input.

Fix: Return negative error code (-EIO) in case of failure.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_attr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index eb33473cbc62..8eee39de15f7 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -1351,6 +1351,8 @@ lpfc_board_mode_store(struct device *dev, struct 
device_attribute *attr,
goto board_mode_out;
}
wait_for_completion(_compl);
+   if (status)
+   status = -EIO;
} else if (strncmp(buf, "offline", sizeof("offline") - 1) == 0)
status = lpfc_do_offline(phba, LPFC_EVT_OFFLINE);
else if (strncmp(buf, "warm", sizeof("warm") - 1) == 0)
-- 
2.11.0

[PATCH v2 12/17] lpfc: Null pointer dereference when log_verbose is set to 0xffffffff

[James Smart]

Kernel panic when log_verbose is set to 0x

phba->pport is dereferenced before it is initialized

Fix: Do not dereference phba->pport if it is NULL

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_sli.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index c4ceef69bd6b..fb4c708ae747 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -7513,7 +7513,8 @@ lpfc_sli_issue_mbox_s3(struct lpfc_hba *phba, 
LPFC_MBOXQ_t *pmbox,
"(%d):0308 Mbox cmd issue - BUSY Data: "
"x%x x%x x%x x%x\n",
pmbox->vport ? pmbox->vport->vpi : 0xff,
-   mbx->mbxCommand, phba->pport->port_state,
+   mbx->mbxCommand,
+   phba->pport ? phba->pport->port_state : 0xff,
psli->sli_flag, flag);
 
psli->slistat.mbox_busy++;
@@ -7565,7 +7566,8 @@ lpfc_sli_issue_mbox_s3(struct lpfc_hba *phba, 
LPFC_MBOXQ_t *pmbox,
"(%d):0309 Mailbox cmd x%x issue Data: x%x x%x "
"x%x\n",
pmbox->vport ? pmbox->vport->vpi : 0,
-   mbx->mbxCommand, phba->pport->port_state,
+   mbx->mbxCommand,
+   phba->pport ? phba->pport->port_state : 0xff,
psli->sli_flag, flag);
 
if (mbx->mbxCommand != MBX_HEARTBEAT) {
-- 
2.11.0

[PATCH v2 16/17] lpfc: Add auto EQ delay logic

[James Smart]

Administrator intervention is currently required to get good numbers
when switching from running latency tests to IOPS tests.

The configured interrupt coalescing values will greatly effect
the results of these tests.  Currently, the driver has a single
coalescing value set by values of the module attribute.  This
patch changes the driver to support auto-configuration of the
coalescing value based on the total number of outstanding IOs
and average number of CQEs processed per interrupt for an EQ.
Values are checked every 5 seconds.

The driver defaults to the automatic selection. Automatic selection
can be disabled by the new lpfc_auto_imax module_parameter.

Older hardware can only change interrupt coalescing by mailbox
command. Newer hardware supports change via a register. The patch
support both.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc.h |   3 ++
 drivers/scsi/lpfc/lpfc_attr.c|  20 +++-
 drivers/scsi/lpfc/lpfc_debugfs.c |   4 +-
 drivers/scsi/lpfc/lpfc_hw4.h |  14 ++
 drivers/scsi/lpfc/lpfc_init.c| 104 ++-
 drivers/scsi/lpfc/lpfc_sli.c |  36 +++---
 drivers/scsi/lpfc/lpfc_sli.h |   1 +
 drivers/scsi/lpfc/lpfc_sli4.h|   8 +--
 8 files changed, 177 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index a9d73728a68c..562dc0139735 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -756,6 +756,7 @@ struct lpfc_hba {
uint8_t  nvmet_support; /* driver supports NVMET */
 #define LPFC_NVMET_MAX_PORTS   32
uint8_t  mds_diags_support;
+   uint32_t initial_imax;
 
/* HBA Config Parameters */
uint32_t cfg_ack0;
@@ -777,6 +778,7 @@ struct lpfc_hba {
uint32_t cfg_poll_tmo;
uint32_t cfg_task_mgmt_tmo;
uint32_t cfg_use_msi;
+   uint32_t cfg_auto_imax;
uint32_t cfg_fcp_imax;
uint32_t cfg_fcp_cpu_map;
uint32_t cfg_fcp_io_channel;
@@ -1050,6 +1052,7 @@ struct lpfc_hba {
 
uint8_t temp_sensor_support;
/* Fields used for heart beat. */
+   unsigned long last_eqdelay_time;
unsigned long last_completion_time;
unsigned long skipped_hb;
struct timer_list hb_tmofunc;
diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 8eee39de15f7..66269e342c7e 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -4481,9 +4481,11 @@ lpfc_fcp_imax_store(struct device *dev, struct 
device_attribute *attr,
return -EINVAL;
 
phba->cfg_fcp_imax = (uint32_t)val;
+   phba->initial_imax = phba->cfg_fcp_imax;
 
for (i = 0; i < phba->io_channel_irqs; i += LPFC_MAX_EQ_DELAY_EQID_CNT)
-   lpfc_modify_hba_eq_delay(phba, i);
+   lpfc_modify_hba_eq_delay(phba, i, LPFC_MAX_EQ_DELAY_EQID_CNT,
+val);
 
return strlen(buf);
 }
@@ -4538,6 +4540,16 @@ lpfc_fcp_imax_init(struct lpfc_hba *phba, int val)
 static DEVICE_ATTR(lpfc_fcp_imax, S_IRUGO | S_IWUSR,
   lpfc_fcp_imax_show, lpfc_fcp_imax_store);
 
+/*
+ * lpfc_auto_imax: Controls Auto-interrupt coalescing values support.
+ *   0   No auto_imax support
+ *   1   auto imax on
+ * Auto imax will change the value of fcp_imax on a per EQ basis, using
+ * the EQ Delay Multiplier, depending on the activity for that EQ.
+ * Value range [0,1]. Default value is 1.
+ */
+LPFC_ATTR_RW(auto_imax, 1, 0, 1, "Enable Auto imax");
+
 /**
  * lpfc_state_show - Display current driver CPU affinity
  * @dev: class converted to a Scsi_host structure.
@@ -5164,6 +5176,7 @@ struct device_attribute *lpfc_hba_attrs[] = {
_attr_lpfc_task_mgmt_tmo,
_attr_lpfc_use_msi,
_attr_lpfc_nvme_oas,
+   _attr_lpfc_auto_imax,
_attr_lpfc_fcp_imax,
_attr_lpfc_fcp_cpu_map,
_attr_lpfc_fcp_io_channel,
@@ -6182,6 +6195,7 @@ lpfc_get_cfgparam(struct lpfc_hba *phba)
lpfc_enable_SmartSAN_init(phba, lpfc_enable_SmartSAN);
lpfc_use_msi_init(phba, lpfc_use_msi);
lpfc_nvme_oas_init(phba, lpfc_nvme_oas);
+   lpfc_auto_imax_init(phba, lpfc_auto_imax);
lpfc_fcp_imax_init(phba, lpfc_fcp_imax);
lpfc_fcp_cpu_map_init(phba, lpfc_fcp_cpu_map);
lpfc_enable_hba_reset_init(phba, lpfc_enable_hba_reset);
@@ -6226,6 +6240,10 @@ lpfc_get_cfgparam(struct lpfc_hba *phba)
phba->cfg_enable_fc4_type |= LPFC_ENABLE_FCP;
}
 
+   if (phba->cfg_auto_imax && !phba->cfg_fcp_imax)
+   phba->cfg_auto_imax = 0;
+   phba->initial_imax = phba->cfg_fcp_imax;
+
/* A value of 0 means use the number of CPUs found in the system */
if (phba->cfg_fcp_io_channel == 0)
phba->cfg_fcp_io_channel = phba->sli4_hba.num_present_cpu;
diff --git 

[PATCH v2 10/17] lpfc: Fix crash on powering off BFS VM with passthrough device

[James Smart]

Null pointer dereference when BFS VM is powered off

The driver incorrectly uses sli3_ring on SLI-4 adapters

Use the correct ring structure based on sli_rev

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
Tested-by: Raphael Silva 
---
 drivers/scsi/lpfc/lpfc_sli.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index e81fa7d4deb5..c4ceef69bd6b 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -10951,6 +10951,7 @@ lpfc_sli_abort_iocb(struct lpfc_vport *vport, struct 
lpfc_sli_ring *pring,
struct lpfc_hba *phba = vport->phba;
struct lpfc_iocbq *iocbq;
struct lpfc_iocbq *abtsiocb;
+   struct lpfc_sli_ring *pring_s4;
IOCB_t *cmd = NULL;
int errcnt = 0, ret_val = 0;
int i;
@@ -11004,8 +11005,15 @@ lpfc_sli_abort_iocb(struct lpfc_vport *vport, struct 
lpfc_sli_ring *pring,
 
/* Setup callback routine and issue the command. */
abtsiocb->iocb_cmpl = lpfc_sli_abort_fcp_cmpl;
-   ret_val = lpfc_sli_issue_iocb(phba, pring->ringno,
- abtsiocb, 0);
+   if (phba->sli_rev == LPFC_SLI_REV4) {
+   pring_s4 = lpfc_sli4_calc_ring(phba, iocbq);
+   if (!pring_s4)
+   continue;
+   ret_val = lpfc_sli_issue_iocb(phba, pring_s4->ringno,
+ abtsiocb, 0);
+   } else
+   ret_val = lpfc_sli_issue_iocb(phba, pring->ringno,
+ abtsiocb, 0);
if (ret_val == IOCB_ERROR) {
lpfc_sli_release_iocbq(phba, abtsiocb);
errcnt++;
-- 
2.11.0

[PATCH v2 14/17] lpfc: Fix vports not logging into target

[James Smart]

vports cannot login to target.

For vports, lpfc_nodelist is allocated for targets only on
completion of GFF_ID command. Driver checks if lpfc_nodelist
exists for target before sending GFF_ID. So, GFF_ID and
PLOGI are not sent.

As mentioned by the comment in lpfc_prep_node_fc4type() routine,
do not send GFF_ID only if this NPortID is previously identified
as FCP target. Send GFF_ID if it is a newly identified remote
port from GID_FT response.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_ct.c | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
index 24ce96dcc94d..9c0c1463057d 100644
--- a/drivers/scsi/lpfc/lpfc_ct.c
+++ b/drivers/scsi/lpfc/lpfc_ct.c
@@ -503,26 +503,23 @@ lpfc_prep_node_fc4type(struct lpfc_vport *vport, uint32_t 
Did, uint8_t fc4_type)
Did, vport->fc_flag, vport->fc_rscn_id_cnt);
 
/*
-* This NPortID was previously a FCP target,
+* This NPortID was previously a FCP/NVMe target,
 * Don't even bother to send GFF_ID.
 */
ndlp = lpfc_findnode_did(vport, Did);
-   if (ndlp && NLP_CHK_NODE_ACT(ndlp))
-   ndlp->nlp_fc4_type = fc4_type;
-
-   if (ndlp && NLP_CHK_NODE_ACT(ndlp)) {
-   ndlp->nlp_fc4_type = fc4_type;
-
-   if (ndlp->nlp_type & NLP_FCP_TARGET)
-   lpfc_setup_disc_node(vport, Did);
-
-   else if (lpfc_ns_cmd(vport, SLI_CTNS_GFF_ID,
-   0, Did) == 0)
-   vport->num_disc_nodes++;
-
-   else
-   lpfc_setup_disc_node(vport, Did);
-   }
+   if (ndlp && NLP_CHK_NODE_ACT(ndlp) &&
+   (ndlp->nlp_type &
+   (NLP_FCP_TARGET | NLP_NVME_TARGET))) {
+   if (fc4_type == FC_TYPE_FCP)
+   ndlp->nlp_fc4_type |= NLP_FC4_FCP;
+   if (fc4_type == FC_TYPE_NVME)
+   ndlp->nlp_fc4_type |= NLP_FC4_NVME;
+   lpfc_setup_disc_node(vport, Did);
+   } else if (lpfc_ns_cmd(vport, SLI_CTNS_GFF_ID,
+  0, Did) == 0)
+   vport->num_disc_nodes++;
+   else
+   lpfc_setup_disc_node(vport, Did);
} else {
lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_CT,
"Skip2 GID_FTrsp: did:x%x flg:x%x cnt:%d",
-- 
2.11.0

[PATCH v2 13/17] lpfc: Fix PRLI retry handling when target rejects it.

[James Smart]

The nvmet driver was rejecting the initiator's PRLI because
its reg_rpi for the PLOGI was still outstanding.  The initiator
would resend the PRLI without delay and get the same answer.
The PRLI retries would exhaust causing the nvme initiator to
set the nvmet ndlp to UNMAPPED.

The driver's lpfc_els_retry handler did not have a policy for an
LS_RJT with explanation CMD_IN_PROGRESS for PRLI or NVME_PRLI.
This caused the delay to remain at 0 but retry set 1.

Fix: When the ELS response is LS_RJT, TPC and the command was PRLI
or NVME_PRLI, just set the delay to 1000 mS to get a 1 second
delay on the PRLI retry.  This was enough to allow the REG_RPI to
complete at the target.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_els.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 8e532b39ae93..a140318d6159 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -3332,6 +3332,19 @@ lpfc_els_retry(struct lpfc_hba *phba, struct lpfc_iocbq 
*cmdiocb,
 */
switch (stat.un.b.lsRjtRsnCode) {
case LSRJT_UNABLE_TPC:
+   /* The driver has a VALID PLOGI but the rport has
+* rejected the PRLI - can't do it now.  Delay
+* for 1 second and try again - don't care about
+* the explanation.
+*/
+   if (cmd == ELS_CMD_PRLI || cmd == ELS_CMD_NVMEPRLI) {
+   delay = 1000;
+   maxretry = lpfc_max_els_tries + 1;
+   retry = 1;
+   break;
+   }
+
+   /* Legacy bug fix code for targets with PLOGI delays. */
if (stat.un.b.lsRjtRsnCodeExp ==
LSEXP_CMD_IN_PROGRESS) {
if (cmd == ELS_CMD_PLOGI) {
@@ -3350,9 +3363,7 @@ lpfc_els_retry(struct lpfc_hba *phba, struct lpfc_iocbq 
*cmdiocb,
retry = 1;
break;
}
-   if ((cmd == ELS_CMD_PLOGI) ||
-   (cmd == ELS_CMD_PRLI) ||
-   (cmd == ELS_CMD_NVMEPRLI)) {
+   if (cmd == ELS_CMD_PLOGI) {
delay = 1000;
maxretry = lpfc_max_els_tries + 1;
retry = 1;
-- 
2.11.0

[PATCH v2 08/17] lpfc: Fix counters so outstandng NVME IO count is accurate

[James Smart]

NVME FC counters don't reflect actual results

Since counters are not atomic, or protected by a lock,
the values often get screwed up.

Make them atomic, like NVMET.
Fix up sysfs and debugfs display accordingly
Added Outstanding IOs to stats display

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc.h | 20 ++--
 drivers/scsi/lpfc/lpfc_attr.c| 32 +---
 drivers/scsi/lpfc/lpfc_debugfs.c | 30 +-
 drivers/scsi/lpfc/lpfc_init.c| 10 ++
 drivers/scsi/lpfc/lpfc_nvme.c| 19 +--
 drivers/scsi/lpfc/lpfc_scsi.c| 19 ++-
 6 files changed, 89 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index f2c0ba6ced78..a9d73728a68c 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -913,16 +913,16 @@ struct lpfc_hba {
/*
 * stat  counters
 */
-   uint64_t fc4ScsiInputRequests;
-   uint64_t fc4ScsiOutputRequests;
-   uint64_t fc4ScsiControlRequests;
-   uint64_t fc4ScsiIoCmpls;
-   uint64_t fc4NvmeInputRequests;
-   uint64_t fc4NvmeOutputRequests;
-   uint64_t fc4NvmeControlRequests;
-   uint64_t fc4NvmeIoCmpls;
-   uint64_t fc4NvmeLsRequests;
-   uint64_t fc4NvmeLsCmpls;
+   atomic_t fc4ScsiInputRequests;
+   atomic_t fc4ScsiOutputRequests;
+   atomic_t fc4ScsiControlRequests;
+   atomic_t fc4ScsiIoCmpls;
+   atomic_t fc4NvmeInputRequests;
+   atomic_t fc4NvmeOutputRequests;
+   atomic_t fc4NvmeControlRequests;
+   atomic_t fc4NvmeIoCmpls;
+   atomic_t fc4NvmeLsRequests;
+   atomic_t fc4NvmeLsCmpls;
 
uint64_t bg_guard_err_cnt;
uint64_t bg_apptag_err_cnt;
diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 200a614bb540..eb33473cbc62 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -150,6 +150,7 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
struct nvme_fc_local_port *localport;
struct lpfc_nodelist *ndlp;
struct nvme_fc_remote_port *nrport;
+   uint64_t data1, data2, data3, tot;
char *statep;
int len = 0;
 
@@ -244,11 +245,18 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
atomic_read(>xmt_abort_rsp),
atomic_read(>xmt_abort_rsp_error));
 
+   spin_lock(>sli4_hba.nvmet_io_lock);
+   tot = phba->sli4_hba.nvmet_xri_cnt -
+   phba->sli4_hba.nvmet_ctx_cnt;
+   spin_unlock(>sli4_hba.nvmet_io_lock);
+
len += snprintf(buf + len, PAGE_SIZE - len,
-   "IO_CTX: %08x outstanding %08x total %x",
+   "IO_CTX: %08x  WAIT: cur %08x tot %08x\n"
+   "CTX Outstanding %08llx\n",
phba->sli4_hba.nvmet_ctx_cnt,
phba->sli4_hba.nvmet_io_wait_cnt,
-   phba->sli4_hba.nvmet_io_wait_total);
+   phba->sli4_hba.nvmet_io_wait_total,
+   tot);
 
len +=  snprintf(buf+len, PAGE_SIZE-len, "\n");
return len;
@@ -337,19 +345,21 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
 
len += snprintf(buf + len, PAGE_SIZE - len, "\nNVME Statistics\n");
len += snprintf(buf+len, PAGE_SIZE-len,
-   "LS: Xmt %016llx Cmpl %016llx\n",
-   phba->fc4NvmeLsRequests,
-   phba->fc4NvmeLsCmpls);
-
+   "LS: Xmt %016x Cmpl %016x\n",
+   atomic_read(>fc4NvmeLsRequests),
+   atomic_read(>fc4NvmeLsCmpls));
+
+   tot = atomic_read(>fc4NvmeIoCmpls);
+   data1 = atomic_read(>fc4NvmeInputRequests);
+   data2 = atomic_read(>fc4NvmeOutputRequests);
+   data3 = atomic_read(>fc4NvmeControlRequests);
len += snprintf(buf+len, PAGE_SIZE-len,
"FCP: Rd %016llx Wr %016llx IO %016llx\n",
-   phba->fc4NvmeInputRequests,
-   phba->fc4NvmeOutputRequests,
-   phba->fc4NvmeControlRequests);
+   data1, data2, data3);
 
len += snprintf(buf+len, PAGE_SIZE-len,
-   "Cmpl %016llx\n", phba->fc4NvmeIoCmpls);
-
+   "Cmpl %016llx Outstanding %016llx\n",
+   tot, (data1 + data2 + data3) - tot);
return len;
 }
 
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index dd8598bc50f3..6089690fb345 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ 

[PATCH v2 06/17] lpfc: Fix nvmet node ref count handling

[James Smart]

When unloading the driver, the NVMET driver would wait
the full 30 seconds for its UNMAPPED initiator node to
get removed before continuing with the unload process.
NVMEI worked correctly.

For each rport put into UNMAPPED or MAPPED state by NVMET,
the driver puts a reference on the NDLP.  The difference is
that NVMEI has a unregister call for its rports and the extra
reference is removed in the unregister process.  For NVMET,
the driver has to remove the reference explicitly when dropping
out of UNMAPPED or MAPPED because there is no unregister call.

Add a call to lpfc_nlp_put on the ndlp when NVMET and the old
state was UNMAPPED or MAPPED.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 055fedd761ea..db2d0e692ddf 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -4167,14 +4167,14 @@ lpfc_nlp_state_cleanup(struct lpfc_vport *vport, struct 
lpfc_nodelist *ndlp,
lpfc_unregister_remote_port(ndlp);
}
 
-   /* Notify the NVME transport of this rport's loss on the
-* Initiator.  For NVME Target, should upcall transport
-* in the else clause when API available.
-*/
if (ndlp->nlp_fc4_type & NLP_FC4_NVME) {
vport->phba->nport_event_cnt++;
if (vport->phba->nvmet_support == 0)
+   /* Start devloss */
lpfc_nvme_unregister_port(vport, ndlp);
+   else
+   /* NVMET has no upcall. */
+   lpfc_nlp_put(ndlp);
}
}
 
-- 
2.11.0

[PATCH v2 01/17] lpfc: Add nvme initiator devloss support

[James Smart]

Add nvme initiator devloss support

The existing implementation was based on no devloss behavior
in the transport (e.g. immediate teardown) so code didn't
properly handle delayed nvme rport device unregister calls.
In addition, the driver was not correctly cycling the rport
port role for each register-unregister-reregister process.

This patch does the following:
Rework the code to properly handle rport device unregister
calls and potential re-allocation of the remoteport structure
if the port comes back in under dev_loss_tmo.

Correct code that was incorrectly cycling the rport
port role for each register-unregister-reregister process.

Prep the code to enable calling the nvme_fc transport api
to dynamically update dev_loss_tmo when the scsi sysfs interface
changes it.

Memset the rpinfo structure in the registration call to enforce
"accept nvme transport defaults" in the registration call.  Driver
parameters do influence the dev_loss_tmo transport setting
dynamically.

Simplifies the register function: the driver was incorrectly
searching its local rport list to determine resume or new semantics,
which is not valid as the transport already handles this.  The rport
was resumed if the rport handed back matches the ndlp->nrport pointer.
Otherwise, devloss fired and the ndlp's nrport is NULL.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_attr.c |   7 ++-
 drivers/scsi/lpfc/lpfc_nvme.c | 141 --
 2 files changed, 57 insertions(+), 91 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index bb2d9e238225..37f258fcf6d4 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -3198,9 +3198,12 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)
 
shost = lpfc_shost_from_vport(vport);
spin_lock_irq(shost->host_lock);
-   list_for_each_entry(ndlp, >fc_nodes, nlp_listp)
-   if (NLP_CHK_NODE_ACT(ndlp) && ndlp->rport)
+   list_for_each_entry(ndlp, >fc_nodes, nlp_listp) {
+   if (!NLP_CHK_NODE_ACT(ndlp))
+   continue;
+   if (ndlp->rport)
ndlp->rport->dev_loss_tmo = vport->cfg_devloss_tmo;
+   }
spin_unlock_irq(shost->host_lock);
 }
 
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 8008c8205fb6..70675fd7d884 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -186,13 +186,13 @@ lpfc_nvme_remoteport_delete(struct nvme_fc_remote_port 
*remoteport)
 
/* Remove this rport from the lport's list - memory is owned by the
 * transport. Remove the ndlp reference for the NVME transport before
-* calling state machine to remove the node, this is devloss = 0
-* semantics.
+* calling state machine to remove the node.
 */
lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_DISC,
"6146 remoteport delete complete %p\n",
remoteport);
list_del(>list);
+   ndlp->nrport = NULL;
lpfc_nlp_put(ndlp);
 
  rport_err:
@@ -1466,7 +1466,7 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port 
*pnvme_lport,
 
/* The remote node has to be ready to send an abort. */
if ((ndlp->nlp_state != NLP_STE_MAPPED_NODE) &&
-   !(ndlp->nlp_type & NLP_NVME_TARGET)) {
+   (ndlp->nlp_type & NLP_NVME_TARGET)) {
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
 "6048 rport %p, DID x%06x not ready for "
 "IO. State x%x, Type x%x\n",
@@ -2340,69 +2340,44 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, 
struct lpfc_nodelist *ndlp)
localport = vport->localport;
lport = (struct lpfc_nvme_lport *)localport->private;
 
-   if (ndlp->nlp_type & (NLP_NVME_TARGET | NLP_NVME_INITIATOR)) {
-
-   /* The driver isn't expecting the rport wwn to change
-* but it might get a different DID on a different
-* fabric.
+   /* NVME rports are not preserved across devloss.
+* Just register this instance.  Note, rpinfo->dev_loss_tmo
+* is left 0 to indicate accept transport defaults.  The
+* driver communicates port role capabilities consistent
+* with the PRLI response data.
+*/
+   memset(, 0, sizeof(struct nvme_fc_port_info));
+   rpinfo.port_id = ndlp->nlp_DID;
+   if (ndlp->nlp_type & NLP_NVME_TARGET)
+   rpinfo.port_role |= FC_PORT_ROLE_NVME_TARGET;
+   if (ndlp->nlp_type & NLP_NVME_INITIATOR)
+   rpinfo.port_role |= FC_PORT_ROLE_NVME_INITIATOR;
+
+   if (ndlp->nlp_type & NLP_NVME_DISCOVERY)
+   rpinfo.port_role |= FC_PORT_ROLE_NVME_DISCOVERY;
+
+   rpinfo.port_name = wwn_to_u64(ndlp->nlp_portname.u.wwn);
+   

[PATCH v2 03/17] lpfc: Fix nvme port role handling in sysfs and debugfs handlers.

[James Smart]

While debugging Devloss and recovery, debugfs and sysfs
were found to not show the NVME port roles consistently.

The port role FC_PORT_ROLE_NVME_DISCOVERY was added with
the devloss bringup and the other issues were just oversight.

Add NVME Target and DISCSRVC to debugfs nodeinfo and sysfs
nvme info handlers. The full port role was added to the NVME
data only not the generic nodelist.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_attr.c| 22 ++
 drivers/scsi/lpfc/lpfc_debugfs.c | 32 ++--
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 6d9b83cd82a2..200a614bb540 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -312,25 +312,23 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
len += snprintf(buf + len, PAGE_SIZE - len, "DID x%06x ",
nrport->port_id);
 
-   switch (nrport->port_role) {
-   case FC_PORT_ROLE_NVME_INITIATOR:
+   /* An NVME rport can have multiple roles. */
+   if (nrport->port_role & FC_PORT_ROLE_NVME_INITIATOR)
len +=  snprintf(buf + len, PAGE_SIZE - len,
 "INITIATOR ");
-   break;
-   case FC_PORT_ROLE_NVME_TARGET:
+   if (nrport->port_role & FC_PORT_ROLE_NVME_TARGET)
len +=  snprintf(buf + len, PAGE_SIZE - len,
 "TARGET ");
-   break;
-   case FC_PORT_ROLE_NVME_DISCOVERY:
+   if (nrport->port_role & FC_PORT_ROLE_NVME_DISCOVERY)
len +=  snprintf(buf + len, PAGE_SIZE - len,
-"DISCOVERY ");
-   break;
-   default:
+"DISCSRVC ");
+   if (nrport->port_role & ~(FC_PORT_ROLE_NVME_INITIATOR |
+ FC_PORT_ROLE_NVME_TARGET |
+ FC_PORT_ROLE_NVME_DISCOVERY))
len +=  snprintf(buf + len, PAGE_SIZE - len,
-"UNKNOWN_ROLE x%x",
+"UNKNOWN ROLE x%x",
 nrport->port_role);
-   break;
-   }
+
len +=  snprintf(buf + len, PAGE_SIZE - len, "%s  ", statep);
/* Terminate the string. */
len +=  snprintf(buf + len, PAGE_SIZE - len, "\n");
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index d71bda24b08e..dd8598bc50f3 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -621,6 +621,13 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char 
*buf, int size)
ndlp->nlp_sid);
if (ndlp->nlp_type & NLP_FCP_INITIATOR)
len += snprintf(buf+len, size-len, "FCP_INITIATOR ");
+   if (ndlp->nlp_type & NLP_NVME_TARGET)
+   len += snprintf(buf + len,
+   size - len, "NVME_TGT sid:%d ",
+   NLP_NO_SID);
+   if (ndlp->nlp_type & NLP_NVME_INITIATOR)
+   len += snprintf(buf + len,
+   size - len, "NVME_INITIATOR ");
len += snprintf(buf+len, size-len, "usgmap:%x ",
ndlp->nlp_usg_map);
len += snprintf(buf+len, size-len, "refcnt:%x",
@@ -698,26 +705,23 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char 
*buf, int size)
nrport->port_name);
len += snprintf(buf + len, size - len, "WWNN x%llx ",
nrport->node_name);
-   switch (nrport->port_role) {
-   case FC_PORT_ROLE_NVME_INITIATOR:
+
+   /* An NVME rport can have multiple roles. */
+   if (nrport->port_role & FC_PORT_ROLE_NVME_INITIATOR)
len +=  snprintf(buf + len, size - len,
-"NVME INITIATOR ");
-   break;
-   case FC_PORT_ROLE_NVME_TARGET:
+"INITIATOR ");
+   if (nrport->port_role & FC_PORT_ROLE_NVME_TARGET)
len +=  snprintf(buf + len, size - len,
-"NVME TARGET ");
-   break;
-   case FC_PORT_ROLE_NVME_DISCOVERY:
+"TARGET ");
+   if (nrport->port_role & 

[PATCH v2 05/17] lpfc: Fix Lun Priority level shown as NA

[James Smart]

Lun Priority level shown as NA

Remote port is not getting registered for nameserver and fdmi.
Due to which dfc SendCTPassThru cmd is failing.

Made changes to register the remote port for both.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 3ffcd9215ca8..055fedd761ea 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -4182,8 +4182,10 @@ lpfc_nlp_state_cleanup(struct lpfc_vport *vport, struct 
lpfc_nodelist *ndlp,
 
if (new_state ==  NLP_STE_MAPPED_NODE ||
new_state == NLP_STE_UNMAPPED_NODE) {
-   if ((ndlp->nlp_fc4_type & NLP_FC4_FCP) ||
-   (ndlp->nlp_DID == Fabric_DID)) {
+   if (ndlp->nlp_fc4_type & NLP_FC4_FCP ||
+   ndlp->nlp_DID == Fabric_DID ||
+   ndlp->nlp_DID == NameServer_DID ||
+   ndlp->nlp_DID == FDMI_DID) {
vport->phba->nport_event_cnt++;
/*
 * Tell the fc transport about the port, if we haven't
-- 
2.11.0

[PATCH v2 07/17] lpfc: Fix Port going offline after multiple resets.

[James Smart]

Observing lpfc port down after issuing hbacmd reset command

Failure in posting SGL buffers. If there is only one SGL buffer
and rrq is valid for its XRI, we are rightly returning NULL but
not adding the buffer back to the SGL list. So, number of buffers
become less than total count and repost fails during reset.

Add SGL buffer back to list before returning NULL.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_sli.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index d6b184839bc2..e81fa7d4deb5 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -968,6 +968,7 @@ __lpfc_sli_get_els_sglq(struct lpfc_hba *phba, struct 
lpfc_iocbq *piocbq)
list_remove_head(lpfc_els_sgl_list, sglq,
struct lpfc_sglq, list);
if (sglq == start_sglq) {
+   list_add_tail(>list, lpfc_els_sgl_list);
sglq = NULL;
break;
} else
-- 
2.11.0

[PATCH v2 04/17] lpfc: Add changes to assist in NVMET debugging

[James Smart]

Inconsistent error messages and context state checks

Context state sanity checks were not accurate or
inconsistent in the code paths.

Separated LS context states from FCP.
Added and modified context state sanity checks.
Use context state to determine if a sol or unsol ABORT is needed.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_nvmet.c | 208 -
 drivers/scsi/lpfc/lpfc_nvmet.h |  14 +--
 2 files changed, 151 insertions(+), 71 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index 24d54dd016d4..9d8c4f07a2be 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -112,6 +112,15 @@ lpfc_nvmet_xmt_ls_rsp_cmp(struct lpfc_hba *phba, struct 
lpfc_iocbq *cmdwqe,
 
status = bf_get(lpfc_wcqe_c_status, wcqe);
result = wcqe->parameter;
+   ctxp = cmdwqe->context2;
+
+   if (ctxp->state != LPFC_NVMET_STE_LS_RSP || ctxp->entry_cnt != 2) {
+   lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR,
+   "6410 NVMET LS cmpl state mismatch IO x%x: "
+   "%d %d\n",
+   ctxp->oxid, ctxp->state, ctxp->entry_cnt);
+   }
+
if (!phba->targetport)
goto out;
 
@@ -123,15 +132,14 @@ lpfc_nvmet_xmt_ls_rsp_cmp(struct lpfc_hba *phba, struct 
lpfc_iocbq *cmdwqe,
atomic_inc(>xmt_ls_rsp_cmpl);
 
 out:
-   ctxp = cmdwqe->context2;
rsp = >ctx.ls_req;
 
lpfc_nvmeio_data(phba, "NVMET LS  CMPL: xri x%x stat x%x result x%x\n",
 ctxp->oxid, status, result);
 
lpfc_printf_log(phba, KERN_INFO, LOG_NVME_DISC,
-   "6038 %s: Entrypoint: ctx %p status %x/%x\n", __func__,
-   ctxp, status, result);
+   "6038 NVMET LS rsp cmpl: %d %d oxid x%x\n",
+   status, result, ctxp->oxid);
 
lpfc_nlp_put(cmdwqe->context1);
cmdwqe->context2 = NULL;
@@ -173,6 +181,12 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct 
lpfc_nvmet_ctxbuf *ctx_buf)
ctxp->txrdy = NULL;
ctxp->txrdy_phys = 0;
}
+
+   if (ctxp->state == LPFC_NVMET_STE_FREE) {
+   lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR,
+   "6411 NVMET free, already free IO x%x: %d %d\n",
+   ctxp->oxid, ctxp->state, ctxp->entry_cnt);
+   }
ctxp->state = LPFC_NVMET_STE_FREE;
 
spin_lock_irqsave(>sli4_hba.nvmet_io_wait_lock, iflag);
@@ -580,8 +594,17 @@ lpfc_nvmet_xmt_ls_rsp(struct nvmet_fc_target_port *tgtport,
int rc;
 
lpfc_printf_log(phba, KERN_INFO, LOG_NVME_DISC,
-   "6023 %s: Entrypoint ctx %p %p\n", __func__,
-   ctxp, tgtport);
+   "6023 NVMET LS rsp oxid x%x\n", ctxp->oxid);
+
+   if ((ctxp->state != LPFC_NVMET_STE_LS_RCV) ||
+   (ctxp->entry_cnt != 1)) {
+   lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR,
+   "6412 NVMET LS rsp state mismatch "
+   "oxid x%x: %d %d\n",
+   ctxp->oxid, ctxp->state, ctxp->entry_cnt);
+   }
+   ctxp->state = LPFC_NVMET_STE_LS_RSP;
+   ctxp->entry_cnt++;
 
nvmewqeq = lpfc_nvmet_prep_ls_wqe(phba, ctxp, rsp->rspdma,
  rsp->rsplen);
@@ -751,15 +774,14 @@ lpfc_nvmet_xmt_fcp_abort(struct nvmet_fc_target_port 
*tgtport,
unsigned long flags;
 
lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS,
-   "6103 Abort op: oxri x%x flg x%x cnt %d\n",
-   ctxp->oxid, ctxp->flag, ctxp->entry_cnt);
+   "6103 NVMET Abort op: oxri x%x flg x%x ste %d\n",
+   ctxp->oxid, ctxp->flag, ctxp->state);
 
-   lpfc_nvmeio_data(phba, "NVMET FCP ABRT: "
-"xri x%x flg x%x cnt x%x\n",
-ctxp->oxid, ctxp->flag, ctxp->entry_cnt);
+   lpfc_nvmeio_data(phba, "NVMET FCP ABRT: xri x%x flg x%x ste x%x\n",
+ctxp->oxid, ctxp->flag, ctxp->state);
 
atomic_inc(_nvmep->xmt_fcp_abort);
-   ctxp->entry_cnt++;
+
spin_lock_irqsave(>ctxlock, flags);
 
/* Since iaab/iaar are NOT set, we need to check
@@ -770,12 +792,17 @@ lpfc_nvmet_xmt_fcp_abort(struct nvmet_fc_target_port 
*tgtport,
return;
}
ctxp->flag |= LPFC_NVMET_ABORT_OP;
-   if (ctxp->flag & LPFC_NVMET_IO_INP)
-   lpfc_nvmet_sol_fcp_issue_abort(phba, ctxp, ctxp->sid,
-  ctxp->oxid);
-   else
+
+   /* An state of LPFC_NVMET_STE_RCV means we have just received
+* the NVME command and have not started 

[PATCH v2 02/17] lpfc: Fix transition nvme-i rport handling to nport only.

[James Smart]

As the devloss API was implemented in the
nvmei driver, an evaluation of the nvme transport
and the lpfc driver showed dual management of the
rports.  This creates a bug possibility when the
thread count and SAN size increases.

The nvmei driver code was based on a very
early transport and was not revisited until the
devloss API was introduced.

Remove the listhead in the driver's rport
data structure and the listhead in the driver's
lport data structure.  Remove all rport_list
traversal.  Convert the driver to use the
nrport (nvme rport) pointer that is now NULL
or nonNULL depending on a devloss action.  Convert
debugfs and nvme_info in sysfs to use the fc_nodes
list in the vport.

Signed-off-by: Dick Kennedy 
Signed-off-by: James Smart 
---
 drivers/scsi/lpfc/lpfc_attr.c| 11 ++-
 drivers/scsi/lpfc/lpfc_debugfs.c | 10 +-
 drivers/scsi/lpfc/lpfc_nvme.c| 18 --
 drivers/scsi/lpfc/lpfc_nvme.h|  2 --
 4 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 37f258fcf6d4..6d9b83cd82a2 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -148,8 +148,7 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
struct lpfc_hba   *phba = vport->phba;
struct lpfc_nvmet_tgtport *tgtp;
struct nvme_fc_local_port *localport;
-   struct lpfc_nvme_lport *lport;
-   struct lpfc_nvme_rport *rport;
+   struct lpfc_nodelist *ndlp;
struct nvme_fc_remote_port *nrport;
char *statep;
int len = 0;
@@ -265,7 +264,6 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
len = snprintf(buf, PAGE_SIZE, "NVME Initiator Enabled\n");
 
spin_lock_irq(shost->host_lock);
-   lport = (struct lpfc_nvme_lport *)localport->private;
 
/* Port state is only one of two values for now. */
if (localport->port_id)
@@ -281,9 +279,12 @@ lpfc_nvme_info_show(struct device *dev, struct 
device_attribute *attr,
wwn_to_u64(vport->fc_nodename.u.wwn),
localport->port_id, statep);
 
-   list_for_each_entry(rport, >rport_list, list) {
+   list_for_each_entry(ndlp, >fc_nodes, nlp_listp) {
+   if (!ndlp->nrport)
+   continue;
+
/* local short-hand pointer. */
-   nrport = rport->remoteport;
+   nrport = ndlp->nrport->remoteport;
 
/* Port state is only one of two values for now. */
switch (nrport->port_state) {
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 4bcb92c844ca..d71bda24b08e 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -550,8 +550,6 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char 
*buf, int size)
struct lpfc_nodelist *ndlp;
unsigned char *statep;
struct nvme_fc_local_port *localport;
-   struct lpfc_nvme_lport *lport;
-   struct lpfc_nvme_rport *rport;
struct lpfc_nvmet_tgtport *tgtp;
struct nvme_fc_remote_port *nrport;
 
@@ -660,7 +658,6 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char 
*buf, int size)
goto out_exit;
 
spin_lock_irq(shost->host_lock);
-   lport = (struct lpfc_nvme_lport *)localport->private;
 
/* Port state is only one of two values for now. */
if (localport->port_id)
@@ -673,9 +670,12 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char 
*buf, int size)
localport->port_id, statep);
 
len += snprintf(buf + len, size - len, "\tRport List:\n");
-   list_for_each_entry(rport, >rport_list, list) {
+   list_for_each_entry(ndlp, >fc_nodes, nlp_listp) {
/* local short-hand pointer. */
-   nrport = rport->remoteport;
+   if (!ndlp->nrport)
+   continue;
+
+   nrport = ndlp->nrport->remoteport;
 
/* Port state is only one of two values for now. */
switch (nrport->port_state) {
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 70675fd7d884..ede42411e57b 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -191,7 +191,6 @@ lpfc_nvme_remoteport_delete(struct nvme_fc_remote_port 
*remoteport)
lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_DISC,
"6146 remoteport delete complete %p\n",
remoteport);
-   list_del(>list);
ndlp->nrport = NULL;
lpfc_nlp_put(ndlp);
 
@@ -2208,7 +2207,6 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport)
lport = (struct lpfc_nvme_lport *)localport->private;
vport->localport = localport;
lport->vport = vport;
-

[PATCH v2 00/17] lpfc updates for 11.4.0.0

[James Smart]

This patch set provides a number of bug fixes, code cleanups, and error
handling in lpfc.

The patches were cut against the Martin's 4.12/scsi-fixes tree.
There are no outside dependencies. The patches should merge via
Martin's tree. 

Version 2 is a recut against scsi-fixes and includes 2 additional
patches (Coverity scan and EQ delay).

James Smart (17):
  lpfc: Add nvme initiator devloss support
  lpfc: Fix transition nvme-i rport handling to nport only.
  lpfc: Fix nvme port role handling in sysfs and debugfs handlers.
  lpfc: Add changes to assist in NVMET debugging
  lpfc: Fix Lun Priority level shown as NA
  lpfc: Fix nvmet node ref count handling
  lpfc: Fix Port going offline after multiple resets.
  lpfc: Fix counters so outstandng NVME IO count is accurate
  lpfc: Fix return value of board_mode store routine in case of online
failure
  lpfc: Fix crash on powering off BFS VM with passthrough device
  lpfc: Fix System panic after loading the driver
  lpfc: Null pointer dereference when log_verbose is set to 0x
  lpfc: Fix PRLI retry handling when target rejects it.
  lpfc: Fix vports not logging into target
  lpfc: Fix defects reported by Coverity Scan
  lpfc: Add auto EQ delay logic
  lpfc: update to revision to 11.4.0.0

 drivers/scsi/lpfc/lpfc.h |  23 +++--
 drivers/scsi/lpfc/lpfc_attr.c|  94 +++--
 drivers/scsi/lpfc/lpfc_ct.c  |  31 +++---
 drivers/scsi/lpfc/lpfc_debugfs.c |  76 --
 drivers/scsi/lpfc/lpfc_els.c |  17 ++-
 drivers/scsi/lpfc/lpfc_hbadisc.c |  14 +--
 drivers/scsi/lpfc/lpfc_hw4.h |  14 +++
 drivers/scsi/lpfc/lpfc_init.c| 125 --
 drivers/scsi/lpfc/lpfc_nvme.c| 178 
 drivers/scsi/lpfc/lpfc_nvme.h|   2 -
 drivers/scsi/lpfc/lpfc_nvmet.c   | 218 ++-
 drivers/scsi/lpfc/lpfc_nvmet.h   |  14 +--
 drivers/scsi/lpfc/lpfc_scsi.c|  19 +++-
 drivers/scsi/lpfc/lpfc_sli.c |  79 +-
 drivers/scsi/lpfc/lpfc_sli.h |   1 +
 drivers/scsi/lpfc/lpfc_sli4.h|  12 ++-
 drivers/scsi/lpfc/lpfc_version.h |   2 +-
 17 files changed, 588 insertions(+), 331 deletions(-)

-- 
2.11.0

Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Milan P. Gandhi]

On 06/01/2017 08:32 PM, Dan Carpenter wrote:
> On Thu, Jun 01, 2017 at 05:41:06PM +0530, Milan P. Gandhi wrote:
>> Simplify the check for return code of fcoe_if_init routine
>> in fcoe_init function such that we could eliminate need for
>> extra 'out_free' label.
>>
>> Signed-off-by: Milan P. Gandhi 
>> ---
>>  drivers/scsi/fcoe/fcoe.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>> index ea21e7b..fb2a4c9 100644
>> --- a/drivers/scsi/fcoe/fcoe.c
>> +++ b/drivers/scsi/fcoe/fcoe.c
>> @@ -2523,13 +2523,11 @@ static int __init fcoe_init(void)
>>  fcoe_dev_setup();
>>  
>>  rc = fcoe_if_init();
>> -if (rc)
>> -goto out_free;
>> -
>> -mutex_unlock(_config_mutex);
>> -return 0;
>> +if (rc == 0) {
>> +mutex_unlock(_config_mutex);
>> +return 0;
>> +}
>>  
>> -out_free:
>>  mutex_unlock(_config_mutex);
> 
> Gar...  Stop!  No1  Don't do this.
> 
> Do failure handling, not success handling.
> 
> People always think they should get creative with the last if statement
> in a function.  This leads to spaghetti code and it's confusing.  Please
> never do this again.
> 
> The original is correct and the new code is bad rubbish code.
> 
> regards,
> dan carpenter
> 
> 

Oops, my bad sir. Will keep this in mind.

Thanks,
Milan.


Thanks,
Milan.

Re: [PATCH] iscsi: Fix a sleep-in-atomic bug

[Nicholas A. Bellinger]

On Fri, 2017-06-02 at 09:13 +0800, Jia-Ju Bai wrote:
> On 06/01/2017 02:21 PM, Nicholas A. Bellinger wrote:
> > Hi Jia-Ju,
> >
> > On Wed, 2017-05-31 at 11:26 +0800, Jia-Ju Bai wrote:
> >> The driver may sleep under a spin lock, and the function call path is:
> >> iscsit_tpg_enable_portal_group (acquire the lock by spin_lock)
> >>iscsi_update_param_value
> >>  kstrdup(GFP_KERNEL) -->  may sleep
> >>
> >> To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC".
> >>
> >> Signed-off-by: Jia-Ju Bai
> >> ---
> >>   drivers/target/iscsi/iscsi_target_parameters.c |2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > Btw, the use of tpg->tpg_state_lock in iscsit_tpg_enable_portal_group()
> > while checking existing state and calling iscsi_update_param_value() is
> > not necessary, since lio_target_tpg_enable_store() is already holding
> > iscsit_get_tpg() ->  tpg->tpg_access_lock.
> >
> > How about the following instead to only take tpg->tpg_state_lock when
> > updating tpg->tpg_state instead..?
> >
> > diff --git a/drivers/target/iscsi/iscsi_target_tpg.c 
> > b/drivers/target/iscsi/iscsi_target_tpg.c
> > index 2e7e08d..abaabba 100644
> > --- a/drivers/target/iscsi/iscsi_target_tpg.c
> > +++ b/drivers/target/iscsi/iscsi_target_tpg.c
> > @@ -311,11 +311,9 @@ int iscsit_tpg_enable_portal_group(struct 
> > iscsi_portal_group *tpg)
> >  struct iscsi_tiqn *tiqn = tpg->tpg_tiqn;
> >  int ret;
> >
> > -   spin_lock(>tpg_state_lock);
> >  if (tpg->tpg_state == TPG_STATE_ACTIVE) {
> >  pr_err("iSCSI target portal group: %hu is already"
> >  " active, ignoring request.\n", tpg->tpgt);
> > -   spin_unlock(>tpg_state_lock);
> >  return -EINVAL;
> >  }
> >  /*
> > @@ -324,10 +322,8 @@ int iscsit_tpg_enable_portal_group(struct 
> > iscsi_portal_group *tpg)
> >   * is enforced (as per default), and remove the NONE option.
> >   */
> >  param = iscsi_find_param_from_key(AUTHMETHOD, tpg->param_list);
> > -   if (!param) {
> > -   spin_unlock(>tpg_state_lock);
> > +   if (!param)
> >  return -EINVAL;
> > -   }
> >
> >  if (tpg->tpg_attrib.authentication) {
> >  if (!strcmp(param->value, NONE)) {
> > @@ -341,6 +337,7 @@ int iscsit_tpg_enable_portal_group(struct 
> > iscsi_portal_group *tpg)
> >  goto err;
> >  }
> >
> > +   spin_lock(>tpg_state_lock);
> >  tpg->tpg_state = TPG_STATE_ACTIVE;
> >  spin_unlock(>tpg_state_lock);
> >
> > @@ -353,7 +350,6 @@ int iscsit_tpg_enable_portal_group(struct 
> > iscsi_portal_group *tpg)
> >  return 0;
> >
> >   err:
> > -   spin_unlock(>tpg_state_lock);
> >  return ret;
> >   }
> >
> I think it is fine to me.
> 
> Thanks,
> Jia-Ju Bai

Applied with your Reported-by and Reviewed-by.

Thanks!

[PATCH-v2] target: Avoid target_shutdown_sessions loop during queue_depth change

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

When target_shutdown_sessions() is invoked to shutdown all active
sessions associated with a se_node_acl when se_node_acl->queue_depth
is changed via core_tpg_set_initiator_node_queue_depth(), it's
possible that new connections reconnect immediately after explicit
shutdown occurs via target_shutdown_sessions().

Which means it's possible for the newly reconnected session with
the proper queue_depth can be shutdown multiple times when
target_shutdown_sessions() loops to drain all active sessions
for all cases.

This was regression was introduced by:

  commit bc6e6bb470eda42f44bcac96c261cff1216577b3
  Author: Christoph Hellwig 
  Date:   Mon May 2 15:45:19 2016 +0200

  target: consolidate and fix session shutdown

To avoid this case, instead change target_shutdown_sessions() to
pass 'do_restart' and avoid the looping drain of sessions when
invoked via core_tpg_set_initiator_node_queue_depth(), but still
loop during normal se_node_acl delete until all associated
sessions have been shutdown.

(v2 - go back to the original version instead of a local list,
 in order to protect list_del_init(>sess_acl_list) from
 transport_deregister_session_configfs.
 Also use safe list walking in target_shutdown_sessions - nab)

Cc: Christoph Hellwig 
Cc: Mike Christie 
Cc: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_tpg.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 3691373..1b2b60e 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -336,14 +336,14 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
return acl;
 }
 
-static void target_shutdown_sessions(struct se_node_acl *acl)
+static void target_shutdown_sessions(struct se_node_acl *acl, bool do_restart)
 {
-   struct se_session *sess;
+   struct se_session *sess, *sess_tmp;
unsigned long flags;
 
 restart:
spin_lock_irqsave(>nacl_sess_lock, flags);
-   list_for_each_entry(sess, >acl_sess_list, sess_acl_list) {
+   list_for_each_entry_safe(sess, sess_tmp, >acl_sess_list, 
sess_acl_list) {
if (sess->sess_tearing_down)
continue;
 
@@ -352,7 +352,11 @@ static void target_shutdown_sessions(struct se_node_acl 
*acl)
 
if (acl->se_tpg->se_tpg_tfo->close_session)
acl->se_tpg->se_tpg_tfo->close_session(sess);
-   goto restart;
+
+   if (do_restart)
+   goto restart;
+
+   spin_lock_irqsave(>nacl_sess_lock, flags);
}
spin_unlock_irqrestore(>nacl_sess_lock, flags);
 }
@@ -367,7 +371,7 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
*acl)
list_del(>acl_list);
mutex_unlock(>acl_node_mutex);
 
-   target_shutdown_sessions(acl);
+   target_shutdown_sessions(acl, true);
 
target_put_nacl(acl);
/*
@@ -414,7 +418,7 @@ int core_tpg_set_initiator_node_queue_depth(
/*
 * Shutdown all pending sessions to force session reinstatement.
 */
-   target_shutdown_sessions(acl);
+   target_shutdown_sessions(acl, false);
 
pr_debug("Successfully changed queue depth to: %d for Initiator"
" Node: %s on %s Target Portal Group: %u\n", acl->queue_depth,
-- 
1.9.1

Re: [PATCH v3 3/9] blk-mq: use the introduced blk_mq_unquiesce_queue()

[Ming Lei]

On Thu, Jun 01, 2017 at 11:09:00PM +, Bart Van Assche wrote:
> On Thu, 2017-06-01 at 08:54 +0800, Ming Lei wrote:
> > On Wed, May 31, 2017 at 03:21:41PM +, Bart Van Assche wrote:
> > > On Wed, 2017-05-31 at 20:37 +0800, Ming Lei wrote:
> > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > > index 99e16ac479e3..ffcf05765e2b 100644
> > > > --- a/drivers/scsi/scsi_lib.c
> > > > +++ b/drivers/scsi/scsi_lib.c
> > > > @@ -3031,7 +3031,10 @@ scsi_internal_device_unblock(struct scsi_device 
> > > > *sdev,
> > > > return -EINVAL;
> > > >  
> > > > if (q->mq_ops) {
> > > > -   blk_mq_start_stopped_hw_queues(q, false);
> > > > +   if (blk_queue_quiesced(q))
> > > > +   blk_mq_unquiesce_queue(q);
> > > > +   else
> > > > +   blk_mq_start_stopped_hw_queues(q, false);
> > > > } else {
> > > > spin_lock_irqsave(q->queue_lock, flags);
> > > > blk_start_queue(q);
> > > 
> > > As I commented on v2, this change is really wrong. All what's needed here 
> > > is
> > > a call to blk_mq_unquiesce_queue() and nothing else. Adding a call to
> > > blk_mq_start_stopped_hw_queues() is wrong because it makes it impossible 
> > > to
> > > use the STOPPED flag in the SCSI core to make the block layer core stop 
> > > calling
> > > .queue_rq() if a SCSI LLD returns "busy".
> > 
> > I am not sure if I understand your idea, could you explain a bit why it is 
> > wrong?
> > 
> > Let's see the function of scsi_internal_device_block():
> > 
> > if (q->mq_ops) {
> > if (wait)
> > blk_mq_quiesce_queue(q);
> > else
> > blk_mq_stop_hw_queues(q);
> > }
> > 
> > So the queue may be put into quiesced if 'wait' is true, or it is
> > stopped if 'wait' is false.
> > 
> > And this patch just makes the two SCSI APIs symmetrical.
> > 
> > Since we will not stop queue in blk_mq_quiesce_queue() later,
> > I have to unquiese one queue only if it is quiesced.
> > 
> > So suppose the queue is put into stopped in scsi_internal_device_block(),
> > do we expect not to restart it in scsi_internal_device_unblock() via
> > blk_mq_unquiesce_queue()?
> 
> Hello Ming,
> 
> My opinion is that scsi_internal_device_block() and 
> scsi_internal_device_unblock()
> should be changed as follows for the scsi-mq code path:
> * scsi_internal_device_block() should call blk_mq_quiesce_queue(q) if the 
> "wait"
>   argument is true and should set the QUEUE_FLAG_QUIESCED flag if the "wait"
>   argument is false.
> * scsi_internal_device_unblock() should call blk_mq_unquiesce_queue().
> 
> I am aware it sounds weird to set the QUEUE_FLAG_QUIESCED flag without waiting
> until ongoing .queue_rq() calls have finished. The only driver that triggers

This way may break the contract of blk_mq_quiesce_queue() and is really weird.
And the above change shouldn't be related with this patchset, so better
to do whatever you want once this patch is merged.

This patchset won't break current SCSI uses of blk_mq_quiesce_queue() and
won't cause regression, and I setup one iSCSI target yesterday and it works
just fine, how about just keeping this patch as it is?

Once it is merged, you can figure out one perfect solution, but
the contract of blk_mq_quiesce_queue() still shouldn't be broken.

Thanks,
Ming

Re: [PATCH] iscsi: Fix a sleep-in-atomic bug

[Jia-Ju Bai]

On 06/01/2017 02:21 PM, Nicholas A. Bellinger wrote:

Hi Jia-Ju,

On Wed, 2017-05-31 at 11:26 +0800, Jia-Ju Bai wrote:

The driver may sleep under a spin lock, and the function call path is:
iscsit_tpg_enable_portal_group (acquire the lock by spin_lock)
   iscsi_update_param_value
 kstrdup(GFP_KERNEL) -->  may sleep

To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC".

Signed-off-by: Jia-Ju Bai
---
  drivers/target/iscsi/iscsi_target_parameters.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Btw, the use of tpg->tpg_state_lock in iscsit_tpg_enable_portal_group()
while checking existing state and calling iscsi_update_param_value() is
not necessary, since lio_target_tpg_enable_store() is already holding
iscsit_get_tpg() ->  tpg->tpg_access_lock.

How about the following instead to only take tpg->tpg_state_lock when
updating tpg->tpg_state instead..?

diff --git a/drivers/target/iscsi/iscsi_target_tpg.c 
b/drivers/target/iscsi/iscsi_target_tpg.c
index 2e7e08d..abaabba 100644
--- a/drivers/target/iscsi/iscsi_target_tpg.c
+++ b/drivers/target/iscsi/iscsi_target_tpg.c
@@ -311,11 +311,9 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
 struct iscsi_tiqn *tiqn = tpg->tpg_tiqn;
 int ret;

-   spin_lock(>tpg_state_lock);
 if (tpg->tpg_state == TPG_STATE_ACTIVE) {
 pr_err("iSCSI target portal group: %hu is already"
 " active, ignoring request.\n", tpg->tpgt);
-   spin_unlock(>tpg_state_lock);
 return -EINVAL;
 }
 /*
@@ -324,10 +322,8 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
  * is enforced (as per default), and remove the NONE option.
  */
 param = iscsi_find_param_from_key(AUTHMETHOD, tpg->param_list);
-   if (!param) {
-   spin_unlock(>tpg_state_lock);
+   if (!param)
 return -EINVAL;
-   }

 if (tpg->tpg_attrib.authentication) {
 if (!strcmp(param->value, NONE)) {
@@ -341,6 +337,7 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
 goto err;
 }

+   spin_lock(>tpg_state_lock);
 tpg->tpg_state = TPG_STATE_ACTIVE;
 spin_unlock(>tpg_state_lock);

@@ -353,7 +350,6 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
 return 0;

  err:
-   spin_unlock(>tpg_state_lock);
 return ret;
  }


I think it is fine to me.

Thanks,
Jia-Ju Bai

[PATCH v2 05/12] Introduce scsi_start_queue()

[Bart Van Assche]

This patch does not change any functionality.

Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Cc: Israel Rukshin 
Cc: Max Gurtovoy 
Cc: Benjamin Block 
---
 drivers/scsi/scsi_lib.c  | 25 +++--
 drivers/scsi/scsi_priv.h |  1 +
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 845d47244e70..6a58a124714f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3030,6 +3030,20 @@ static int scsi_internal_device_block(struct scsi_device 
*sdev)
return err;
 }
  
+void scsi_start_queue(struct scsi_device *sdev)
+{
+   struct request_queue *q = sdev->request_queue;
+   unsigned long flags;
+
+   if (q->mq_ops) {
+   blk_mq_start_stopped_hw_queues(q, false);
+   } else {
+   spin_lock_irqsave(q->queue_lock, flags);
+   blk_start_queue(q);
+   spin_unlock_irqrestore(q->queue_lock, flags);
+   }
+}
+
 /**
  * scsi_internal_device_unblock_nowait - resume a device after a block request
  * @sdev:  device to resume
@@ -3048,9 +3062,6 @@ static int scsi_internal_device_block(struct scsi_device 
*sdev)
 int scsi_internal_device_unblock_nowait(struct scsi_device *sdev,
enum scsi_device_state new_state)
 {
-   struct request_queue *q = sdev->request_queue; 
-   unsigned long flags;
-
/*
 * Try to transition the scsi device to SDEV_RUNNING or one of the
 * offlined states and goose the device queue if successful.
@@ -3068,13 +3079,7 @@ int scsi_internal_device_unblock_nowait(struct 
scsi_device *sdev,
 sdev->sdev_state != SDEV_OFFLINE)
return -EINVAL;
 
-   if (q->mq_ops) {
-   blk_mq_start_stopped_hw_queues(q, false);
-   } else {
-   spin_lock_irqsave(q->queue_lock, flags);
-   blk_start_queue(q);
-   spin_unlock_irqrestore(q->queue_lock, flags);
-   }
+   scsi_start_queue(sdev);
 
return 0;
 }
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 59ebc1795bb3..f86057842f9a 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -88,6 +88,7 @@ extern void scsi_run_host_queues(struct Scsi_Host *shost);
 extern void scsi_requeue_run_queue(struct work_struct *work);
 extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev);
 extern struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev);
+extern void scsi_start_queue(struct scsi_device *sdev);
 extern int scsi_mq_setup_tags(struct Scsi_Host *shost);
 extern void scsi_mq_destroy_tags(struct Scsi_Host *shost);
 extern int scsi_init_queue(void);
-- 
2.12.2

[PATCH v2 11/12] virtio: Remove code that zeroes driver-private command data

[Bart Van Assche]

Since the SCSI core zeroes driver-private command data, remove
that code from the virtio driver.

Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Cc: Michael S. Tsirkin 
Cc: Christoph Hellwig 
Cc: Johannes Thumshirn 
---
 drivers/scsi/virtio_scsi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index f8dbfeee6c63..dc2e97c543a5 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -547,7 +547,6 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
dev_dbg(>device->sdev_gendev,
"cmd %p CDB: %#02x\n", sc, sc->cmnd[0]);
 
-   memset(cmd, 0, sizeof(*cmd));
cmd->sc = sc;
 
BUG_ON(sc->cmd_len > VIRTIO_SCSI_CDB_SIZE);
-- 
2.12.2

[PATCH v2 00/12] SCSI patches for kernel v4.13

[Bart Van Assche]

Hello Martin,

This patch series consists of the bug fixes and improvements I came up
with during the past two months. Please consider these patches for kernel
v4.13.

Thanks,

Bart.

The changes compared to v1 of this patch series are:
- Left out the block layer patches from this series.
- Reworked this patch series such that it applies cleanly on the 4.13 SCSI
  patch queue and no longer depends on any block layer changes that are not
  yet upstream.
- In patch "Avoid that scsi_exit_rq() triggers a use-after-free", make the
  prep functions save and restore the SCMD_UNCHECKED_ISA_DMA flag.
- Addd patch "Introduce scsi_start_queue()".

Bart Van Assche (12):
  Avoid that scsi_exit_rq() triggers a use-after-free
  Split scsi_internal_device_block()
  Create two versions of scsi_internal_device_unblock()
  Protect SCSI device state changes with a mutex
  Introduce scsi_start_queue()
  Make __scsi_remove_device go straight from BLOCKED to DEL
  Only add commands to the device command list if required by the LLD
  Introduce scsi_mq_sgl_size()
  Make scsi_mq_prep_fn() call scsi_init_command()
  snic: Remove code that zeroes driver-private command data
  virtio: Remove code that zeroes driver-private command data
  xen/scsifront: Remove code that zeroes driver-private command data

 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   8 +-
 drivers/scsi/scsi.c  |   9 +-
 drivers/scsi/scsi_error.c|  10 +-
 drivers/scsi/scsi_lib.c  | 309 +--
 drivers/scsi/scsi_priv.h |   7 +-
 drivers/scsi/scsi_scan.c |  16 +-
 drivers/scsi/scsi_sysfs.c|  37 -
 drivers/scsi/scsi_transport_srp.c|   7 +-
 drivers/scsi/sd.c|   7 +-
 drivers/scsi/snic/snic_scsi.c|   2 -
 drivers/scsi/virtio_scsi.c   |   1 -
 drivers/scsi/xen-scsifront.c |   1 -
 include/scsi/scsi_cmnd.h |   1 +
 include/scsi/scsi_device.h   |   7 +-
 14 files changed, 263 insertions(+), 159 deletions(-)

-- 
2.12.2

[PATCH v2 12/12] xen/scsifront: Remove code that zeroes driver-private command data

[Bart Van Assche]

Since the SCSI core zeroes driver-private command data, remove
that code from the xen-scsifront driver.

Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Reviewed-by: Juergen Gross 
Cc: xen-de...@lists.xenproject.org
Cc: Johannes Thumshirn 
---
 drivers/scsi/xen-scsifront.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index a6a8b60d4902..36f59a1be7e9 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -534,7 +534,6 @@ static int scsifront_queuecommand(struct Scsi_Host *shost,
int err;
 
sc->result = 0;
-   memset(shadow, 0, sizeof(*shadow));
 
shadow->sc  = sc;
shadow->act = VSCSIIF_ACT_SCSI_CDB;
-- 
2.12.2

[PATCH v2 07/12] Only add commands to the device command list if required by the LLD

[Bart Van Assche]

Just like for the scsi-mq code path, in the single queue SCSI code
path only add commands to the per-device command list if required
by the SCSI LLD. This patch will make it easier to merge the
single-queue and multiqueue command initialization code.

Signed-off-by: Bart Van Assche 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Hannes Reinecke 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi.c  |  9 +
 drivers/scsi/scsi_lib.c  | 52 +---
 drivers/scsi/scsi_priv.h |  2 ++
 3 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 7bfbcfa7af40..485684aafb9b 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -108,14 +108,7 @@ EXPORT_SYMBOL(scsi_sd_pm_domain);
  */
 void scsi_put_command(struct scsi_cmnd *cmd)
 {
-   unsigned long flags;
-
-   /* serious error if the command hasn't come from a device list */
-   spin_lock_irqsave(>device->list_lock, flags);
-   BUG_ON(list_empty(>list));
-   list_del_init(>list);
-   spin_unlock_irqrestore(>device->list_lock, flags);
-
+   scsi_del_cmd_from_list(cmd);
BUG_ON(delayed_work_pending(>abort_work));
 }
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8665eccd2fc8..2c43b500e9f4 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -583,19 +583,9 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
 {
-   struct scsi_device *sdev = cmd->device;
-   struct Scsi_Host *shost = sdev->host;
-   unsigned long flags;
-
scsi_mq_free_sgtables(cmd);
scsi_uninit_cmd(cmd);
-
-   if (shost->use_cmd_list) {
-   BUG_ON(list_empty(>list));
-   spin_lock_irqsave(>list_lock, flags);
-   list_del_init(>list);
-   spin_unlock_irqrestore(>list_lock, flags);
-   }
+   scsi_del_cmd_from_list(cmd);
 }
 
 /*
@@ -1133,12 +1123,40 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 }
 EXPORT_SYMBOL(scsi_init_io);
 
+/* Add a command to the list used by the aacraid and dpt_i2o drivers */
+void scsi_add_cmd_to_list(struct scsi_cmnd *cmd)
+{
+   struct scsi_device *sdev = cmd->device;
+   struct Scsi_Host *shost = sdev->host;
+   unsigned long flags;
+
+   if (shost->use_cmd_list) {
+   spin_lock_irqsave(>list_lock, flags);
+   list_add_tail(>list, >cmd_list);
+   spin_unlock_irqrestore(>list_lock, flags);
+   }
+}
+
+/* Remove a command from the list used by the aacraid and dpt_i2o drivers */
+void scsi_del_cmd_from_list(struct scsi_cmnd *cmd)
+{
+   struct scsi_device *sdev = cmd->device;
+   struct Scsi_Host *shost = sdev->host;
+   unsigned long flags;
+
+   if (shost->use_cmd_list) {
+   spin_lock_irqsave(>list_lock, flags);
+   BUG_ON(list_empty(>list));
+   list_del_init(>list);
+   spin_unlock_irqrestore(>list_lock, flags);
+   }
+}
+
 void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 {
void *buf = cmd->sense_buffer;
void *prot = cmd->prot_sdb;
unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
-   unsigned long flags;
 
/* zero out the cmd, except for the embedded scsi_request */
memset((char *)cmd + sizeof(cmd->req), 0,
@@ -1151,9 +1169,7 @@ void scsi_init_command(struct scsi_device *dev, struct 
scsi_cmnd *cmd)
INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler);
cmd->jiffies_at_alloc = jiffies;
 
-   spin_lock_irqsave(>list_lock, flags);
-   list_add_tail(>list, >cmd_list);
-   spin_unlock_irqrestore(>list_lock, flags);
+   scsi_add_cmd_to_list(cmd);
 }
 
 static int scsi_setup_scsi_cmnd(struct scsi_device *sdev, struct request *req)
@@ -1870,11 +1886,7 @@ static int scsi_mq_prep_fn(struct request *req)
INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler);
cmd->jiffies_at_alloc = jiffies;
 
-   if (shost->use_cmd_list) {
-   spin_lock_irq(>list_lock);
-   list_add_tail(>list, >cmd_list);
-   spin_unlock_irq(>list_lock);
-   }
+   scsi_add_cmd_to_list(cmd);
 
sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
cmd->sdb.table.sgl = sg;
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index f86057842f9a..c11c1f9c912c 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -80,6 +80,8 @@ int scsi_eh_get_sense(struct list_head *work_q,
 int scsi_noretry_cmd(struct scsi_cmnd *scmd);
 
 /* scsi_lib.c */
+extern void scsi_add_cmd_to_list(struct scsi_cmnd *cmd);
+extern void scsi_del_cmd_from_list(struct scsi_cmnd *cmd);
 extern int scsi_maybe_unblock_host(struct scsi_device *sdev);
 extern void scsi_device_unbusy(struct 

[PATCH v2 10/12] snic: Remove code that zeroes driver-private command data

[Bart Van Assche]

Since the SCSI core zeroes driver-private command data, remove
that code from the snic driver.

Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Cc: Narsimhulu Musini 
Cc: Sesidhar Baddela 
Cc: Christoph Hellwig 
Cc: Johannes Thumshirn 
---
 drivers/scsi/snic/snic_scsi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index da979a73baa0..05c3a7282d4a 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -359,8 +359,6 @@ snic_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd 
*sc)
SNIC_SCSI_DBG(shost, "sc %p Tag %d (sc %0x) lun %lld in snic_qcmd\n",
  sc, snic_cmd_tag(sc), sc->cmnd[0], sc->device->lun);
 
-   memset(scsi_cmd_priv(sc), 0, sizeof(struct snic_internal_io_state));
-
ret = snic_issue_scsi_req(snic, tgt, sc);
if (ret) {
SNIC_HOST_ERR(shost, "Failed to Q, Scsi Req w/ err %d.\n", ret);
-- 
2.12.2

[PATCH v2 09/12] Make scsi_mq_prep_fn() call scsi_init_command()

[Bart Van Assche]

This patch reduces code duplication. The only functional change in
this patch is that it causes scsi_mq_prep_fn() clear driver-private
command data, just like the already upstream commit 1bad6c4a57ef
("scsi: zero per-cmd private driver data for each MQ I/O").

Signed-off-by: Bart Van Assche 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi_error.c |  2 +-
 drivers/scsi/scsi_lib.c   | 28 +++-
 drivers/scsi/scsi_priv.h  |  4 +++-
 3 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ac3196420435..2e73ef6c1857 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2283,7 +2283,7 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user *arg)
blk_rq_init(NULL, rq);
 
scmd = (struct scsi_cmnd *)(rq + 1);
-   scsi_init_command(dev, scmd);
+   scsi_init_command(dev, scmd, NULL);
scmd->request = rq;
scmd->cmnd = scsi_req(rq)->cmd;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6b4fb48033fb..4041d4c70845 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1152,10 +1152,10 @@ void scsi_del_cmd_from_list(struct scsi_cmnd *cmd)
}
 }
 
-void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
+void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd,
+  struct scsi_data_buffer *prot_sdb)
 {
void *buf = cmd->sense_buffer;
-   void *prot = cmd->prot_sdb;
unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
 
/* zero out the cmd, except for the embedded scsi_request */
@@ -1164,7 +1164,7 @@ void scsi_init_command(struct scsi_device *dev, struct 
scsi_cmnd *cmd)
 
cmd->device = dev;
cmd->sense_buffer = buf;
-   cmd->prot_sdb = prot;
+   cmd->prot_sdb = prot_sdb;
cmd->flags = unchecked_isa_dma;
INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler);
cmd->jiffies_at_alloc = jiffies;
@@ -1342,7 +1342,7 @@ static int scsi_prep_fn(struct request_queue *q, struct 
request *req)
goto out;
}
 
-   scsi_init_command(sdev, cmd);
+   scsi_init_command(sdev, cmd, cmd->prot_sdb);
req->special = cmd;
}
 
@@ -1870,36 +1870,22 @@ static int scsi_mq_prep_fn(struct request *req)
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
struct scsi_device *sdev = req->q->queuedata;
struct Scsi_Host *shost = sdev->host;
-   unsigned char *sense_buf = cmd->sense_buffer;
-   unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
struct scatterlist *sg;
 
-   /* zero out the cmd, except for the embedded scsi_request */
-   memset((char *)cmd + sizeof(cmd->req), 0,
-   sizeof(*cmd) - sizeof(cmd->req));
+   sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
+   scsi_init_command(sdev, cmd, scsi_host_get_prot(shost) ?
+ (void *)sg + scsi_mq_sgl_size(shost) : NULL);
 
req->special = cmd;
 
cmd->request = req;
-   cmd->device = sdev;
-   cmd->sense_buffer = sense_buf;
-   cmd->flags = unchecked_isa_dma;
 
cmd->tag = req->tag;
-
cmd->prot_op = SCSI_PROT_NORMAL;
 
-   INIT_LIST_HEAD(>list);
-   INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler);
-   cmd->jiffies_at_alloc = jiffies;
-
-   scsi_add_cmd_to_list(cmd);
-
-   sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
cmd->sdb.table.sgl = sg;
 
if (scsi_host_get_prot(shost)) {
-   cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost);
memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
 
cmd->prot_sdb->table.sgl =
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index c11c1f9c912c..c43a138423d7 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -8,6 +8,7 @@
 struct request_queue;
 struct request;
 struct scsi_cmnd;
+struct scsi_data_buffer;
 struct scsi_device;
 struct scsi_target;
 struct scsi_host_template;
@@ -30,7 +31,8 @@ extern void scsi_exit_hosts(void);
 /* scsi.c */
 extern bool scsi_use_blk_mq;
 int scsi_init_sense_cache(struct Scsi_Host *shost);
-void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd);
+void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd,
+  struct scsi_data_buffer *prot_sdb);
 #ifdef CONFIG_SCSI_LOGGING
 void scsi_log_send(struct scsi_cmnd *cmd);
 void scsi_log_completion(struct scsi_cmnd *cmd, int disposition);
-- 
2.12.2

[PATCH v2 03/12] Create two versions of scsi_internal_device_unblock()

[Bart Van Assche]

This will make it easier to serialize SCSI device state changes
through a mutex.

Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Reviewed-by: Johannes Thumshirn 
Cc: Christoph Hellwig 
Cc: Sreekanth Reddy 
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |  4 ++--
 drivers/scsi/scsi_lib.c  | 46 +---
 include/scsi/scsi_device.h   |  4 ++--
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index c63bc5ccce37..22998cbd538f 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2883,7 +2883,7 @@ _scsih_internal_device_unblock(struct scsi_device *sdev,
sdev_printk(KERN_WARNING, sdev, "device_unblock and setting to running, 
"
"handle(0x%04x)\n", sas_device_priv_data->sas_target->handle);
sas_device_priv_data->block = 0;
-   r = scsi_internal_device_unblock(sdev, SDEV_RUNNING);
+   r = scsi_internal_device_unblock_nowait(sdev, SDEV_RUNNING);
if (r == -EINVAL) {
/* The device has been set to SDEV_RUNNING by SD layer during
 * device addition but the request queue is still stopped by
@@ -2902,7 +2902,7 @@ _scsih_internal_device_unblock(struct scsi_device *sdev,
r, sas_device_priv_data->sas_target->handle);
 
sas_device_priv_data->block = 0;
-   r = scsi_internal_device_unblock(sdev, SDEV_RUNNING);
+   r = scsi_internal_device_unblock_nowait(sdev, SDEV_RUNNING);
if (r)
sdev_printk(KERN_WARNING, sdev, "retried device_unblock"
" failed with return(%d) for handle(0x%04x)\n",
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c9ce36d16df0..aa84b77e41dc 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3022,24 +3022,22 @@ static int scsi_internal_device_block(struct 
scsi_device *sdev)
 }
  
 /**
- * scsi_internal_device_unblock - resume a device after a block request
+ * scsi_internal_device_unblock_nowait - resume a device after a block request
  * @sdev:  device to resume
- * @new_state: state to set devices to after unblocking
+ * @new_state: state to set the device to after unblocking
  *
- * Called by scsi lld's or the midlayer to restart the device queue
- * for the previously suspended scsi device.  Called from interrupt or
- * normal process context.
+ * Restart the device queue for a previously suspended SCSI device. Does not
+ * sleep.
  *
- * Returns zero if successful or error if not.
+ * Returns zero if successful or a negative error code upon failure.
  *
- * Notes:   
- * This routine transitions the device to the SDEV_RUNNING state
- * or to one of the offline states (which must be a legal transition)
- * allowing the midlayer to goose the queue for this device.
+ * Notes:
+ * This routine transitions the device to the SDEV_RUNNING state or to one of
+ * the offline states (which must be a legal transition) allowing the midlayer
+ * to goose the queue for this device.
  */
-int
-scsi_internal_device_unblock(struct scsi_device *sdev,
-enum scsi_device_state new_state)
+int scsi_internal_device_unblock_nowait(struct scsi_device *sdev,
+   enum scsi_device_state new_state)
 {
struct request_queue *q = sdev->request_queue; 
unsigned long flags;
@@ -3071,7 +3069,27 @@ scsi_internal_device_unblock(struct scsi_device *sdev,
 
return 0;
 }
-EXPORT_SYMBOL_GPL(scsi_internal_device_unblock);
+EXPORT_SYMBOL_GPL(scsi_internal_device_unblock_nowait);
+
+/**
+ * scsi_internal_device_unblock - resume a device after a block request
+ * @sdev:  device to resume
+ * @new_state: state to set the device to after unblocking
+ *
+ * Restart the device queue for a previously suspended SCSI device. May sleep.
+ *
+ * Returns zero if successful or a negative error code upon failure.
+ *
+ * Notes:
+ * This routine transitions the device to the SDEV_RUNNING state or to one of
+ * the offline states (which must be a legal transition) allowing the midlayer
+ * to goose the queue for this device.
+ */
+static int scsi_internal_device_unblock(struct scsi_device *sdev,
+   enum scsi_device_state new_state)
+{
+   return scsi_internal_device_unblock_nowait(sdev, new_state);
+}
 
 static void
 device_block(struct scsi_device *sdev, void *data)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 6ce6888f3c69..5f24dae2a8e1 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -473,8 +473,8 @@ static inline int scsi_device_created(struct scsi_device 
*sdev)
 }
 
 int 

[PATCH v2 04/12] Protect SCSI device state changes with a mutex

[Bart Van Assche]

Enable this mechanism for all scsi_target_*block() callers but not
for the scsi_internal_device_unblock() calls from the mpt3sas driver
because that driver can call scsi_internal_device_unblock() from
atomic context.

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi_error.c |  8 +++-
 drivers/scsi/scsi_lib.c   | 27 +--
 drivers/scsi/scsi_scan.c  | 16 +---
 drivers/scsi/scsi_sysfs.c | 24 +++-
 drivers/scsi/scsi_transport_srp.c |  7 ---
 drivers/scsi/sd.c |  7 +--
 include/scsi/scsi_device.h|  1 +
 7 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ecc07dab893d..ac3196420435 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1628,11 +1628,17 @@ static void scsi_eh_offline_sdevs(struct list_head 
*work_q,
  struct list_head *done_q)
 {
struct scsi_cmnd *scmd, *next;
+   struct scsi_device *sdev;
 
list_for_each_entry_safe(scmd, next, work_q, eh_entry) {
sdev_printk(KERN_INFO, scmd->device, "Device offlined - "
"not ready after error recovery\n");
-   scsi_device_set_state(scmd->device, SDEV_OFFLINE);
+   sdev = scmd->device;
+
+   mutex_lock(>state_mutex);
+   scsi_device_set_state(sdev, SDEV_OFFLINE);
+   mutex_unlock(>state_mutex);
+
scsi_eh_finish_cmd(scmd, done_q);
}
return;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index aa84b77e41dc..845d47244e70 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2881,7 +2881,12 @@ static void scsi_wait_for_queuecommand(struct 
scsi_device *sdev)
 int
 scsi_device_quiesce(struct scsi_device *sdev)
 {
-   int err = scsi_device_set_state(sdev, SDEV_QUIESCE);
+   int err;
+
+   mutex_lock(>state_mutex);
+   err = scsi_device_set_state(sdev, SDEV_QUIESCE);
+   mutex_unlock(>state_mutex);
+
if (err)
return err;
 
@@ -2909,10 +2914,11 @@ void scsi_device_resume(struct scsi_device *sdev)
 * so assume the state is being managed elsewhere (for example
 * device deleted during suspend)
 */
-   if (sdev->sdev_state != SDEV_QUIESCE ||
-   scsi_device_set_state(sdev, SDEV_RUNNING))
-   return;
-   scsi_run_queue(sdev->request_queue);
+   mutex_lock(>state_mutex);
+   if (sdev->sdev_state == SDEV_QUIESCE &&
+   scsi_device_set_state(sdev, SDEV_RUNNING) == 0)
+   scsi_run_queue(sdev->request_queue);
+   mutex_unlock(>state_mutex);
 }
 EXPORT_SYMBOL(scsi_device_resume);
 
@@ -3011,6 +3017,7 @@ static int scsi_internal_device_block(struct scsi_device 
*sdev)
struct request_queue *q = sdev->request_queue;
int err;
 
+   mutex_lock(>state_mutex);
err = scsi_internal_device_block_nowait(sdev);
if (err == 0) {
if (q->mq_ops)
@@ -3018,6 +3025,8 @@ static int scsi_internal_device_block(struct scsi_device 
*sdev)
else
scsi_wait_for_queuecommand(sdev);
}
+   mutex_unlock(>state_mutex);
+
return err;
 }
  
@@ -3088,7 +3097,13 @@ EXPORT_SYMBOL_GPL(scsi_internal_device_unblock_nowait);
 static int scsi_internal_device_unblock(struct scsi_device *sdev,
enum scsi_device_state new_state)
 {
-   return scsi_internal_device_unblock_nowait(sdev, new_state);
+   int ret;
+
+   mutex_lock(>state_mutex);
+   ret = scsi_internal_device_unblock_nowait(sdev, new_state);
+   mutex_unlock(>state_mutex);
+
+   return ret;
 }
 
 static void
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6f7128f49c30..e6de4eee97a3 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -231,6 +231,7 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
sdev->id = starget->id;
sdev->lun = lun;
sdev->channel = starget->channel;
+   mutex_init(>state_mutex);
sdev->sdev_state = SDEV_CREATED;
INIT_LIST_HEAD(>siblings);
INIT_LIST_HEAD(>same_target_siblings);
@@ -943,16 +944,17 @@ static int scsi_add_lun(struct scsi_device *sdev, 
unsigned char *inq_result,
 
/* set the device running here so that slave configure
 * may do I/O */
+   mutex_lock(>state_mutex);
ret = scsi_device_set_state(sdev, SDEV_RUNNING);
-   if (ret) {
+   if (ret)
ret = scsi_device_set_state(sdev, SDEV_BLOCK);
+   mutex_unlock(>state_mutex);
 
-   if (ret) {
-   

[PATCH v2 06/12] Make __scsi_remove_device go straight from BLOCKED to DEL

[Bart Van Assche]

If a device is blocked, make __scsi_remove_device() cause it to
transition to the DEL state. This means that all the commands
issued in .shutdown() will error in the mid-layer, thus making
the removal proceed without being stopped.

This patch is a slightly modified version of a patch from James
Bottomley. This patch avoids that the following lockup occurs:

Call Trace:
 schedule+0x35/0x80
 schedule_timeout+0x237/0x2d0
 io_schedule_timeout+0xa6/0x110
 wait_for_completion_io+0xa3/0x110
 blk_execute_rq+0xdf/0x120
 scsi_execute+0xce/0x150 [scsi_mod]
 scsi_execute_req_flags+0x8f/0xf0 [scsi_mod]
 sd_sync_cache+0xa9/0x190 [sd_mod]
 sd_shutdown+0x6a/0x100 [sd_mod]
 sd_remove+0x64/0xc0 [sd_mod]
 __device_release_driver+0x8d/0x120
 device_release_driver+0x1e/0x30
 bus_remove_device+0xf9/0x170
 device_del+0x127/0x240
 __scsi_remove_device+0xc1/0xd0 [scsi_mod]
 scsi_forget_host+0x57/0x60 [scsi_mod]
 scsi_remove_host+0x72/0x110 [scsi_mod]
 srp_remove_work+0x8b/0x200 [ib_srp]

Reported-by: Israel Rukshin 
Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Cc: James Bottomley 
Cc: Israel Rukshin 
Cc: Max Gurtovoy 
Cc: Benjamin Block 
---
 drivers/scsi/scsi_lib.c   |  2 +-
 drivers/scsi/scsi_sysfs.c | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6a58a124714f..8665eccd2fc8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2624,7 +2624,6 @@ scsi_device_set_state(struct scsi_device *sdev, enum 
scsi_device_state state)
case SDEV_QUIESCE:
case SDEV_OFFLINE:
case SDEV_TRANSPORT_OFFLINE:
-   case SDEV_BLOCK:
break;
default:
goto illegal;
@@ -2638,6 +2637,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum 
scsi_device_state state)
case SDEV_OFFLINE:
case SDEV_TRANSPORT_OFFLINE:
case SDEV_CANCEL:
+   case SDEV_BLOCK:
case SDEV_CREATED_BLOCK:
break;
default:
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index a91537a3abbf..1f243ac16010 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1290,7 +1290,20 @@ void __scsi_remove_device(struct scsi_device *sdev)
 * wait until it has finished before changing the device state.
 */
mutex_lock(>state_mutex);
+   /*
+* If blocked, we go straight to DEL and restart the queue so
+* any commands issued during driver shutdown (like sync
+* cache) are errored immediately.
+*/
res = scsi_device_set_state(sdev, SDEV_CANCEL);
+   if (res != 0) {
+   res = scsi_device_set_state(sdev, SDEV_DEL);
+   if (res == 0) {
+   scsi_start_queue(sdev);
+   sdev_printk(KERN_DEBUG, sdev,
+   "Changed state from BLOCKED to DEL\n");
+   }
+   }
mutex_unlock(>state_mutex);
 
if (res != 0)
-- 
2.12.2

[PATCH v2 02/12] Split scsi_internal_device_block()

[Bart Van Assche]

Instead of passing a "wait" argument to scsi_internal_device_block(),
split this function into a function that waits and a function that
doesn't wait. This will make it easier to serialize SCSI device state
changes through a mutex.

Signed-off-by: Bart Van Assche 
Reviewed-by: Hannes Reinecke 
Reviewed-by: Johannes Thumshirn 
Cc: Christoph Hellwig 
Cc: Sreekanth Reddy 
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |  4 +-
 drivers/scsi/scsi_lib.c  | 73 +++-
 include/scsi/scsi_device.h   |  2 +-
 3 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index a5d872664257..c63bc5ccce37 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2859,7 +2859,7 @@ _scsih_internal_device_block(struct scsi_device *sdev,
sas_device_priv_data->sas_target->handle);
sas_device_priv_data->block = 1;
 
-   r = scsi_internal_device_block(sdev, false);
+   r = scsi_internal_device_block_nowait(sdev);
if (r == -EINVAL)
sdev_printk(KERN_WARNING, sdev,
"device_block failed with return(%d) for handle(0x%04x)\n",
@@ -2895,7 +2895,7 @@ _scsih_internal_device_unblock(struct scsi_device *sdev,
"performing a block followed by an unblock\n",
r, sas_device_priv_data->sas_target->handle);
sas_device_priv_data->block = 1;
-   r = scsi_internal_device_block(sdev, false);
+   r = scsi_internal_device_block_nowait(sdev);
if (r)
sdev_printk(KERN_WARNING, sdev, "retried device_block "
"failed with return(%d) for handle(0x%04x)\n",
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cc9f792cd12b..c9ce36d16df0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2943,28 +2943,20 @@ scsi_target_resume(struct scsi_target *starget)
 EXPORT_SYMBOL(scsi_target_resume);
 
 /**
- * scsi_internal_device_block - internal function to put a device temporarily 
into the SDEV_BLOCK state
- * @sdev:  device to block
- * @wait:  Whether or not to wait until ongoing .queuecommand() /
- * .queue_rq() calls have finished.
+ * scsi_internal_device_block_nowait - try to transition to the SDEV_BLOCK 
state
+ * @sdev: device to block
  *
- * Block request made by scsi lld's to temporarily stop all
- * scsi commands on the specified device. May sleep.
+ * Pause SCSI command processing on the specified device. Does not sleep.
  *
- * Returns zero if successful or error if not
+ * Returns zero if successful or a negative error code upon failure.
  *
- * Notes:   
- * This routine transitions the device to the SDEV_BLOCK state
- * (which must be a legal transition).  When the device is in this
- * state, all commands are deferred until the scsi lld reenables
- * the device with scsi_device_unblock or device_block_tmo fires.
- *
- * To do: avoid that scsi_send_eh_cmnd() calls queuecommand() after
- * scsi_internal_device_block() has blocked a SCSI device and also
- * remove the rport mutex lock and unlock calls from srp_queuecommand().
+ * Notes:
+ * This routine transitions the device to the SDEV_BLOCK state (which must be
+ * a legal transition). When the device is in this state, command processing
+ * is paused until the device leaves the SDEV_BLOCK state. See also
+ * scsi_internal_device_unblock_nowait().
  */
-int
-scsi_internal_device_block(struct scsi_device *sdev, bool wait)
+int scsi_internal_device_block_nowait(struct scsi_device *sdev)
 {
struct request_queue *q = sdev->request_queue;
unsigned long flags;
@@ -2984,21 +2976,50 @@ scsi_internal_device_block(struct scsi_device *sdev, 
bool wait)
 * request queue. 
 */
if (q->mq_ops) {
-   if (wait)
-   blk_mq_quiesce_queue(q);
-   else
-   blk_mq_stop_hw_queues(q);
+   blk_mq_stop_hw_queues(q);
} else {
spin_lock_irqsave(q->queue_lock, flags);
blk_stop_queue(q);
spin_unlock_irqrestore(q->queue_lock, flags);
-   if (wait)
-   scsi_wait_for_queuecommand(sdev);
}
 
return 0;
 }
-EXPORT_SYMBOL_GPL(scsi_internal_device_block);
+EXPORT_SYMBOL_GPL(scsi_internal_device_block_nowait);
+
+/**
+ * scsi_internal_device_block - try to transition to the SDEV_BLOCK state
+ * @sdev: device to block
+ *
+ * Pause SCSI command processing on the specified device and wait until all
+ * ongoing scsi_request_fn() / scsi_queue_rq() calls have finished. May sleep.
+ *
+ * Returns zero if successful or a negative error code upon failure.
+ *
+ * Note:
+ * 

[PATCH v2 08/12] Introduce scsi_mq_sgl_size()

[Bart Van Assche]

This patch does not change any functionality but makes the next
patch easier to read.

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
---
 drivers/scsi/scsi_lib.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 2c43b500e9f4..6b4fb48033fb 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1858,6 +1858,13 @@ static inline int prep_to_mq(int ret)
}
 }
 
+/* Size in bytes of the sg-list stored in the scsi-mq command-private data. */
+static unsigned int scsi_mq_sgl_size(struct Scsi_Host *shost)
+{
+   return min_t(unsigned int, shost->sg_tablesize, SG_CHUNK_SIZE) *
+   sizeof(struct scatterlist);
+}
+
 static int scsi_mq_prep_fn(struct request *req)
 {
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
@@ -1892,10 +1899,7 @@ static int scsi_mq_prep_fn(struct request *req)
cmd->sdb.table.sgl = sg;
 
if (scsi_host_get_prot(shost)) {
-   cmd->prot_sdb = (void *)sg +
-   min_t(unsigned int,
- shost->sg_tablesize, SG_CHUNK_SIZE) *
-   sizeof(struct scatterlist);
+   cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost);
memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
 
cmd->prot_sdb->table.sgl =
@@ -2201,12 +2205,9 @@ struct request_queue *scsi_mq_alloc_queue(struct 
scsi_device *sdev)
 
 int scsi_mq_setup_tags(struct Scsi_Host *shost)
 {
-   unsigned int cmd_size, sgl_size, tbl_size;
+   unsigned int cmd_size, sgl_size;
 
-   tbl_size = shost->sg_tablesize;
-   if (tbl_size > SG_CHUNK_SIZE)
-   tbl_size = SG_CHUNK_SIZE;
-   sgl_size = tbl_size * sizeof(struct scatterlist);
+   sgl_size = scsi_mq_sgl_size(shost);
cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size;
if (scsi_host_get_prot(shost))
cmd_size += sizeof(struct scsi_data_buffer) + sgl_size;
-- 
2.12.2

[PATCH v2 01/12] Avoid that scsi_exit_rq() triggers a use-after-free

[Bart Van Assche]

Dereferencing shost from scsi_exit_rq() is not safe because the
SCSI host may already have been freed when scsi_exit_rq() is
called. Increasing the shost reference count in scsi_init_rq()
and dropping that reference in scsi_exit_rq() is nontrivial since
scsi_host_dev_release() may sleep and since scsi_exit_rq() may
be called from interrupt context. Since scsi_exit_rq() only needs
a single bit from shost, copy that bit into struct scsi_cmnd.

Reported-by: Scott Bauer 
Fixes: e9c787e65c0c ("scsi: allocate scsi_cmnd structures as part of struct 
request")
Signed-off-by: Bart Van Assche 
Cc: Hannes Reinecke 
Cc: Scott Bauer 
Cc: Christoph Hellwig 
Cc: Jan Kara 
Cc: 
---
 drivers/scsi/scsi_lib.c  | 47 +--
 include/scsi/scsi_cmnd.h |  1 +
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 814a4bd8405d..cc9f792cd12b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -44,23 +44,23 @@ static struct kmem_cache *scsi_sense_isadma_cache;
 static DEFINE_MUTEX(scsi_sense_cache_mutex);
 
 static inline struct kmem_cache *
-scsi_select_sense_cache(struct Scsi_Host *shost)
+scsi_select_sense_cache(bool unchecked_isa_dma)
 {
-   return shost->unchecked_isa_dma ?
-   scsi_sense_isadma_cache : scsi_sense_cache;
+   return unchecked_isa_dma ? scsi_sense_isadma_cache : scsi_sense_cache;
 }
 
-static void scsi_free_sense_buffer(struct Scsi_Host *shost,
-   unsigned char *sense_buffer)
+static void scsi_free_sense_buffer(bool unchecked_isa_dma,
+  unsigned char *sense_buffer)
 {
-   kmem_cache_free(scsi_select_sense_cache(shost), sense_buffer);
+   kmem_cache_free(scsi_select_sense_cache(unchecked_isa_dma),
+   sense_buffer);
 }
 
-static unsigned char *scsi_alloc_sense_buffer(struct Scsi_Host *shost,
+static unsigned char *scsi_alloc_sense_buffer(bool unchecked_isa_dma,
gfp_t gfp_mask, int numa_node)
 {
-   return kmem_cache_alloc_node(scsi_select_sense_cache(shost), gfp_mask,
-   numa_node);
+   return kmem_cache_alloc_node(scsi_select_sense_cache(unchecked_isa_dma),
+gfp_mask, numa_node);
 }
 
 int scsi_init_sense_cache(struct Scsi_Host *shost)
@@ -68,7 +68,7 @@ int scsi_init_sense_cache(struct Scsi_Host *shost)
struct kmem_cache *cache;
int ret = 0;
 
-   cache = scsi_select_sense_cache(shost);
+   cache = scsi_select_sense_cache(shost->unchecked_isa_dma);
if (cache)
return 0;
 
@@ -1137,6 +1137,7 @@ void scsi_init_command(struct scsi_device *dev, struct 
scsi_cmnd *cmd)
 {
void *buf = cmd->sense_buffer;
void *prot = cmd->prot_sdb;
+   unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
unsigned long flags;
 
/* zero out the cmd, except for the embedded scsi_request */
@@ -1146,6 +1147,7 @@ void scsi_init_command(struct scsi_device *dev, struct 
scsi_cmnd *cmd)
cmd->device = dev;
cmd->sense_buffer = buf;
cmd->prot_sdb = prot;
+   cmd->flags = unchecked_isa_dma;
INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler);
cmd->jiffies_at_alloc = jiffies;
 
@@ -1846,6 +1848,7 @@ static int scsi_mq_prep_fn(struct request *req)
struct scsi_device *sdev = req->q->queuedata;
struct Scsi_Host *shost = sdev->host;
unsigned char *sense_buf = cmd->sense_buffer;
+   unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
struct scatterlist *sg;
 
/* zero out the cmd, except for the embedded scsi_request */
@@ -1857,6 +1860,7 @@ static int scsi_mq_prep_fn(struct request *req)
cmd->request = req;
cmd->device = sdev;
cmd->sense_buffer = sense_buf;
+   cmd->flags = unchecked_isa_dma;
 
cmd->tag = req->tag;
 
@@ -2003,10 +2007,13 @@ static int scsi_init_request(struct blk_mq_tag_set 
*set, struct request *rq,
unsigned int hctx_idx, unsigned int numa_node)
 {
struct Scsi_Host *shost = set->driver_data;
+   const bool unchecked_isa_dma = shost->unchecked_isa_dma;
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 
-   cmd->sense_buffer =
-   scsi_alloc_sense_buffer(shost, GFP_KERNEL, numa_node);
+   if (unchecked_isa_dma)
+   cmd->flags |= SCMD_UNCHECKED_ISA_DMA;
+   cmd->sense_buffer = scsi_alloc_sense_buffer(unchecked_isa_dma,
+   GFP_KERNEL, numa_node);
if (!cmd->sense_buffer)
return -ENOMEM;
cmd->req.sense = cmd->sense_buffer;
@@ -2016,10 +2023,10 @@ static int scsi_init_request(struct blk_mq_tag_set 
*set, struct request *rq,
 static 

Re: [PATCH v3 3/9] blk-mq: use the introduced blk_mq_unquiesce_queue()

[Bart Van Assche]

On Thu, 2017-06-01 at 08:54 +0800, Ming Lei wrote:
> On Wed, May 31, 2017 at 03:21:41PM +, Bart Van Assche wrote:
> > On Wed, 2017-05-31 at 20:37 +0800, Ming Lei wrote:
> > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > index 99e16ac479e3..ffcf05765e2b 100644
> > > --- a/drivers/scsi/scsi_lib.c
> > > +++ b/drivers/scsi/scsi_lib.c
> > > @@ -3031,7 +3031,10 @@ scsi_internal_device_unblock(struct scsi_device 
> > > *sdev,
> > >   return -EINVAL;
> > >  
> > >   if (q->mq_ops) {
> > > - blk_mq_start_stopped_hw_queues(q, false);
> > > + if (blk_queue_quiesced(q))
> > > + blk_mq_unquiesce_queue(q);
> > > + else
> > > + blk_mq_start_stopped_hw_queues(q, false);
> > >   } else {
> > >   spin_lock_irqsave(q->queue_lock, flags);
> > >   blk_start_queue(q);
> > 
> > As I commented on v2, this change is really wrong. All what's needed here is
> > a call to blk_mq_unquiesce_queue() and nothing else. Adding a call to
> > blk_mq_start_stopped_hw_queues() is wrong because it makes it impossible to
> > use the STOPPED flag in the SCSI core to make the block layer core stop 
> > calling
> > .queue_rq() if a SCSI LLD returns "busy".
> 
> I am not sure if I understand your idea, could you explain a bit why it is 
> wrong?
> 
> Let's see the function of scsi_internal_device_block():
> 
>   if (q->mq_ops) {
> if (wait)
> blk_mq_quiesce_queue(q);
> else
> blk_mq_stop_hw_queues(q);
>   }
> 
> So the queue may be put into quiesced if 'wait' is true, or it is
> stopped if 'wait' is false.
> 
> And this patch just makes the two SCSI APIs symmetrical.
> 
> Since we will not stop queue in blk_mq_quiesce_queue() later,
> I have to unquiese one queue only if it is quiesced.
> 
> So suppose the queue is put into stopped in scsi_internal_device_block(),
> do we expect not to restart it in scsi_internal_device_unblock() via
> blk_mq_unquiesce_queue()?

Hello Ming,

My opinion is that scsi_internal_device_block() and 
scsi_internal_device_unblock()
should be changed as follows for the scsi-mq code path:
* scsi_internal_device_block() should call blk_mq_quiesce_queue(q) if the "wait"
  argument is true and should set the QUEUE_FLAG_QUIESCED flag if the "wait"
  argument is false.
* scsi_internal_device_unblock() should call blk_mq_unquiesce_queue().

I am aware it sounds weird to set the QUEUE_FLAG_QUIESCED flag without waiting
until ongoing .queue_rq() calls have finished. The only driver that triggers
that code path is the mpt3sas driver. I think it's unfortunate that that driver
has ever been allowed to call scsi_internal_device_block() because it's the only
driver that calls that function from a context where sleeping is not allowed.
No matter whether the scsi_internal_device_block() call from the mpt3sas driver
sets the QUEUE_FLAG_QUIESCED or the BLK_MQ_S_STOPPED flag, I don't think that
will have the effect the authors of this driver intended. Unfortunately I'm not
familiar enough with the mpt3sas driver to fix that driver myself.

Note: these changes conflict with a patch SCSI patch series I started working on
about two months ago. See also 
https://www.spinics.net/lists/linux-scsi/msg109103.html.

Bart.

Re: [PATCH 10/15] lpfc: Fix crash on powering off BFS VM with passthrough device

[Raphael Philipe Mendes da Silva]

Answering to https://marc.info/?l=linux-scsi=149625285323719=2
Yes, it solves the error.

Tested-by: Raphael Silva 

Re: [PATCH v2 13/17] qla2xxx: Remove redundant code

[Madhani, Himanshu]

> On May 31, 2017, at 4:37 PM, Bart Van Assche  
> wrote:
> 
> On Tue, 2017-05-30 at 10:54 -0700, Himanshu Madhani wrote:
>> The reason for hard coding LUN ID to 0 is that, from the FC
>> protocol perspective, ABTS does not have any knowledge of
>> LUN ID. So, there is no reason for qla2xxx driver to
>> manufacture the LUN ID.
> 
> Sorry but I think this is completely wrong. Are you aware that
> target_submit_tmr() performs an exact match on the LUN specified as the fourth
> argument? This patch will cause ABTS requests to be ignored that apply to
> commands that have been submitted to another LUN than LUN 0.
> 
> Please note that I had proposed a better approach three months ago on the
> target-devel mailing list and that I'm still waiting for someone from Cavium 
> to
> review these patches:
> * [PATCH v6 09/33] target: Make it possible to specify I_T nexus for SCSI
>   abort (http://www.spinics.net/lists/target-devel/msg14534.html).
> * [PATCH v6 10/33] tcm_qla2xxx: Let the target core look up the LUN of the
>   aborted cmd (http://www.spinics.net/lists/target-devel/msg14563.html).
> 
> Bart.

We will drop this patch from current series and will look at the alternatives 
you have posted.

Thanks,
- Himanshu

Re: [PATCH v2 15/17] qla2xxx: Accelerate SCSI BUSY status generation in target mode

[Madhani, Himanshu]

> On May 31, 2017, at 4:39 PM, Bart Van Assche  
> wrote:
> 
> On Tue, 2017-05-30 at 10:54 -0700, Himanshu Madhani wrote:
>> +/* FW perform Exchang validation */
> 
> Did you perhaps intend "exchange" instead of "Exchang"? Please fix this if you
> repost this patch.
> 
> Thanks,
> 
> Bart.

Sure. Will fix this up when i repost this series.

Thanks,
- Himanshu

Re: [PATCH v2 12/17] qla2xxx: Add ql2xiniexchg parameter

[Madhani, Himanshu]

> On May 31, 2017, at 4:29 PM, Bart Van Assche  
> wrote:
> 
> On Tue, 2017-05-30 at 10:54 -0700, Himanshu Madhani wrote:
>> -   "For Dual Mode (qlini_mode=dual), this parameter determines "
>> -   "the percentage of exchanges/cmds FW will allocate resources "
>> -   "for Target mode.");
>> + "For Dual Mode (qlini_mode=dual), this parameter determines "
>> + "the percentage of exchanges/cmds FW will allocate resources "
>> + "for Target mode.");
> 
> Hello Himanshu and Quinn,
> 
> There are several pointless changes in this patch series like the above
> that changes indentation from 7 spaces to five spaces. Please leave such
> changes out from future patch submissions.
> 
> Thanks,
> 
> Bart.

Sure.

Thanks,
Himanshu

Re: [PATCH v2 04/17] qla2xxx: Include Exchange offload/Extended Login into FW dump

[Madhani, Himanshu]

> On May 31, 2017, at 4:12 PM, Bart Van Assche  
> wrote:
> 
> On Tue, 2017-05-30 at 10:54 -0700, Himanshu Madhani wrote:
>> static inline void *
>> +qla25xx_copy_exlogin(struct qla_hw_data *ha, void *ptr, uint32_t 
>> **last_chain)
>> +{
>> +struct qla2xxx_offld_chain *c = ptr;
>> +
>> +if (!ha->exlogin_buf)
>> +return ptr;
>> +
>> +*last_chain = >type;
>> +
>> +c->type = htonl(DUMP_CHAIN_EXLOGIN);
>> +c->chain_size = htonl(sizeof(struct qla2xxx_offld_chain) +
>> +ha->exlogin_size);
>> +c->size = htonl(ha->exlogin_size);
> 
> Since this is not networking code, why is this code using htonl() instead of 
> cpu_to_be32()?
> 
>> +c->addr_l = htonl(LSD(ha->exlogin_buf_dma));
>> +c->addr_h = htonl(MSD(ha->exlogin_buf_dma));
> 
> Please use cpu_to_be64() instead of this weird construct.
> 
>> +uint32_t addr_l;
>> +uint32_t addr_h;
> 
> Please declare this as a single 64-bit variable instead of using this weird
> split into two 32-bit variables.
> 
> Thanks,
> 
> Bart.

Sure. Will update patch with the correct construct. 

Thanks,
- Himanshu

Re: [RFC v2 0/2] generate uevent for SCSI sense code

[Song Liu]

> On May 18, 2017, at 11:14 AM, Song Liu  wrote:
> 
> This change is to follow up our discussion on event log for media
> management during LSF/MM 2017.
> 
> I have included feedbacks from Hannes Reinecke, Ewan D. Milne, and
> Benjamin Block.
> 
> Please kindly let me know your thoughts on this.
> 
> Thanks,
> Song
> 
> Song Liu (2):
>  scsi: generate uevent for SCSI sense code
>  scsi: add rate limit to scsi sense code uevent
> 
> drivers/scsi/Kconfig   | 14 +++
> drivers/scsi/scsi_error.c  | 58 ++
> drivers/scsi/scsi_lib.c| 58 +-
> drivers/scsi/scsi_scan.c   |  4 
> drivers/scsi/scsi_sysfs.c  | 51 
> include/scsi/scsi_common.h |  6 +
> include/scsi/scsi_device.h | 37 -
> 7 files changed, 226 insertions(+), 2 deletions(-)
> 
> --
> 2.9.3

Dear Hannes, James, Benjamin, and Ewan, 

Could you please share your feedback on this version of the RFC? Does it 
need major changes?

Thanks,
Song

Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Dan Carpenter]

On Thu, Jun 01, 2017 at 05:41:06PM +0530, Milan P. Gandhi wrote:
> Simplify the check for return code of fcoe_if_init routine
> in fcoe_init function such that we could eliminate need for
> extra 'out_free' label.
> 
> Signed-off-by: Milan P. Gandhi 
> ---
>  drivers/scsi/fcoe/fcoe.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ea21e7b..fb2a4c9 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -2523,13 +2523,11 @@ static int __init fcoe_init(void)
>   fcoe_dev_setup();
>  
>   rc = fcoe_if_init();
> - if (rc)
> - goto out_free;
> -
> - mutex_unlock(_config_mutex);
> - return 0;
> + if (rc == 0) {
> + mutex_unlock(_config_mutex);
> + return 0;
> + }
>  
> -out_free:
>   mutex_unlock(_config_mutex);

Gar...  Stop!  No1  Don't do this.

Do failure handling, not success handling.

People always think they should get creative with the last if statement
in a function.  This leads to spaghetti code and it's confusing.  Please
never do this again.

The original is correct and the new code is bad rubbish code.

regards,
dan carpenter

Re: [PATCH] scsi: lpfc: fix spelling mistake "entrys" -> "entries"

[Colin Ian King]

On 01/06/17 15:35, Dan Carpenter wrote:
> On Fri, May 26, 2017 at 11:11:37AM +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> Trivial fix to spelling mistake in debugfs message
>>
> 
> Are you using a tool to find all these spelling mistakes?

Yep, I'm using kernelscan [1]

kernelscan -c path-to-kernel-source

I run this daily and diff the latest with the previous results. It uses
the dictionary from "spell". It's been hand optimized to be rather fast.

[1] https://github.com/ColinIanKing/kernelscan

> 
> regards,
> dan carpenter
> 

Re: [PATCH] scsi: lpfc: fix spelling mistake "entrys" -> "entries"

[Dan Carpenter]

On Fri, May 26, 2017 at 11:11:37AM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in debugfs message
> 

Are you using a tool to find all these spelling mistakes?

regards,
dan carpenter

Re: [PATCH 00/31] SCSI patches for kernel v4.13.

[Christoph Hellwig]

On Thu, Jun 01, 2017 at 02:08:04PM +, Bart Van Assche wrote:
> The first eight patches in this series do not depend on any block layer 
> changes.
> Do you want me to repost this patches or can you perhaps queue these without a
> repost?

It would be great if you could repost them, also for the reviewers.

Re: [PATCH 00/31] SCSI patches for kernel v4.13.

[Bart Van Assche]

On Wed, 2017-05-24 at 22:04 -0400, Martin K. Petersen wrote:
> > This patch series consists of the bug fixes I came up with during the
> > past two months. Please consider these patches for kernel v4.13.
> 
> No major objections from me. Although you may have to slice and dice the
> series differently so we can get the block bits queued through Jens and
> then do the rest as a stage 2 merge.

Hello Martin,

The first eight patches in this series do not depend on any block layer changes.
Do you want me to repost this patches or can you perhaps queue these without a
repost?

Thanks,

Bart.

Re: [ufs]: [scsi]: BUG: spinlock recursion on CPU#4

[Bart Van Assche]

On Thu, 2017-06-01 at 12:28 +0530, Asutosh Das (asd) wrote:
> Please can you check if this is actually a bug and my understanding is 
> correct.

Hello Asutosh,

Spinlock recursion is always a bug. With what kernel version did you encounter
this? Was it with a kernel from kernel.org, a distro kernel or an Android 
kernel?
Have you already tried to reproduce this with kernel debugging enabled? Enabling
CONFIG_DEBUG_SPINLOCK and CONFIG_PROVE_LOCKING should provide more detailed
information.

Bart.

Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Johannes Thumshirn]

On 06/01/2017 02:11 PM, Milan P. Gandhi wrote:
> Simplify the check for return code of fcoe_if_init routine
> in fcoe_init function such that we could eliminate need for
> extra 'out_free' label.
> 
> Signed-off-by: Milan P. Gandhi 
> ---

Ahm and what happens to the fcoe_wq then?


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] Remove an extra out label in _fcoe_create function

[Johannes Thumshirn]

On 06/01/2017 02:08 PM, Milan P. Gandhi wrote:
> This patch removes an extra out label in _fcoe_create function
> where we return if creation of FCOE interface is failed.
> 
> Signed-off-by: Milan P. Gandhi 
> ---

Thanks,
Acked-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] Fix few small typos in fcoe.c

[Johannes Thumshirn]

On 06/01/2017 02:05 PM, Milan P. Gandhi wrote:
> This patch does a cleanup and fixes few small typos in fcoe.c
> 
> Signed-off-by: Milan P. Gandhi 
> ---

Looks good,
Acked-by: Johannes Thumshirn 


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Milan P. Gandhi]

Simplify the check for return code of fcoe_if_init routine
in fcoe_init function such that we could eliminate need for
extra 'out_free' label.

Signed-off-by: Milan P. Gandhi 
---
 drivers/scsi/fcoe/fcoe.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ea21e7b..fb2a4c9 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2523,13 +2523,11 @@ static int __init fcoe_init(void)
fcoe_dev_setup();
 
rc = fcoe_if_init();
-   if (rc)
-   goto out_free;
-
-   mutex_unlock(_config_mutex);
-   return 0;
+   if (rc == 0) {
+   mutex_unlock(_config_mutex);
+   return 0;
+   }
 
-out_free:
mutex_unlock(_config_mutex);
 out_destroy:
destroy_workqueue(fcoe_wq);

[PATCH] Remove an extra out label in _fcoe_create function

[Milan P. Gandhi]

This patch removes an extra out label in _fcoe_create function
where we return if creation of FCOE interface is failed.

Signed-off-by: Milan P. Gandhi 
---
 drivers/scsi/fcoe/fcoe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 7b960d3..ea21e7b 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2258,7 +2258,7 @@ static int _fcoe_create(struct net_device *netdev, enum 
fip_mode fip_mode,
fcoe_interface_cleanup(fcoe);
mutex_unlock(_config_mutex);
fcoe_ctlr_device_delete(ctlr_dev);
-   goto out;
+   return rc;
}
 
/* Make this the "master" N_Port */
@@ -2299,7 +2299,7 @@ static int _fcoe_create(struct net_device *netdev, enum 
fip_mode fip_mode,
 out_nodev:
rtnl_unlock();
mutex_unlock(_config_mutex);
-out:
+
return rc;
 }
 

[PATCH] Fix few small typos in fcoe.c

[Milan P. Gandhi]

This patch does a cleanup and fixes few small typos in fcoe.c

Signed-off-by: Milan P. Gandhi 
---
 drivers/scsi/fcoe/fcoe.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 90939f6..7b960d3 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -519,7 +519,7 @@ static void fcoe_interface_cleanup(struct fcoe_interface 
*fcoe)
  * @skb:  The receive skb
  * @netdev:   The associated net device
  * @ptype:The packet_type structure which was used to register this handler
- * @orig_dev: The original net_device the the skb was received on.
+ * @orig_dev: The original net_device the skb was received on.
  *   (in case dev is a bond)
  *
  * Returns: 0 for success
@@ -542,7 +542,7 @@ static int fcoe_fip_recv(struct sk_buff *skb, struct 
net_device *netdev,
  * @skb:  The receive skb
  * @netdev:   The associated net device
  * @ptype:The packet_type structure which was used to register this handler
- * @orig_dev: The original net_device the the skb was received on.
+ * @orig_dev: The original net_device the skb was received on.
  *   (in case dev is a bond)
  *
  * Returns: 0 for success
@@ -2590,7 +2590,7 @@ module_exit(fcoe_exit);
  * fcoe_flogi_resp() - FCoE specific FLOGI and FDISC response handler
  * @seq: active sequence in the FLOGI or FDISC exchange
  * @fp: response frame, or error encoded in a pointer (timeout)
- * @arg: pointer the the fcoe_ctlr structure
+ * @arg: pointer to the fcoe_ctlr structure
  *
  * This handles MAC address management for FCoE, then passes control on to
  * the libfc FLOGI response handler.
@@ -2619,7 +2619,7 @@ static void fcoe_flogi_resp(struct fc_seq *seq, struct 
fc_frame *fp, void *arg)
  * fcoe_logo_resp() - FCoE specific LOGO response handler
  * @seq: active sequence in the LOGO exchange
  * @fp: response frame, or error encoded in a pointer (timeout)
- * @arg: pointer the the fcoe_ctlr structure
+ * @arg: pointer to the fcoe_ctlr structure
  *
  * This handles MAC address management for FCoE, then passes control on to
  * the libfc LOGO response handler.

Re: [PATCH 2/2] target/configfs: Kill se_lun->lun_link_magic

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH 1/2] target/configfs: Kill se_device->dev_link_magic

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH v3 14/23] scsi: hisi_sas: add v3 cq interrupt handler

[John Garry]

On 01/06/2017 06:41, Christoph Hellwig wrote:

+   case SAS_PROTOCOL_SSP:
+   {
+   unsigned char op = task->ssp_task.cmd->cmnd[0];
+
+   if (op == READ_6 || op == WRITE_6 ||
+   op == READ_10 || op == WRITE_10 ||
+   op == READ_12 || op == WRITE_12 ||
+   op == READ_16 || op == WRITE_16)
+   return true;
+   break;
+   }


Wee. why do you care?  >


In the command completion code we need to check for abnormal completion 
due to underflow, but ignore it when it occurs in some commands, like 
inquiry. This is why we check for rw command - it is equivalent to !inquiry.


I'll see if we can change the check to explicitly ignore certain 
commands which complete abnormally with underflow.



What about 32-byte CDs or things like Write Same?



You're talking about VARIABLE_LENGTH_CMD (opcode 0x7f) for 32-byte CDs, 
right?


I need to check on WRITE SAME.


.



Thanks,
John

[PATCH 1/2] target/configfs: Kill se_device->dev_link_magic

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

Instead of using a hardcoded magic value in se_device when verifying
a target config_item symlink source during target_fabric_port_link(),
go ahead and use target_core_dev_item_ops directly instead.

Cc: Christoph Hellwig 
Cc: Mike Christie 
Cc: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_configfs.c|  6 +-
 drivers/target/target_core_device.c  |  1 -
 drivers/target/target_core_fabric_configfs.c | 12 +++-
 include/target/target_core_base.h|  2 --
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_configfs.c 
b/drivers/target/target_core_configfs.c
index 0326607..9b8abd5 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2236,7 +2236,11 @@ static void target_core_dev_release(struct config_item 
*item)
target_free_device(dev);
 }
 
-static struct configfs_item_operations target_core_dev_item_ops = {
+/*
+ * Used in target_core_fabric_configfs.c to verify valid se_device symlink
+ * within target_fabric_port_link()
+ */
+struct configfs_item_operations target_core_dev_item_ops = {
.release= target_core_dev_release,
 };
 
diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index a5762e6..e4771ce 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -756,7 +756,6 @@ struct se_device *target_alloc_device(struct se_hba *hba, 
const char *name)
if (!dev)
return NULL;
 
-   dev->dev_link_magic = SE_DEV_LINK_MAGIC;
dev->se_hba = hba;
dev->transport = hba->backend->ops;
dev->prot_length = sizeof(struct t10_pi_tuple);
diff --git a/drivers/target/target_core_fabric_configfs.c 
b/drivers/target/target_core_fabric_configfs.c
index d1e6cab..2cbaecd 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -620,6 +620,8 @@ static ssize_t target_fabric_port_alua_tg_pt_write_md_store(
NULL,
 };
 
+extern struct configfs_item_operations target_core_dev_item_ops;
+
 static int target_fabric_port_link(
struct config_item *lun_ci,
struct config_item *se_dev_ci)
@@ -628,16 +630,16 @@ static int target_fabric_port_link(
struct se_lun *lun = container_of(to_config_group(lun_ci),
struct se_lun, lun_group);
struct se_portal_group *se_tpg;
-   struct se_device *dev =
-   container_of(to_config_group(se_dev_ci), struct se_device, 
dev_group);
+   struct se_device *dev;
struct target_fabric_configfs *tf;
int ret;
 
-   if (dev->dev_link_magic != SE_DEV_LINK_MAGIC) {
-   pr_err("Bad dev->dev_link_magic, not a valid se_dev_ci pointer:"
-   " %p to struct se_device: %p\n", se_dev_ci, dev);
+   if (!se_dev_ci->ci_type ||
+   se_dev_ci->ci_type->ct_item_ops != _core_dev_item_ops) {
+   pr_err("Bad se_dev_ci, not a valid se_dev_ci pointer: %p\n", 
se_dev_ci);
return -EFAULT;
}
+   dev = container_of(to_config_group(se_dev_ci), struct se_device, 
dev_group);
 
if (!(dev->dev_flags & DF_CONFIGURED)) {
pr_err("se_device not configured yet, cannot port link\n");
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 0c1dce2..c3c14d0 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -746,8 +746,6 @@ struct se_dev_stat_grps {
 };
 
 struct se_device {
-#define SE_DEV_LINK_MAGIC  0xfeeddeef
-   u32 dev_link_magic;
/* RELATIVE TARGET PORT IDENTIFER Counter */
u16 dev_rpti_counter;
/* Used for SAM Task Attribute ordering */
-- 
1.9.1

[PATCH 2/2] target/configfs: Kill se_lun->lun_link_magic

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

Instead of using a hardcoded magic value in se_lun when verifying
a target config_item symlink source during target_fabric_mappedlun_link(),
go ahead and use target_fabric_port_item_ops directly instead.

Cc: Christoph Hellwig 
Cc: Mike Christie 
Cc: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_fabric_configfs.c | 13 -
 drivers/target/target_core_tpg.c |  1 -
 include/target/target_core_base.h|  2 --
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/target/target_core_fabric_configfs.c 
b/drivers/target/target_core_fabric_configfs.c
index 2cbaecd..e9e917c 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -65,6 +65,8 @@
pr_debug("Setup generic %s\n", __stringify(_name)); \
 }
 
+static struct configfs_item_operations target_fabric_port_item_ops;
+
 /* Start of tfc_tpg_mappedlun_cit */
 
 static int target_fabric_mappedlun_link(
@@ -72,19 +74,20 @@ static int target_fabric_mappedlun_link(
struct config_item *lun_ci)
 {
struct se_dev_entry *deve;
-   struct se_lun *lun = container_of(to_config_group(lun_ci),
-   struct se_lun, lun_group);
+   struct se_lun *lun;
struct se_lun_acl *lacl = container_of(to_config_group(lun_acl_ci),
struct se_lun_acl, se_lun_group);
struct se_portal_group *se_tpg;
struct config_item *nacl_ci, *tpg_ci, *tpg_ci_s, *wwn_ci, *wwn_ci_s;
bool lun_access_ro;
 
-   if (lun->lun_link_magic != SE_LUN_LINK_MAGIC) {
-   pr_err("Bad lun->lun_link_magic, not a valid lun_ci pointer:"
-   " %p to struct lun: %p\n", lun_ci, lun);
+   if (!lun_ci->ci_type ||
+   lun_ci->ci_type->ct_item_ops != _fabric_port_item_ops) {
+   pr_err("Bad lun_ci, not a valid lun_ci pointer: %p\n", lun_ci);
return -EFAULT;
}
+   lun = container_of(to_config_group(lun_ci), struct se_lun, lun_group);
+
/*
 * Ensure that the source port exists
 */
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 74893ac..6b20b9f 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -584,7 +584,6 @@ struct se_lun *core_tpg_alloc_lun(
return ERR_PTR(-ENOMEM);
}
lun->unpacked_lun = unpacked_lun;
-   lun->lun_link_magic = SE_LUN_LINK_MAGIC;
atomic_set(>lun_acl_count, 0);
init_completion(>lun_ref_comp);
init_completion(>lun_shutdown_comp);
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index c3c14d0..47d9f38 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -701,8 +701,6 @@ struct scsi_port_stats {
 
 struct se_lun {
u64 unpacked_lun;
-#define SE_LUN_LINK_MAGIC  0x7771
-   u32 lun_link_magic;
boollun_shutdown;
boollun_access_ro;
u32 lun_index;
-- 
1.9.1

Re: [PATCH v3 13/23] scsi: hisi_sas: add phy up/down/bcast and channel ISR

[John Garry]

On 01/06/2017 06:41, Christoph Hellwig wrote:

> +static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba)
> +{
> +  struct device *dev = hisi_hba->dev;
> +  struct pci_dev *pdev = hisi_hba->pci_dev;
> +  int vectors, i, irq, rc;
> +  int max_msi = HISI_SAS_MSI_COUNT_V3_HW;
> +  int msi_vectors[HISI_SAS_MSI_COUNT_V3_HW];
> +
> +  if (pdev->msi_enabled)
> +  pci_disable_msi(pdev);

How could MSIs be enabled at init time?  Even if so you should use
pci_free_irq_vectors.


Right, I don't think it could, so this can be removed.




> +  for (i = 0; i < vectors; i++)
> +  msi_vectors[i] = pdev->irq + i;

You should not need this array, just use pci_irq_vectors().



That should be ok.


.



Thanks,
John

[PATCH] scsi: csiostor: add check for supported fw version

[Varun Prakash]

T6 FCoE support is added in fw version 1.16.45.0 so
return error if fw version < 1.16.45.0 for T6 adapters.

Signed-off-by: Varun Prakash 
---
 drivers/scsi/csiostor/csio_hw.c | 19 +++
 drivers/scsi/csiostor/csio_hw.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index c6e1814..2029ad2 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -2068,6 +2068,17 @@ csio_hw_flash_fw(struct csio_hw *hw, int *reset)
return ret;
 }
 
+static int csio_hw_check_fwver(struct csio_hw *hw)
+{
+   if (csio_is_t6(hw->pdev->device & CSIO_HW_CHIP_MASK) &&
+   (hw->fwrev < CSIO_MIN_T6_FW)) {
+   csio_hw_print_fw_version(hw, "T6 unsupported fw");
+   return -1;
+   }
+
+   return 0;
+}
+
 /*
  * csio_hw_configure - Configure HW
  * @hw - HW module
@@ -2135,6 +2146,10 @@ csio_hw_configure(struct csio_hw *hw)
if (rv != 0)
goto out;
 
+   rv = csio_hw_check_fwver(hw);
+   if (rv < 0)
+   goto out;
+
/* If the firmware doesn't support Configuration Files,
 * return an error.
 */
@@ -2162,6 +2177,10 @@ csio_hw_configure(struct csio_hw *hw)
}
 
} else {
+   rv = csio_hw_check_fwver(hw);
+   if (rv < 0)
+   goto out;
+
if (hw->fw_state == CSIO_DEV_STATE_INIT) {
 
hw->flags |= CSIO_HWF_USING_SOFT_PARAMS;
diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 62758e8..9acb895 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -71,6 +71,7 @@
 #define CSIO_MAX_CMD_PER_LUN   32
 #define CSIO_MAX_DDP_BUF_SIZE  (1024 * 1024)
 #define CSIO_MAX_SECTOR_SIZE   128
+#define CSIO_MIN_T6_FW 0x01102D00  /* FW 1.16.45.0 */
 
 /* Interrupts */
 #define CSIO_EXTRA_MSI_IQS 2   /* Extra iqs for INTX/MSI mode
-- 
2.0.2

Re: [PATCH] target: Avoid target_shutdown_sessions loop during queue_depth change

[Nicholas A. Bellinger]

On Thu, 2017-06-01 at 08:57 +0200, Christoph Hellwig wrote:
> How about this slightly easier to read version?

Fine by me.

Applied.

[PATCH-v2] target/iblock: Convert WRITE_SAME to blkdev_issue_zeroout

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

The people who are actively using iblock_execute_write_same_direct() are
doing so in the context of ESX VAAI BlockZero, together with
EXTENDED_COPY and COMPARE_AND_WRITE primitives.

In practice though I've not seen any users of IBLOCK WRITE_SAME for
anything other than VAAI BlockZero, so just using blkdev_issue_zeroout()
when available, and falling back to iblock_execute_write_same() if the
WRITE_SAME buffer contains anything other than zeros should be OK.

(Hook up max_write_zeroes_sectors to signal LBPRZ feature bit in
 target_configure_unmap_from_queue - nab)

Signed-off-by: Christoph Hellwig 
Cc: Christoph Hellwig 
Cc: Mike Christie 
Cc: Hannes Reinecke 
Cc: Martin K. Petersen 
Cc: Jens Axboe 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_device.c |  2 +-
 drivers/target/target_core_iblock.c | 44 +++--
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index 8add07f38..a5762e6 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -851,7 +851,7 @@ bool target_configure_unmap_from_queue(struct se_dev_attrib 
*attrib,
attrib->unmap_granularity = q->limits.discard_granularity / block_size;
attrib->unmap_granularity_alignment = q->limits.discard_alignment /
block_size;
-   attrib->unmap_zeroes_data = 0;
+   attrib->unmap_zeroes_data = (q->limits.max_write_zeroes_sectors);
return true;
 }
 EXPORT_SYMBOL(target_configure_unmap_from_queue);
diff --git a/drivers/target/target_core_iblock.c 
b/drivers/target/target_core_iblock.c
index bb069eb..b204413 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -86,6 +86,7 @@ static int iblock_configure_device(struct se_device *dev)
struct block_device *bd = NULL;
struct blk_integrity *bi;
fmode_t mode;
+   unsigned int max_write_zeroes_sectors;
int ret = -ENOMEM;
 
if (!(ib_dev->ibd_flags & IBDF_HAS_UDEV_PATH)) {
@@ -129,7 +130,11 @@ static int iblock_configure_device(struct se_device *dev)
 * Enable write same emulation for IBLOCK and use 0x as
 * the smaller WRITE_SAME(10) only has a two-byte block count.
 */
-   dev->dev_attrib.max_write_same_len = 0x;
+   max_write_zeroes_sectors = bdev_write_zeroes_sectors(bd);
+   if (max_write_zeroes_sectors)
+   dev->dev_attrib.max_write_same_len = max_write_zeroes_sectors;
+   else
+   dev->dev_attrib.max_write_same_len = 0x;
 
if (blk_queue_nonrot(q))
dev->dev_attrib.is_nonrot = 1;
@@ -415,28 +420,31 @@ static void iblock_end_io_flush(struct bio *bio)
 }
 
 static sense_reason_t
-iblock_execute_write_same_direct(struct block_device *bdev, struct se_cmd *cmd)
+iblock_execute_zero_out(struct block_device *bdev, struct se_cmd *cmd)
 {
struct se_device *dev = cmd->se_dev;
struct scatterlist *sg = >t_data_sg[0];
-   struct page *page = NULL;
-   int ret;
+   unsigned char *buf, zero = 0x00, *p = 
+   int rc, ret;
 
-   if (sg->offset) {
-   page = alloc_page(GFP_KERNEL);
-   if (!page)
-   return TCM_OUT_OF_RESOURCES;
-   sg_copy_to_buffer(sg, cmd->t_data_nents, page_address(page),
- dev->dev_attrib.block_size);
-   }
+   buf = kmap(sg_page(sg)) + sg->offset;
+   if (!buf)
+   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+   /*
+* Fall back to block_execute_write_same() slow-path if
+* incoming WRITE_SAME payload does not contain zeros.
+*/
+   rc = memcmp(buf, p, cmd->data_length);
+   kunmap(sg_page(sg));
+
+   if (rc)
+   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
-   ret = blkdev_issue_write_same(bdev,
+   ret = blkdev_issue_zeroout(bdev,
target_to_linux_sector(dev, cmd->t_task_lba),
target_to_linux_sector(dev,
sbc_get_write_same_sectors(cmd)),
-   GFP_KERNEL, page ? page : sg_page(sg));
-   if (page)
-   __free_page(page);
+   GFP_KERNEL, false);
if (ret)
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
@@ -472,8 +480,10 @@ static void iblock_end_io_flush(struct bio *bio)
return TCM_INVALID_CDB_FIELD;
}
 
-   if (bdev_write_same(bdev))
-   return iblock_execute_write_same_direct(bdev, cmd);
+   if (bdev_write_zeroes_sectors(bdev)) {
+ 

[ufs]: [scsi]: BUG: spinlock recursion on CPU#4

[Asutosh Das (asd)]

Hi All,

Recently, I came across an issue with the below call stack.

-000|arch_counter_get_cntvct(inline)
-000|__delay()
-001|__const_udelay(?)
-002|msm_trigger_wdog_bite()
-003|spin_dump(inline)
-003|spin_bug(lock = ?, ?)
-004|current_thread_info(inline)
-004|debug_spin_lock_before(inline)
-004|do_raw_spin_lock()
-005|raw_spin_lock_irqsave(lock = ?)
-006|blk_end_bidi_request(inline)
-006|blk_end_request_all(rq = ?, error = 0) <-- this tries to acquire 
the lock acquired by blk_delay_work (-024) and spinbug recursion occurs


-007|dm_end_request(clone = ?, error = 0)
-008|dm_done(inline)
-008|dm_softirq_done()
-009|blk_done_softirq()
-010|__read_once_size(inline)
-010|static_key_count(inline)
-010|static_key_false(inline)
-010|trace_softirq_exit(inline)
-010|__do_softirq()
-011|do_softirq_own_stack(inline)
-011|invoke_softirq(inline) <-- softirq is triggered because 
scsi_request_fn (-016) enabled interrupts on this cpu


-011|irq_exit()
-012|handle_IPI()
-013|gic_handle_irq()
-014|el1_irq(asm)
 -->|exception
-015|__raw_spin_unlock_irq(inline)
-015|raw_spin_unlock_irq(lock = ?)
-016|scsi_request_fn() <-- Unlocks the queue using spin_unlock, doesn't 
restore the flags, thus enabling the interrupts


-017|__blk_run_queue_uncond(inline)
-017|__blk_run_queue(q = ?)
-018|__elv_add_request()
-019|blk_insert_cloned_request() <-- acquires the queue lock & saves the 
flags


-020|dm_dispatch_clone_request(clone = ?, rq = ?)
-021|map_request()
-022|dm_request_fn()
-023|__blk_run_queue_uncond(inline)
-023|__blk_run_queue
-024|spin_unlock_irq(inline)
-024|blk_delay_work(?) <-- also acquires a queue lock, but this is a 
different queue, blk_end_request_all will reference this queue


-025|__read_once_size(inline)
-025|static_key_count(inline)
-025|static_key_false(inline)
-025|trace_workqueue_execute_end(inline)
-025|process_one_work()
-026|worker_thread()
-027|kthread()
-028|ret_from_fork(asm)
 ---|end of frame

Please can you check if this is actually a bug and my understanding is 
correct.

If so, I can put up a patch for the same.

--
Asutosh Das (asd)

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH] target: Avoid target_shutdown_sessions loop during queue_depth change

[Christoph Hellwig]

How about this slightly easier to read version?

---
>From 75276cd521ccecba244c1ee6c1100e27518c628d Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger 
Date: Thu, 1 Jun 2017 06:54:06 +
Subject: target: Avoid target_shutdown_sessions loop during queue_depth change

When target_shutdown_sessions() is invoked to shutdown all active
sessions associated with a se_node_acl when se_node_acl->queue_depth
is changed via core_tpg_set_initiator_node_queue_depth(), it's
possible that new connections reconnect immediately after explicit
shutdown occurs via target_shutdown_sessions().

Which means it's possible for the newly reconnected session with
the proper queue_depth can be shutdown multiple times when
target_shutdown_sessions() loops to drain all active sessions
for all cases.

This was regression was introduced by:

  commit bc6e6bb470eda42f44bcac96c261cff1216577b3
  Author: Christoph Hellwig 
  Date:   Mon May 2 15:45:19 2016 +0200

  target: consolidate and fix session shutdown

To avoid this case, instead move sessions into a local list and
avoid draining the same session multiple times when invoked via
core_tpg_set_initiator_node_queue_depth(), but still loop during
normal se_node_acl delete until all associated sessions have
been shutdown.

Cc: Christoph Hellwig 
Cc: Mike Christie 
Cc: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_tpg.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 310d9e55c6eb..74893acb8b5e 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -336,25 +336,32 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
return acl;
 }
 
-static void target_shutdown_sessions(struct se_node_acl *acl)
+static bool target_shutdown_sessions(struct se_node_acl *acl)
 {
struct se_session *sess;
unsigned long flags;
+   LIST_HEAD(tmp_list);
 
-restart:
spin_lock_irqsave(>nacl_sess_lock, flags);
list_for_each_entry(sess, >acl_sess_list, sess_acl_list) {
if (sess->sess_tearing_down)
continue;
 
+   list_move_tail(>sess_acl_list, _list);
+   }
+   spin_unlock_irqrestore(>nacl_sess_lock, flags);
+
+   if (list_empty(_list))
+   return true;
+
+   list_for_each_entry(sess, _list, sess_acl_list) {
list_del_init(>sess_acl_list);
-   spin_unlock_irqrestore(>nacl_sess_lock, flags);
 
if (acl->se_tpg->se_tpg_tfo->close_session)
acl->se_tpg->se_tpg_tfo->close_session(sess);
-   goto restart;
}
-   spin_unlock_irqrestore(>nacl_sess_lock, flags);
+
+   return false;
 }
 
 void core_tpg_del_initiator_node_acl(struct se_node_acl *acl)
@@ -367,7 +374,8 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
*acl)
list_del(>acl_list);
mutex_unlock(>acl_node_mutex);
 
-   target_shutdown_sessions(acl);
+   while (!target_shutdown_sessions(acl))
+   ;
 
target_put_nacl(acl);
/*
-- 
2.11.0

[PATCH] target: Avoid target_shutdown_sessions loop during queue_depth change

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

When target_shutdown_sessions() is invoked to shutdown all active
sessions associated with a se_node_acl when se_node_acl->queue_depth
is changed via core_tpg_set_initiator_node_queue_depth(), it's
possible that new connections reconnect immediately after explicit
shutdown occurs via target_shutdown_sessions().

Which means it's possible for the newly reconnected session with
the proper queue_depth can be shutdown multiple times when
target_shutdown_sessions() loops to drain all active sessions
for all cases.

This was regression was introduced by:

  commit bc6e6bb470eda42f44bcac96c261cff1216577b3
  Author: Christoph Hellwig 
  Date:   Mon May 2 15:45:19 2016 +0200

  target: consolidate and fix session shutdown

To avoid this case, instead move sessions into a local list and
avoid draining the same session multiple times when invoked via
core_tpg_set_initiator_node_queue_depth(), but still loop during
normal se_node_acl delete until all associated sessions have
been shutdown.

Cc: Christoph Hellwig 
Cc: Mike Christie 
Cc: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_tpg.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 310d9e5..7a7c2b5 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -336,10 +336,11 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
return acl;
 }
 
-static void target_shutdown_sessions(struct se_node_acl *acl)
+static void target_shutdown_sessions(struct se_node_acl *acl, bool do_restart)
 {
struct se_session *sess;
unsigned long flags;
+   LIST_HEAD(tmp_list);
 
 restart:
spin_lock_irqsave(>nacl_sess_lock, flags);
@@ -347,14 +348,22 @@ static void target_shutdown_sessions(struct se_node_acl 
*acl)
if (sess->sess_tearing_down)
continue;
 
+   list_move_tail(>sess_acl_list, _list);
+   }
+   spin_unlock_irqrestore(>nacl_sess_lock, flags);
+
+   if (list_empty(_list))
+   return;
+
+   list_for_each_entry(sess, _list, sess_acl_list) {
list_del_init(>sess_acl_list);
-   spin_unlock_irqrestore(>nacl_sess_lock, flags);
 
if (acl->se_tpg->se_tpg_tfo->close_session)
acl->se_tpg->se_tpg_tfo->close_session(sess);
-   goto restart;
}
-   spin_unlock_irqrestore(>nacl_sess_lock, flags);
+
+   if (do_restart)
+   goto restart;
 }
 
 void core_tpg_del_initiator_node_acl(struct se_node_acl *acl)
@@ -367,7 +376,7 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
*acl)
list_del(>acl_list);
mutex_unlock(>acl_node_mutex);
 
-   target_shutdown_sessions(acl);
+   target_shutdown_sessions(acl, true);
 
target_put_nacl(acl);
/*
@@ -414,7 +423,7 @@ int core_tpg_set_initiator_node_queue_depth(
/*
 * Shutdown all pending sessions to force session reinstatement.
 */
-   target_shutdown_sessions(acl);
+   target_shutdown_sessions(acl, false);
 
pr_debug("Successfully changed queue depth to: %d for Initiator"
" Node: %s on %s Target Portal Group: %u\n", acl->queue_depth,
-- 
1.9.1

Re: [PATCH 2/8] target: remove iblock WRITE_SAME passthrough support

[Christoph Hellwig]

On Wed, May 31, 2017 at 11:27:01PM -0700, Nicholas A. Bellinger wrote:
> Hey HCH & Jens,
> 
> Is this already queued up for v4.13 to address the missing LBPRZ feature
> bit..?
> 
> If not, I'll happy to take it via target-pending along with the
> following to re-enable it via max_write_zeroes_sectors.

It's not queued up.  Feel free to take it through the target tree.

Re: [PATCH 2/8] target: remove iblock WRITE_SAME passthrough support

[Nicholas A. Bellinger]

Hey HCH & Jens,

Is this already queued up for v4.13 to address the missing LBPRZ feature
bit..?

If not, I'll happy to take it via target-pending along with the
following to re-enable it via max_write_zeroes_sectors.

diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index d2f089c..e7caf78 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -851,7 +851,7 @@ bool target_configure_unmap_from_queue(struct se_dev_attrib 
*attrib,
attrib->unmap_granularity = q->limits.discard_granularity / block_size;
attrib->unmap_granularity_alignment = q->limits.discard_alignment /
block_size;
-   attrib->unmap_zeroes_data = 0;
+   attrib->unmap_zeroes_data = (q->limits.max_write_zeroes_sectors);
return true;
 }
 EXPORT_SYMBOL(target_configure_unmap_from_queue);

Any objections..?

On Tue, 2017-04-11 at 22:30 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2017-04-10 at 18:08 +0200, Christoph Hellwig wrote:
> > Use the pscsi driver to support arbitrary command passthrough
> > instead.
> > 
> 
> The people who are actively using iblock_execute_write_same_direct() are
> doing so in the context of ESX VAAI BlockZero, together with
> EXTENDED_COPY and COMPARE_AND_WRITE primitives.  Just using PSCSI is not
> an option for them.
> 
> In practice though I've not seen any users of IBLOCK WRITE_SAME for
> anything other than VAAI BlockZero, so just using blkdev_issue_zeroout()
> when available, and falling back to iblock_execute_write_same() if the
> WRITE_SAME buffer contains anything other than zeros should be OK.
> 
> How about something like the following below..?
> 
> This would bring parity to how blkdev_issue_write_same() works atm wrt
> to synchronous bio completions.  However, most folks with a raw
> make_request or blk-mq backend driver that supports multiple GB/sec of
> zero bandwidth end up changing IBLOCK to support asynchronous
> REQ_WRITE_SAME completions anyways.
> 
> I'd be happy to add support for that using __blkdev_issue_zeroout() once
> the basic conversion is in place.
> 
> From ff74012eaff38f9fa0d74aca60507b9964f484ce Mon Sep 17 00:00:00 2001
> From: Nicholas Bellinger 
> Date: Tue, 11 Apr 2017 22:21:47 -0700
> Subject: [PATCH] target/iblock: Convert WRITE_SAME to blkdev_issue_zeroout
> 
> Signed-off-by: Nicholas Bellinger 
> ---
>  drivers/target/target_core_iblock.c | 44 
> +++--
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/target/target_core_iblock.c 
> b/drivers/target/target_core_iblock.c
> index d316ed5..5bfde20 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
> @@ -86,6 +86,7 @@ static int iblock_configure_device(struct se_device *dev)
>   struct block_device *bd = NULL;
>   struct blk_integrity *bi;
>   fmode_t mode;
> + unsigned int max_write_zeroes_sectors;
>   int ret = -ENOMEM;
>  
>   if (!(ib_dev->ibd_flags & IBDF_HAS_UDEV_PATH)) {
> @@ -129,7 +130,11 @@ static int iblock_configure_device(struct se_device *dev)
>* Enable write same emulation for IBLOCK and use 0x as
>* the smaller WRITE_SAME(10) only has a two-byte block count.
>*/
> - dev->dev_attrib.max_write_same_len = 0x;
> + max_write_zeroes_sectors = bdev_write_zeroes_sectors(bd);
> + if (max_write_zeroes_sectors)
> + dev->dev_attrib.max_write_same_len = max_write_zeroes_sectors;
> + else
> + dev->dev_attrib.max_write_same_len = 0x;
>  
>   if (blk_queue_nonrot(q))
>   dev->dev_attrib.is_nonrot = 1;
> @@ -415,28 +420,31 @@ static void iblock_end_io_flush(struct bio *bio)
>  }
>  
>  static sense_reason_t
> -iblock_execute_write_same_direct(struct block_device *bdev, struct se_cmd 
> *cmd)
> +iblock_execute_zero_out(struct block_device *bdev, struct se_cmd *cmd)
>  {
>   struct se_device *dev = cmd->se_dev;
>   struct scatterlist *sg = >t_data_sg[0];
> - struct page *page = NULL;
> - int ret;
> + unsigned char *buf, zero = 0x00, *p = 
> + int rc, ret;
>  
> - if (sg->offset) {
> - page = alloc_page(GFP_KERNEL);
> - if (!page)
> - return TCM_OUT_OF_RESOURCES;
> - sg_copy_to_buffer(sg, cmd->t_data_nents, page_address(page),
> -   dev->dev_attrib.block_size);
> - }
> + buf = kmap(sg_page(sg)) + sg->offset;
> + if (!buf)
> + return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> + /*
> +  * Fall back to block_execute_write_same() slow-path if
> +  * incoming WRITE_SAME payload does not contain zeros.
> +  */
> + rc = memcmp(buf, p, cmd->data_length);
> + kunmap(sg_page(sg));
> +
> + if (rc)
> + return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;

Re: [PATCH] iscsi: Fix a sleep-in-atomic bug

[Nicholas A. Bellinger]

Hi Jia-Ju,

On Wed, 2017-05-31 at 11:26 +0800, Jia-Ju Bai wrote:
> The driver may sleep under a spin lock, and the function call path is:
> iscsit_tpg_enable_portal_group (acquire the lock by spin_lock)
>   iscsi_update_param_value
> kstrdup(GFP_KERNEL) --> may sleep
> 
> To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC".
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/target/iscsi/iscsi_target_parameters.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Btw, the use of tpg->tpg_state_lock in iscsit_tpg_enable_portal_group()
while checking existing state and calling iscsi_update_param_value() is
not necessary, since lio_target_tpg_enable_store() is already holding
iscsit_get_tpg() -> tpg->tpg_access_lock.

How about the following instead to only take tpg->tpg_state_lock when
updating tpg->tpg_state instead..?

diff --git a/drivers/target/iscsi/iscsi_target_tpg.c 
b/drivers/target/iscsi/iscsi_target_tpg.c
index 2e7e08d..abaabba 100644
--- a/drivers/target/iscsi/iscsi_target_tpg.c
+++ b/drivers/target/iscsi/iscsi_target_tpg.c
@@ -311,11 +311,9 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
struct iscsi_tiqn *tiqn = tpg->tpg_tiqn;
int ret;
 
-   spin_lock(>tpg_state_lock);
if (tpg->tpg_state == TPG_STATE_ACTIVE) {
pr_err("iSCSI target portal group: %hu is already"
" active, ignoring request.\n", tpg->tpgt);
-   spin_unlock(>tpg_state_lock);
return -EINVAL;
}
/*
@@ -324,10 +322,8 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
 * is enforced (as per default), and remove the NONE option.
 */
param = iscsi_find_param_from_key(AUTHMETHOD, tpg->param_list);
-   if (!param) {
-   spin_unlock(>tpg_state_lock);
+   if (!param)
return -EINVAL;
-   }
 
if (tpg->tpg_attrib.authentication) {
if (!strcmp(param->value, NONE)) {
@@ -341,6 +337,7 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
goto err;
}
 
+   spin_lock(>tpg_state_lock);
tpg->tpg_state = TPG_STATE_ACTIVE;
spin_unlock(>tpg_state_lock);
 
@@ -353,7 +350,6 @@ int iscsit_tpg_enable_portal_group(struct 
iscsi_portal_group *tpg)
return 0;
 
 err:
-   spin_unlock(>tpg_state_lock);
return ret;
 }

Re: [PATCH] tcmu: Add fifo type waiter list support to avoid starvation

[Nicholas A. Bellinger]

Hey MNC,

Any comments on this..?

It's been sitting on the list for a while now..  ;)

On Fri, 2017-05-05 at 10:51 +0800, lixi...@cmss.chinamobile.com wrote:
> From: Xiubo Li 
> 
> The fifo type waiter list will hold the udevs who are waiting for the
> blocks from the data global pool. The unmap thread will try to feed the
> first udevs in waiter list, if the global free blocks available are
> not enough, it will stop traversing the list and abort waking up the
> others.
> 
> Signed-off-by: Xiubo Li 
> ---
>  drivers/target/target_core_user.c | 82 
> +++
>  1 file changed, 66 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/target/target_core_user.c 
> b/drivers/target/target_core_user.c
> index 9045837..10c99e7 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -97,6 +97,7 @@ struct tcmu_hba {
>  
>  struct tcmu_dev {
>   struct list_head node;
> + struct list_head waiter;
>  
>   struct se_device se_dev;
>  
> @@ -123,7 +124,7 @@ struct tcmu_dev {
>   wait_queue_head_t wait_cmdr;
>   struct mutex cmdr_lock;
>  
> - bool waiting_global;
> + uint32_t waiting_blocks;
>   uint32_t dbi_max;
>   uint32_t dbi_thresh;
>   DECLARE_BITMAP(data_bitmap, DATA_BLOCK_BITS);
> @@ -165,6 +166,10 @@ struct tcmu_cmd {
>  static DEFINE_MUTEX(root_udev_mutex);
>  static LIST_HEAD(root_udev);
>  
> +/* The data blocks global pool waiter list */
> +static DEFINE_MUTEX(root_udev_waiter_mutex);
> +static LIST_HEAD(root_udev_waiter);
> +
>  static atomic_t global_db_count = ATOMIC_INIT(0);
>  
>  static struct kmem_cache *tcmu_cmd_cache;
> @@ -195,6 +200,11 @@ enum tcmu_multicast_groups {
>  #define tcmu_cmd_set_dbi(cmd, index) ((cmd)->dbi[(cmd)->dbi_cur++] = (index))
>  #define tcmu_cmd_get_dbi(cmd) ((cmd)->dbi[(cmd)->dbi_cur++])
>  
> +static inline bool is_in_waiter_list(struct tcmu_dev *udev)
> +{
> + return !!udev->waiting_blocks;
> +}
> +
>  static void tcmu_cmd_free_data(struct tcmu_cmd *tcmu_cmd, uint32_t len)
>  {
>   struct tcmu_dev *udev = tcmu_cmd->tcmu_dev;
> @@ -250,8 +260,6 @@ static bool tcmu_get_empty_blocks(struct tcmu_dev *udev,
>  {
>   int i;
>  
> - udev->waiting_global = false;
> -
>   for (i = tcmu_cmd->dbi_cur; i < tcmu_cmd->dbi_cnt; i++) {
>   if (!tcmu_get_empty_block(udev, tcmu_cmd))
>   goto err;
> @@ -259,9 +267,7 @@ static bool tcmu_get_empty_blocks(struct tcmu_dev *udev,
>   return true;
>  
>  err:
> - udev->waiting_global = true;
> - /* Try to wake up the unmap thread */
> - wake_up(_wait);
> + udev->waiting_blocks += tcmu_cmd->dbi_cnt - i;
>   return false;
>  }
>  
> @@ -671,6 +677,7 @@ static inline size_t tcmu_cmd_get_cmd_size(struct 
> tcmu_cmd *tcmu_cmd,
>  
>   while (!is_ring_space_avail(udev, tcmu_cmd, command_size, data_length)) 
> {
>   int ret;
> + bool is_waiting_blocks = !!udev->waiting_blocks;
>   DEFINE_WAIT(__wait);
>  
>   prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
> @@ -688,6 +695,18 @@ static inline size_t tcmu_cmd_get_cmd_size(struct 
> tcmu_cmd *tcmu_cmd,
>   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>   }
>  
> + /*
> +  * Try to insert the current udev to waiter list and
> +  * then wake up the unmap thread
> +  */
> + if (is_waiting_blocks) {
> + mutex_lock(_udev_waiter_mutex);
> + list_add_tail(>waiter, _udev_waiter);
> + mutex_unlock(_udev_waiter_mutex);
> +
> + wake_up(_wait);
> + }
> +
>   mutex_lock(>cmdr_lock);
>  
>   /* We dropped cmdr_lock, cmd_head is stale */
> @@ -902,8 +921,6 @@ static unsigned int tcmu_handle_completions(struct 
> tcmu_dev *udev)
>   if (mb->cmd_tail == mb->cmd_head)
>   del_timer(>timeout); /* no more pending cmds */
>  
> - wake_up(>wait_cmdr);
> -
>   return handled;
>  }
>  
> @@ -996,7 +1013,17 @@ static int tcmu_irqcontrol(struct uio_info *info, s32 
> irq_on)
>   struct tcmu_dev *tcmu_dev = container_of(info, struct tcmu_dev, 
> uio_info);
>  
>   mutex_lock(_dev->cmdr_lock);
> - tcmu_handle_completions(tcmu_dev);
> + /*
> +  * If the current udev is also in waiter list, this will
> +  * make sure that the other waiters in list be feeded ahead
> +  * of it.
> +  */
> + if (is_in_waiter_list(tcmu_dev)) {
> + wake_up(_wait);
> + } else {
> + tcmu_handle_completions(tcmu_dev);
> + wake_up(_dev->wait_cmdr);
> + }
>   mutex_unlock(_dev->cmdr_lock);
>  
>   return 0;
> @@ -1231,7 +1258,7 @@ static int tcmu_configure_device(struct se_device *dev)
>   udev->data_off = CMDR_SIZE;
>