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

[Martin K. Petersen]

Bart,

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

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 12/31] bsg: Check queue type before attaching to a queue

[Martin K. Petersen]

Bart,

> Since BSG only supports request queues for which struct scsi_request
> is the first member of their private request data, refuse to register
> block layer queues for which struct scsi_request is not the first
> member of their private data.

> +
> + if (!blk_queue_scsi_sup(rq)) {

If you are renaming the flag, how about blk_queue_scsi_pdu()?

> + WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
>   if (!blk_get_queue(rq))
>   return ERR_PTR(-ENXIO);

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 0/7] qla2xxx: Bug Fixes for driver.

[Martin K. Petersen]

Himanshu,

> I have reduced the series for 4.12 rc merge to 1-10 patches that were
> submitted earlier.
>
> Changes from v1 --> v2
> o Drop patches that can be queued for 4.13 scsi-misc merge and will be
>   sent as new series.
> o Addressed commit summary of patches from Bart's review where
>   applicable.
>
> Please include them in 4.12.0-rc3 fixes at your earliest convenience.

Applied to 4.12/scsi-fixes. Thanks much!

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH v2 0/7] qla2xxx: Bug Fixes for driver.

[Himanshu Madhani]

Hi Martin,

I have reduced the series for 4.12 rc merge to 1-10 patches that
were submitted earlier.

Changes from v1 --> v2
o Drop patches that can be queued for 4.13 scsi-misc merge and will be
  sent as new series.
o Addressed commit summary of patches from Bart's review where applicable.

Please include them in 4.12.0-rc3 fixes at your earliest convenience.

Thanks,
Himanshu

Himanshu Madhani (1):
  qla2xxx: Fix recursive loop during target mode configuration for
ISP25XX leaving system unresponsive.

Joe Carnuccio (4):
  qla2xxx: Modify T262 FW dump template to specify same start/end to
debug customer issues.
  qla2xxx: Set bit 15 for DIAG_ECHO_TEST MBC.
  qla2xxx: Fix mailbox pointer error in fwdump capture.
  qla2xxx: Fix crash due to NULL pointer dereference of ctx.

Quinn Tran (1):
  qla2xxx: Fix NULL pointer access due to redundant fc_host_port_name
call

Sawan Chandak (1):
  qla2xxx: Fix crash due to mismatch mumber of Q-pair creation for Multi
queue

 drivers/scsi/qla2xxx/qla_bsg.c|  9 +
 drivers/scsi/qla2xxx/qla_dbg.c|  4 ++--
 drivers/scsi/qla2xxx/qla_def.h|  1 +
 drivers/scsi/qla2xxx/qla_init.c   |  5 -
 drivers/scsi/qla2xxx/qla_inline.h | 26 +++---
 drivers/scsi/qla2xxx/qla_isr.c|  2 +-
 drivers/scsi/qla2xxx/qla_mbx.c| 13 ++---
 drivers/scsi/qla2xxx/qla_os.c | 30 +++---
 drivers/scsi/qla2xxx/qla_target.c |  8 +---
 drivers/scsi/qla2xxx/qla_tmpl.c   |  2 +-
 10 files changed, 47 insertions(+), 53 deletions(-)

-- 
2.12.0

[PATCH v2 2/7] qla2xxx: Fix NULL pointer access due to redundant fc_host_port_name call

[Himanshu Madhani]

From: Quinn Tran 

Remove redundant fc_host_port_name calls to prevent
early access of scsi_host->shost_data buffer. This
prevent null pointer access.

Following stack trace is seen

BUG: unable to handle kernel NULL pointer dereference at 08
IP: qla24xx_report_id_acquisition+0x22d/0x3a0 [qla2xxx]

Cc:  # 4.11
Signed-off-by: Quinn Tran 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_mbx.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index a113ab3592a7..12fea77e31c6 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -3676,15 +3676,6 @@ qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
qlt_update_host_map(vha, id);
}
 
-   fc_host_port_name(vha->host) =
-   wwn_to_u64(vha->port_name);
-
-   if (qla_ini_mode_enabled(vha))
-   ql_dbg(ql_dbg_mbx, vha, 0x1018,
-   "FA-WWN portname %016llx (%x)\n",
-   fc_host_port_name(vha->host),
-   rptid_entry->vp_status);
-
set_bit(REGISTER_FC4_NEEDED, >dpc_flags);
set_bit(REGISTER_FDMI_NEEDED, >dpc_flags);
} else {
-- 
2.12.0

[PATCH v2 1/7] qla2xxx: Fix recursive loop during target mode configuration for ISP25XX leaving system unresponsive.

[Himanshu Madhani]

Following messages are seen into system logs

qla2xxx [:09:00.0]-00af:9: Performing ISP error recovery -
ha=98315ee3.
qla2xxx [:09:00.0]-504b:9: RISC paused -- HCCR=40, Dumping firmware.
qla2xxx [:09:00.0]-d009:9: Firmware has been previously dumped
(ba488c001000) -- ignoring request.
qla2xxx [:09:00.0]-504b:9: RISC paused -- HCCR=40, Dumping firmware.

See Bugzilla for details
https://bugzilla.kernel.org/show_bug.cgi?id=195285

Fixes: d74595278f4ab ("scsi: qla2xxx: Add multiple queue pair functionality.")
Cc:  # 4.10
Reported-by: Laurence Oberman 
Reported-by: Anthony Bloodoff 
Tested-by: Laurence Oberman 
Tested-by: Anthony Bloodoff 
Signed-off-by: Himanshu Madhani 
Signed-off-by: Giridhar Malavali 
---
 drivers/scsi/qla2xxx/qla_isr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index aac03504d9a3..2572121b765b 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -3282,7 +3282,7 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct 
rsp_que *rsp)
}
 
/* Enable MSI-X vector for response queue update for queue 0 */
-   if (IS_QLA83XX(ha) || IS_QLA27XX(ha)) {
+   if (IS_QLA25XX(ha) || IS_QLA83XX(ha) || IS_QLA27XX(ha)) {
if (ha->msixbase && ha->mqiobase &&
(ha->max_rsp_queues > 1 || ha->max_req_queues > 1 ||
 ql2xmqsupport))
-- 
2.12.0

[PATCH v2 4/7] qla2xxx: Modify T262 FW dump template to specify same start/end to debug customer issues.

[Himanshu Madhani]

From: Joe Carnuccio 

Firmware dump allows for debugging customer issues. This patch fixes
start/end pointer calculation to capture T262 template entryfor dump
tool.

Cc:  # 4.10
Signed-off-by: Joe Carnuccio 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_tmpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c
index 8a58ef3adab4..c197972a3e2d 100644
--- a/drivers/scsi/qla2xxx/qla_tmpl.c
+++ b/drivers/scsi/qla2xxx/qla_tmpl.c
@@ -371,7 +371,7 @@ qla27xx_fwdt_entry_t262(struct scsi_qla_host *vha,
goto done;
}
 
-   if (end <= start || start == 0 || end == 0) {
+   if (end < start || start == 0 || end == 0) {
ql_dbg(ql_dbg_misc, vha, 0xd023,
"%s: unusable range (start=%x end=%x)\n", __func__,
ent->t262.end_addr, ent->t262.start_addr);
-- 
2.12.0

[PATCH v2 3/7] qla2xxx: Fix crash due to mismatch mumber of Q-pair creation for Multi queue

[Himanshu Madhani]

From: Sawan Chandak 

when driver is loaded with Multi Queue enabled, it was
noticed that there was one less queue pair created.

Following message would indicate this

"No resources to create additional q pair."

The result of one less queue pair means that system can crash,
if the block mq layer thinks there is an extra hardware queue
available, and the driver will use a NULL ptr qpair in that instance.

Following stack trace is seen in one of the crash

irq_create_affinity_masks+0x98/0x530
irq_create_affinity_masks+0x98/0x530
__pci_enable_msix+0x321/0x4e0
mutex_lock+0x12/0x40
pci_alloc_irq_vectors_affinity+0xb5/0x140
qla24xx_enable_msix+0x79/0x530 [qla2xxx]
qla2x00_request_irqs+0x61/0x2d0 [qla2xxx]
qla2x00_probe_one+0xc73/0x2390 [qla2xxx]
ida_simple_get+0x98/0x100
kernfs_next_descendant_post+0x40/0x50
local_pci_probe+0x45/0xa0
pci_device_probe+0xfc/0x140
driver_probe_device+0x2c5/0x470
__driver_attach+0xdd/0xe0
driver_probe_device+0x470/0x470
bus_for_each_dev+0x6c/0xc0
driver_attach+0x1e/0x20
bus_add_driver+0x45/0x270
driver_register+0x60/0xe0
__pci_register_driver+0x4c/0x50
qla2x00_module_init+0x1ce/0x21e [qla2xxx]

Cc:  # 4.10
Signed-off-by: Sawan Chandak 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_def.h  | 1 +
 drivers/scsi/qla2xxx/qla_init.c | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index ae119018dfaa..eddbc1218a39 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3425,6 +3425,7 @@ struct qla_hw_data {
uint8_t max_req_queues;
uint8_t max_rsp_queues;
uint8_t max_qpairs;
+   uint8_t num_qpairs;
struct qla_qpair *base_qpair;
struct qla_npiv_entry *npiv_info;
uint16_tnvram_npiv_size;
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 034743309ada..0391fc317003 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -7543,12 +7543,13 @@ struct qla_qpair *qla2xxx_create_qpair(struct 
scsi_qla_host *vha, int qos, int v
/* Assign available que pair id */
mutex_lock(>mq_lock);
qpair_id = find_first_zero_bit(ha->qpair_qid_map, 
ha->max_qpairs);
-   if (qpair_id >= ha->max_qpairs) {
+   if (ha->num_qpairs >= ha->max_qpairs) {
mutex_unlock(>mq_lock);
ql_log(ql_log_warn, vha, 0x0183,
"No resources to create additional q pair.\n");
goto fail_qid_map;
}
+   ha->num_qpairs++;
set_bit(qpair_id, ha->qpair_qid_map);
ha->queue_pair_map[qpair_id] = qpair;
qpair->id = qpair_id;
@@ -7635,6 +7636,7 @@ struct qla_qpair *qla2xxx_create_qpair(struct 
scsi_qla_host *vha, int qos, int v
 fail_msix:
ha->queue_pair_map[qpair_id] = NULL;
clear_bit(qpair_id, ha->qpair_qid_map);
+   ha->num_qpairs--;
mutex_unlock(>mq_lock);
 fail_qid_map:
kfree(qpair);
@@ -7660,6 +7662,7 @@ int qla2xxx_delete_qpair(struct scsi_qla_host *vha, 
struct qla_qpair *qpair)
mutex_lock(>mq_lock);
ha->queue_pair_map[qpair->id] = NULL;
clear_bit(qpair->id, ha->qpair_qid_map);
+   ha->num_qpairs--;
list_del(>qp_list_elem);
if (list_empty(>qp_list))
vha->flags.qpairs_available = 0;
-- 
2.12.0

[PATCH v2 6/7] qla2xxx: Fix mailbox pointer error in fwdump capture.

[Himanshu Madhani]

From: Joe Carnuccio 

Cc:  # 4.10
Signed-off-by: Joe Carnuccio 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_dbg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 51b4179469d1..88748a6ab73f 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -1131,7 +1131,7 @@ qla24xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked)
 
/* Mailbox registers. */
mbx_reg = >mailbox0;
-   for (cnt = 0; cnt < sizeof(fw->mailbox_reg) / 2; cnt++, dmp_reg++)
+   for (cnt = 0; cnt < sizeof(fw->mailbox_reg) / 2; cnt++, mbx_reg++)
fw->mailbox_reg[cnt] = htons(RD_REG_WORD(mbx_reg));
 
/* Transfer sequence registers. */
@@ -2090,7 +2090,7 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked)
 
/* Mailbox registers. */
mbx_reg = >mailbox0;
-   for (cnt = 0; cnt < sizeof(fw->mailbox_reg) / 2; cnt++, dmp_reg++)
+   for (cnt = 0; cnt < sizeof(fw->mailbox_reg) / 2; cnt++, mbx_reg++)
fw->mailbox_reg[cnt] = htons(RD_REG_WORD(mbx_reg));
 
/* Transfer sequence registers. */
-- 
2.12.0

[PATCH v2 7/7] qla2xxx: Fix crash due to NULL pointer dereference of ctx.

[Himanshu Madhani]

From: Joe Carnuccio 

Fixes following signature in the stack trace:

BUG: unable to handle kernel NULL pointer dereference at 0374
IP: [] qla2x00_sp_free_dma+0xeb/0x2a0 [qla2xxx]

Cc:  # 4.10
Signed-off-by: Joe Carnuccio 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_inline.h | 26 +++---
 drivers/scsi/qla2xxx/qla_os.c | 30 +++---
 drivers/scsi/qla2xxx/qla_target.c |  8 +---
 3 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_inline.h 
b/drivers/scsi/qla2xxx/qla_inline.h
index 66df6cec59da..c61a6a871c8e 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -129,28 +129,16 @@ qla2x00_clear_loop_id(fc_port_t *fcport) {
 }
 
 static inline void
-qla2x00_clean_dsd_pool(struct qla_hw_data *ha, srb_t *sp,
-   struct qla_tgt_cmd *tc)
+qla2x00_clean_dsd_pool(struct qla_hw_data *ha, struct crc_context *ctx)
 {
-   struct dsd_dma *dsd_ptr, *tdsd_ptr;
-   struct crc_context *ctx;
-
-   if (sp)
-   ctx = (struct crc_context *)GET_CMD_CTX_SP(sp);
-   else if (tc)
-   ctx = (struct crc_context *)tc->ctx;
-   else {
-   BUG();
-   return;
-   }
+   struct dsd_dma *dsd, *tdsd;
 
/* clean up allocated prev pool */
-   list_for_each_entry_safe(dsd_ptr, tdsd_ptr,
-   >dsd_list, list) {
-   dma_pool_free(ha->dl_dma_pool, dsd_ptr->dsd_addr,
-   dsd_ptr->dsd_list_dma);
-   list_del(_ptr->list);
-   kfree(dsd_ptr);
+   list_for_each_entry_safe(dsd, tdsd, >dsd_list, list) {
+   dma_pool_free(ha->dl_dma_pool, dsd->dsd_addr,
+   dsd->dsd_list_dma);
+   list_del(>list);
+   kfree(dsd);
}
INIT_LIST_HEAD(>dsd_list);
 }
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 1c7957903283..c8282a1ab6dc 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -630,29 +630,34 @@ qla2x00_sp_free_dma(void *ptr)
sp->flags &= ~SRB_CRC_PROT_DMA_VALID;
}
 
+   if (!ctx)
+   goto end;
+
if (sp->flags & SRB_CRC_CTX_DSD_VALID) {
/* List assured to be having elements */
-   qla2x00_clean_dsd_pool(ha, sp, NULL);
+   qla2x00_clean_dsd_pool(ha, ctx);
sp->flags &= ~SRB_CRC_CTX_DSD_VALID;
}
 
if (sp->flags & SRB_CRC_CTX_DMA_VALID) {
-   dma_pool_free(ha->dl_dma_pool, ctx,
-   ((struct crc_context *)ctx)->crc_ctx_dma);
+   struct crc_context *ctx0 = ctx;
+
+   dma_pool_free(ha->dl_dma_pool, ctx0, ctx0->crc_ctx_dma);
sp->flags &= ~SRB_CRC_CTX_DMA_VALID;
}
 
if (sp->flags & SRB_FCP_CMND_DMA_VALID) {
-   struct ct6_dsd *ctx1 = (struct ct6_dsd *)ctx;
+   struct ct6_dsd *ctx1 = ctx;
 
dma_pool_free(ha->fcp_cmnd_dma_pool, ctx1->fcp_cmnd,
-   ctx1->fcp_cmnd_dma);
+   ctx1->fcp_cmnd_dma);
list_splice(>dsd_list, >gbl_dsd_list);
ha->gbl_dsd_inuse -= ctx1->dsd_use_cnt;
ha->gbl_dsd_avail += ctx1->dsd_use_cnt;
mempool_free(ctx1, ha->ctx_mempool);
}
 
+end:
CMD_SP(cmd) = NULL;
qla2x00_rel_sp(sp);
 }
@@ -699,21 +704,24 @@ qla2xxx_qpair_sp_free_dma(void *ptr)
sp->flags &= ~SRB_CRC_PROT_DMA_VALID;
}
 
+   if (!ctx)
+   goto end;
+
if (sp->flags & SRB_CRC_CTX_DSD_VALID) {
/* List assured to be having elements */
-   qla2x00_clean_dsd_pool(ha, sp, NULL);
+   qla2x00_clean_dsd_pool(ha, ctx);
sp->flags &= ~SRB_CRC_CTX_DSD_VALID;
}
 
if (sp->flags & SRB_CRC_CTX_DMA_VALID) {
-   dma_pool_free(ha->dl_dma_pool, ctx,
-   ((struct crc_context *)ctx)->crc_ctx_dma);
+   struct crc_context *ctx0 = ctx;
+
+   dma_pool_free(ha->dl_dma_pool, ctx, ctx0->crc_ctx_dma);
sp->flags &= ~SRB_CRC_CTX_DMA_VALID;
}
 
if (sp->flags & SRB_FCP_CMND_DMA_VALID) {
-   struct ct6_dsd *ctx1 = (struct ct6_dsd *)ctx;
-
+   struct ct6_dsd *ctx1 = ctx;
dma_pool_free(ha->fcp_cmnd_dma_pool, ctx1->fcp_cmnd,
ctx1->fcp_cmnd_dma);
list_splice(>dsd_list, >gbl_dsd_list);
@@ -721,7 +729,7 @@ qla2xxx_qpair_sp_free_dma(void *ptr)
ha->gbl_dsd_avail += ctx1->dsd_use_cnt;
mempool_free(ctx1, ha->ctx_mempool);
}
-
+end:
CMD_SP(cmd) = NULL;
qla2xxx_rel_qpair_sp(sp->qpair, sp);
 }
diff --git 

[PATCH v2 5/7] qla2xxx: Set bit 15 for DIAG_ECHO_TEST MBC.

[Himanshu Madhani]

From: Joe Carnuccio 

Set bit (BIT_15) to send right ECHO payload information
for Diagnostic Echo Test command.

Cc:  # 4.10
Signed-off-by: Joe Carnuccio 
Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_bsg.c | 9 +
 drivers/scsi/qla2xxx/qla_mbx.c | 4 ++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 16d1cd50feed..ca3420de5a01 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -730,6 +730,8 @@ qla2x00_process_loopback(struct bsg_job *bsg_job)
return -EIO;
}
 
+   memset(, 0, sizeof(elreq));
+
elreq.req_sg_cnt = dma_map_sg(>pdev->dev,
bsg_job->request_payload.sg_list, 
bsg_job->request_payload.sg_cnt,
DMA_TO_DEVICE);
@@ -795,10 +797,9 @@ qla2x00_process_loopback(struct bsg_job *bsg_job)
 
if (atomic_read(>loop_state) == LOOP_READY &&
(ha->current_topology == ISP_CFG_F ||
-   ((IS_QLA81XX(ha) || IS_QLA8031(ha) || IS_QLA8044(ha)) &&
-   le32_to_cpu(*(uint32_t *)req_data) == ELS_OPCODE_BYTE
-   && req_data_len == MAX_ELS_FRAME_PAYLOAD)) &&
-   elreq.options == EXTERNAL_LOOPBACK) {
+   (le32_to_cpu(*(uint32_t *)req_data) == ELS_OPCODE_BYTE &&
+req_data_len == MAX_ELS_FRAME_PAYLOAD)) &&
+   elreq.options == EXTERNAL_LOOPBACK) {
type = "FC_BSG_HST_VENDOR_ECHO_DIAG";
ql_dbg(ql_dbg_user, vha, 0x701e,
"BSG request type: %s.\n", type);
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 12fea77e31c6..cba1fc5e8be9 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -4812,9 +4812,9 @@ qla2x00_echo_test(scsi_qla_host_t *vha, struct 
msg_echo_lb *mreq,
 
memset(mcp->mb, 0 , sizeof(mcp->mb));
mcp->mb[0] = MBC_DIAGNOSTIC_ECHO;
-   mcp->mb[1] = mreq->options | BIT_6; /* BIT_6 specifies 64bit 
address */
+   /* BIT_6 specifies 64bit address */
+   mcp->mb[1] = mreq->options | BIT_15 | BIT_6;
if (IS_CNA_CAPABLE(ha)) {
-   mcp->mb[1] |= BIT_15;
mcp->mb[2] = vha->fcoe_fcf_idx;
}
mcp->mb[16] = LSW(mreq->rcv_dma);
-- 
2.12.0

[GIT PULL] SCSI fixes for 4.12-rc2

[James Bottomley]

This is quite a big update because it includes a rework of the lpfc
driver to separate the NVMe part from the FC part.  The reason for
doing this is because two separate trees (the nvme and scsi trees
respectively) want to update the individual components and this
separation will prevent a really nasty cross tree entanglement by the
time we reach the next merge window.  The rest of the fixes are the
usual minor sort with no significant security implications.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Damien Le Moal (2):
  scsi: sd: Write lock zone for REQ_OP_WRITE_ZEROES
  scsi: sd: Unlock zone in case of error in sd_setup_write_same_cmnd()

Derek Basehore (1):
  scsi: sd: Ignore sync cache failures when not supported

Guilherme G. Piccoli (1):
  scsi: lpfc: Fix NULL pointer dereference during PCI error recovery

Gustavo A. R. Silva (1):
  scsi: libfc: fix incorrect variable assignment

James Smart (16):
  scsi: lpfc: fix build issue if NVME_FC_TARGET is not defined
  scsi: lpfc: update version to 11.2.0.14
  scsi: lpfc: Add MDS Diagnostic support.
  scsi: lpfc: Fix NVMEI's handling of NVMET's PRLI response attributes
  scsi: lpfc: Cleanup entry_repost settings on SLI4 queues
  scsi: lpfc: Fix debugfs root inode "lpfc" not getting deleted on driver 
unload.
  scsi: lpfc: Fix NVME I+T not registering NVME as a supported FC4 type
  scsi: lpfc: Added recovery logic for running out of NVMET IO context 
resources
  scsi: lpfc: Separate NVMET RQ buffer posting from IO resources 
SGL/iocbq/context
  scsi: lpfc: Separate NVMET data buffer pool fir ELS/CT.
  scsi: lpfc: Fix NMI watchdog assertions when running nvmet IOPS tests
  scsi: lpfc: Fix NVMEI driver not decrementing counter causing bad rport 
state.
  scsi: lpfc: Fix nvmet RQ resource needs for large block writes.
  scsi: lpfc: Adding additional stats counters for nvme.
  scsi: lpfc: Fix system crash when port is reset.
  scsi: lpfc: Fix used-RPI accounting problem.

Johannes Thumshirn (1):
  scsi: sg: don't return bogus Sg_requests

Long Li (1):
  scsi: zero per-cmd private driver data for each MQ I/O

Michał Potomski (1):
  scsi: ufs: Clean up some rpm/spm level SysFS nodes upon remove

Varun Prakash (1):
  scsi: csiostor: fix use after free in csio_hw_use_fwconfig()

With diffstat:

 drivers/scsi/csiostor/csio_hw.c|   5 +-
 drivers/scsi/libfc/fc_rport.c  |   2 +-
 drivers/scsi/lpfc/lpfc.h   |  23 ++-
 drivers/scsi/lpfc/lpfc_attr.c  |  47 +++--
 drivers/scsi/lpfc/lpfc_crtn.h  |  11 +-
 drivers/scsi/lpfc/lpfc_ct.c|   1 +
 drivers/scsi/lpfc/lpfc_debugfs.c   |  69 ---
 drivers/scsi/lpfc/lpfc_disc.h  |   1 +
 drivers/scsi/lpfc/lpfc_els.c   |  26 ++-
 drivers/scsi/lpfc/lpfc_hbadisc.c   |   9 +-
 drivers/scsi/lpfc/lpfc_hw4.h   |  16 +-
 drivers/scsi/lpfc/lpfc_init.c  | 137 
 drivers/scsi/lpfc/lpfc_mem.c   | 100 +++--
 drivers/scsi/lpfc/lpfc_nportdisc.c |   6 +
 drivers/scsi/lpfc/lpfc_nvmet.c | 414 ++---
 drivers/scsi/lpfc/lpfc_nvmet.h |  14 +-
 drivers/scsi/lpfc/lpfc_sli.c   | 357 +---
 drivers/scsi/lpfc/lpfc_sli4.h  |  19 +-
 drivers/scsi/lpfc/lpfc_version.h   |   2 +-
 drivers/scsi/scsi_lib.c|   2 +-
 drivers/scsi/sd.c  |  63 --
 drivers/scsi/sg.c  |   5 +-
 drivers/scsi/ufs/ufshcd.c  |   7 +
 23 files changed, 902 insertions(+), 434 deletions(-)

With full diff below

James

---

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 622bdabc8894..dab195f04da7 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -1769,7 +1769,6 @@ csio_hw_use_fwconfig(struct csio_hw *hw, int reset, u32 
*fw_cfg_param)
goto bye;
}
 
-   mempool_free(mbp, hw->mb_mempool);
if (finicsum != cfcsum) {
csio_warn(hw,
  "Config File checksum mismatch: csum=%#x, computed=%#x\n",
@@ -1780,6 +1779,10 @@ csio_hw_use_fwconfig(struct csio_hw *hw, int reset, u32 
*fw_cfg_param)
rv = csio_hw_validate_caps(hw, mbp);
if (rv != 0)
goto bye;
+
+   mempool_free(mbp, hw->mb_mempool);
+   mbp = NULL;
+
/*
 * Note that we're operating with parameters
 * not supplied by the driver, rather than from hard-wired
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index b44c3136eb51..520325867e2b 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -1422,7 +1422,7 @@ static void fc_rport_recv_rtv_req(struct fc_rport_priv 
*rdata,
fp = fc_frame_alloc(lport, sizeof(*rtv));
if (!fp) {
rjt_data.reason = ELS_RJT_UNAB;
-   rjt_data.reason 

[PATCH] scsi: lpfc: Avoid NULL pointer dereference in lpfc_els_abort()

[Guilherme G. Piccoli]

We might have a NULL pring in lpfc_els_abort(), for example on
error recovery path, since queues are destroyed during error
recovery mechanism.

In this case, we should just drop the abort since the queues will
be recreated anyway. This patch just verifies for NULL pointer
and stop the abortion of the queue in case of a NULL pring.

Also, this patch converts return type of lpfc_els_abort() from int
to void, since it's not checked anywhere.

Reported-by: Harsha Thyagaraja 
Reported-by: Naresh Bannoth 
Tested-by: Raphael Silva 
Signed-off-by: Guilherme G. Piccoli 
---
* This patch was rebased against Martin's 4.12/scsi-fixes.

 drivers/scsi/lpfc/lpfc_crtn.h  | 2 +-
 drivers/scsi/lpfc/lpfc_nportdisc.c | 7 +--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
index 8912767e7bc8..da669dce12fe 100644
--- a/drivers/scsi/lpfc/lpfc_crtn.h
+++ b/drivers/scsi/lpfc/lpfc_crtn.h
@@ -127,7 +127,7 @@ int lpfc_disc_state_machine(struct lpfc_vport *, struct 
lpfc_nodelist *, void *,
 void lpfc_do_scr_ns_plogi(struct lpfc_hba *, struct lpfc_vport *);
 int lpfc_check_sparm(struct lpfc_vport *, struct lpfc_nodelist *,
 struct serv_parm *, uint32_t, int);
-int lpfc_els_abort(struct lpfc_hba *, struct lpfc_nodelist *);
+void lpfc_els_abort(struct lpfc_hba *, struct lpfc_nodelist *);
 void lpfc_more_plogi(struct lpfc_vport *);
 void lpfc_more_adisc(struct lpfc_vport *);
 void lpfc_end_rscn(struct lpfc_vport *);
diff --git a/drivers/scsi/lpfc/lpfc_nportdisc.c 
b/drivers/scsi/lpfc/lpfc_nportdisc.c
index bff3de053df4..f74cb0142fd4 100644
--- a/drivers/scsi/lpfc/lpfc_nportdisc.c
+++ b/drivers/scsi/lpfc/lpfc_nportdisc.c
@@ -206,7 +206,7 @@ lpfc_check_elscmpl_iocb(struct lpfc_hba *phba, struct 
lpfc_iocbq *cmdiocb,
  * associated with a LPFC_NODELIST entry. This
  * routine effectively results in a "software abort".
  */
-int
+void
 lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
 {
LIST_HEAD(abort_list);
@@ -215,6 +215,10 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist 
*ndlp)
 
pring = lpfc_phba_elsring(phba);
 
+   /* In case of error recovery path, we might have a NULL pring here */
+   if (!pring)
+   return;
+
/* Abort outstanding I/O on NPort  */
lpfc_printf_vlog(ndlp->vport, KERN_INFO, LOG_DISCOVERY,
 "2819 Abort outstanding I/O on NPort x%x "
@@ -273,7 +277,6 @@ lpfc_els_abort(struct lpfc_hba *phba, struct lpfc_nodelist 
*ndlp)
  IOSTAT_LOCAL_REJECT, IOERR_SLI_ABORTED);
 
lpfc_cancel_retry_delay_tmo(phba->pport, ndlp);
-   return 0;
 }
 
 static int
-- 
2.12.0.rc0

Re: [PATCH] bnx2fc: fix race condition in bnx2fc_get_host_stats()

[Martin K. Petersen]

Maurizio,

> If multiple tasks attempt to read the stats, it may happen that the
> start_req_done completion is re-initialized while still being used by
> another task, causing a list corruption.
>
> This patch fixes the bug by adding a mutex to serialize the calls to
> bnx2fc_get_host_stats().

Applied to 4.12/scsi-fixes. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 00/15] qedf: Update driver to version 8.18.22.0.

[Chad Dupuis]

On Wed, 24 May 2017, 3:12pm, Martin K. Petersen wrote:

> 
> Chad,
> 
> > Please apply the following patches to the scsi tree at your earliest
> > convenience.
> 
> Please address Bart's comments and resubmit.
> 
> Thanks!
> 
> 

Sure, will submit a V2 that addresses Bart's comments.

Re: [PATCH 00/15] qedf: Update driver to version 8.18.22.0.

[Martin K. Petersen]

Chad,

> Please apply the following patches to the scsi tree at your earliest
> convenience.

Please address Bart's comments and resubmit.

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] qla2xxx: don't disable a not previously enabled PCI device

[Martin K. Petersen]

Johannes,

> When pci_enable_device() or pci_enable_device_mem() fail in
> qla2x00_probe_one() we bail out but do a call to
> pci_disable_device(). This causes the dev_WARN_ON() in
> pci_disable_device() to trigger, as the device wasn't enabled
> previously.
>
> So instead of taking the 'probe_out' error path we can directly return
> *iff* one of the pci_enable_device() calls fails.
>
> Additionally rename the 'probe_out' goto label's name to the more
> descriptive 'disable_device'.

Applied to 4.12/scsi-fixes. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] qla2xxx: don't disable a not previously enabled PCI device

[Malavali, Giridhar]

On 5/24/17, 8:47 AM, "Bart Van Assche"  wrote:

>On Tue, 2017-05-23 at 16:50 +0200, Johannes Thumshirn wrote:
>> When pci_enable_device() or pci_enable_device_mem() fail in
>> qla2x00_probe_one() we bail out but do a call to
>> pci_disable_device(). This causes the dev_WARN_ON() in
>> pci_disable_device() to trigger, as the device wasn't enabled
>> previously.
>> 
>> So instead of taking the 'probe_out' error path we can directly return
>> *iff* one of the pci_enable_device() calls fails.
>> 
>> Additionally rename the 'probe_out' goto label's name to the more
>> descriptive 'disable_device'.
>> 
>> Signed-off-by: Johannes Thumshirn 
>> Fixes: e315cd28b9ef ("[SCSI] qla2xxx: Code changes for qla data
>>structure refactoring")
>
>Hello Johannes,
>
>Please consider adding a Cc: stable tag to this patch. Since otherwise
>this
>patch looks fine to me:
>
>Reviewed-by: Bart Van Assche 

Looks good to me. 

Reviewed-by: Giridhar Malavali 

Re: [PATCH 15/15] qedf: Update version number to 8.18.22.0.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Signed-off-by: Chad Dupuis 
> ---
>  drivers/scsi/qedf/qedf_version.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/qedf/qedf_version.h 
> b/drivers/scsi/qedf/qedf_version.h
> index d46c487..6fa4420 100644
> --- a/drivers/scsi/qedf/qedf_version.h
> +++ b/drivers/scsi/qedf/qedf_version.h
> @@ -7,9 +7,9 @@
>   *  this source tree.
>   */
>  
> -#define QEDF_VERSION "8.10.7.0"
> +#define QEDF_VERSION "8.18.22.0"
>  #define QEDF_DRIVER_MAJOR_VER8
> -#define QEDF_DRIVER_MINOR_VER10
> -#define QEDF_DRIVER_REV_VER  7
> +#define QEDF_DRIVER_MINOR_VER18
> +#define QEDF_DRIVER_REV_VER  22
>  #define QEDF_DRIVER_ENG_VER  0
>  

Although I'm not sure having driver version information in an upstream
driver is useful:

Reviewed-by: Bart Van Assche 

Re: [PATCH 14/15] qedf: Add change_queue_depth member to scsi_host_template().

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Add the change_queue_depth member to our SCSI host template so the queue
> depth of devices attached to qedf can be changed dynamically.

Reviewed-by: Bart Van Assche 

Re: [PATCH 13/15] qedf: Change cmd_per_lun in scsi_host_template to 32 to increase performance.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Increase the default number of commands that the driver tells the
> SCSI mid-layer it can do to increase the default performance of the
> driver.

Reviewed-by: Bart Van Assche 

Re: [PATCH 12/15] qedf: Move some prints to a debug level so they do not print when no debugging is enabled.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
>   if (lport->port_id != ntoh24(fh->fh_d_id) && !vn_port) {
> - QEDF_ERR(&(qedf->dbg_ctx), "Dropping frame due to "
> - "destination mismatch: lport->port_id=%x "
> - "fh->d_id=%x.\n",
> + QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_LL2,
> + "Dropping frame due to destination mismatch: "
> + "lport->port_id=%x fh->d_id=%x.\n",
>   lport->port_id, ntoh24(fh->fh_d_id));
>   kfree_skb(skb);
>   return;

Hello Chad,

Please consider to keep the above error message on a single line.

Thanks,

Bart.

Re: [PATCH 11/15] qedf: Fixup unnecessary paratheses around test_bit operations.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Signed-off-by: Chad Dupuis 

Did you perhaps mean "parentheses" instead of "paratheses"? Please also
keep in mind that the recommended style for patch titles is the imperative
mood without trailing dot.

Anyway:

Reviewed-by: Bart Van Assche 

Re: [PATCH 10/15] qedf: Add non-offload receive filters.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> + if (ntoh24(_mac[3]) != ntoh24(fh->fh_d_id)) {
> + QEDF_ERR(&(qedf->dbg_ctx), "FC frame d_id mismatch with MAC "
> + "%pM.\n", dest_mac);
> [ ... ]
> + QEDF_ERR(&(qedf->dbg_ctx), "Wrong source address: "
> + "mac:%pM dest_addr:%pM.\n", mac,
> + qedf->ctlr.dest_addr);

Hello Chad,

Are you aware that the 80 column limit does not hold for error messages
and that the recommended style is to keep these on a single line?

Bart.

Re: [PATCH 09/15] qedf: Add bus_reset No-op.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> We need to add a bus reset no-op as without it some of the LUNs attached to a
> vport may go offline when the error handler escalates to host reset due to not
> having a bus reset handler in the driver. What happens is we escalate to host
> reset which does a soft link down/link up to reset the adapter. However with
> multiple vports attached it's been observed that if the vports do log back 
> into
> the target within 5 seconds, the SCSI layer offlines the devices most likely
> due to a TUR timing out to verify that the device is online. Adding a bus
> reset handler will cause the TUR to be sent after the bus reset handler where
> the devices will still be online if the bus reset is initiated by sg_reset
> (which is the case in the test that was failing). The bus reset will succeed
> and not needlessly bring the device offline/online.

Reviewed-by: Bart Van Assche 

Re: [PATCH 08/15] qedf: Use same logic for SCSI host reset and FC lip_reset.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> We should be using the same logic to do a soft reset of the FCoE function
> whether it is initiated via sg_reset or the fc_host issue_lip attribute.
> Refactor the host reset and fcoe reset handlers to use the preferred logic
> which is currently contained in qedf_eh_host_reset().

Reviewed-by: Bart Van Assche 

Re: [PATCH 07/15] qedf: Set qed logging level to QED_LEVEL_NOTICE.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Reduce the logging level we set for qed messages pertaining to this PCI
> function so that unnecessary messages are not printed in the kernel
> message log.

Reviewed-by: Bart Van Assche 

Re: [PATCH 06/15] qedf: Add fka_period SCSI host attribute to show fip keep alive period.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Expose this information for interested applications.
> 
> Signed-off-by: Chad Dupuis 
> ---
>  drivers/scsi/qedf/qedf_attr.c | 60 
> +--
>  1 file changed, 41 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/scsi/qedf/qedf_attr.c b/drivers/scsi/qedf/qedf_attr.c
> index 1349f8a..68e2b77 100644
> --- a/drivers/scsi/qedf/qedf_attr.c
> +++ b/drivers/scsi/qedf/qedf_attr.c
> @@ -8,6 +8,25 @@
>   */
>  #include "qedf.h"
>  
> +inline bool qedf_is_vport(struct qedf_ctx *qedf)
> +{
> + return (!(qedf->lport->vport == NULL));
> +}

Have you considered to write the return statement as follows?

return qedf->lport->vport != NULL;

Checkpatch should have recommended not to use parentheses in the return
statement.

> +
> +/* Get base qedf for physical port from vport */
> +static struct qedf_ctx *qedf_get_base_qedf(struct qedf_ctx *qedf)
> +{
> + struct fc_lport *lport;
> + struct fc_lport *base_lport;
> +
> + if (!(qedf_is_vport(qedf)))
> + return NULL;
> +
> + lport = qedf->lport;
> + base_lport = shost_priv(vport_to_shost(lport->vport));
> + return (struct qedf_ctx *)(lport_priv(base_lport));
> +}

lport_priv() returns a void pointer so the cast in the return statement is not
necessary.

> +static ssize_t
> +qedf_fka_period_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fc_lport *lport = shost_priv(class_to_shost(dev));
> + struct qedf_ctx *qedf = lport_priv(lport);
> + int fka_period = -1;
> +
> + if (qedf_is_vport(qedf))
> + qedf = qedf_get_base_qedf(qedf);
> +
> + if (!qedf->ctlr.sel_fcf)
> + goto out;
> +
> + fka_period = qedf->ctlr.sel_fcf->fka_period;
> +
> +out:
> + return scnprintf(buf, PAGE_SIZE, "%d\n", fka_period);
> +}

Do we really need a goto statement to skip a single statement? How about the
following:

if (qedf->ctlr.sel_fcf)
fka_period = qedf->ctlr.sel_fcf->fka_period;

return scnprintf(buf, PAGE_SIZE, "%d\n", fka_period);

or this:

return scnprintf(buf, PAGE_SIZE, "%d\n", qedf->ctlr.sel_fcf ?
 qedf->ctlr.sel_fcf->fka_period : -1);

Bart.

Re: [PATCH 05/15] qedf: Check that fcport is offloaded before dereferencing pointers in initiate_abts|cleanup.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> If an fcport is not offloaded then the members of the qedf_rport struct
> are undefined which may cause a system crash.

Reviewed-by: Bart Van Assche 

Re: [PATCH 04/15] qedf: Look at all descriptors when processing a clear virtual link.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> If there are multiple descriptors for a particular type in a clear virtual
> link we receive, we will not process it correctly but rather take the last
> value. This can cause us not to not flap the virtual link as the value from
> the descriptors that we compare against the our stored FCF or fc_lport values
> may not match.
> 
> Change is to do a comparison when processing the each descriptor instead of at
> the end and then set a bool if we need to do the reset.

Did you perhaps mean "Change this" instead of "Change is"? Anyway:

Reviewed-by: Bart Van Assche 

Re: [PATCH 03/15] qedf: Honor qed_ops->common->set_fp_int() return code.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> We need to check the return code the set_fp_int() callback in case we were
> not allocated any fastpath interrupts or there was an error setting up the
> fastpath interrupts from the qed perspective.

Reviewed-by: Bart Van Assche 

Re: [PATCH 01/15] qedf: Enable basic FDMI information.

[Bart Van Assche]

On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> + snprintf(fc_host_serial_number(lport->host),
> + FC_SERIAL_NUMBER_SIZE,
> + "%02X%02X%02X%02X%02X%02X%02X%02X",
> + buf[7], buf[6], buf[5], buf[4],
> + buf[3], buf[2], buf[1], buf[0]);
> + } else
> + snprintf(fc_host_serial_number(lport->host),
> + FC_SERIAL_NUMBER_SIZE, "Unknown");
> +
> + snprintf(fc_host_manufacturer(lport->host),
> + FC_SERIAL_NUMBER_SIZE, "%s", "Cavium Inc.");

Hello Chad,

I think this code would be a lot easier to read and to verify if it would be
modified as follows:
* Instead of using the fc_host_() macros, assign 
shost_to_fc_host(lport->host)
  to a variable and change fc_host_() into ...->.
* Instead of using the FC_*_SIZE macros, use sizeof(...->).

Thanks,

Bart.

Re: [PATCH] qla2xxx: don't disable a not previously enabled PCI device

[Bart Van Assche]

On Tue, 2017-05-23 at 16:50 +0200, Johannes Thumshirn wrote:
> When pci_enable_device() or pci_enable_device_mem() fail in
> qla2x00_probe_one() we bail out but do a call to
> pci_disable_device(). This causes the dev_WARN_ON() in
> pci_disable_device() to trigger, as the device wasn't enabled
> previously.
> 
> So instead of taking the 'probe_out' error path we can directly return
> *iff* one of the pci_enable_device() calls fails.
> 
> Additionally rename the 'probe_out' goto label's name to the more
> descriptive 'disable_device'.
> 
> Signed-off-by: Johannes Thumshirn 
> Fixes: e315cd28b9ef ("[SCSI] qla2xxx: Code changes for qla data structure 
> refactoring")

Hello Johannes,

Please consider adding a Cc: stable tag to this patch. Since otherwise this
patch looks fine to me:

Reviewed-by: Bart Van Assche 

Re: [PATCH 03/31] Protect SCSI device state changes with a mutex

[Bart Van Assche]

On Wed, 2017-05-24 at 07:51 +0200, Hannes Reinecke wrote:
> On 05/24/2017 02:33 AM, Bart Van Assche wrote:
> > 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_transport_srp.c 
> > b/drivers/scsi/scsi_transport_srp.c
> > index 3c5d89852e9f..f617021c94f7 100644
> > --- a/drivers/scsi/scsi_transport_srp.c
> > +++ b/drivers/scsi/scsi_transport_srp.c
> > @@ -554,11 +554,12 @@ int srp_reconnect_rport(struct srp_rport *rport)
> >  * invoking scsi_target_unblock() won't change the state of
> >  * these devices into running so do that explicitly.
> >  */
> > -   spin_lock_irq(shost->host_lock);
> > -   __shost_for_each_device(sdev, shost)
> > +   shost_for_each_device(sdev, shost) {
> > +   mutex_lock(>state_mutex);
> > if (sdev->sdev_state == SDEV_OFFLINE)
> > sdev->sdev_state = SDEV_RUNNING;
> > -   spin_unlock_irq(shost->host_lock);
> > +   mutex_unlock(>state_mutex);
> > +   }
> > } else if (rport->state == SRP_RPORT_RUNNING) {
> > /*
> >  * srp_reconnect_rport() has been invoked with fast_io_fail
> 
> Why do you drop the host lock here? I thought that the host lock is
> needed to protect shost_for_each_device()?

Hello Hannes,

The only purpose of holding the host lock was to protect the SCSI device list
iteration by __shost_for_each_device(). shost_for_each_device() obtains that
lock itself. From :

#define shost_for_each_device(sdev, shost) \
for ((sdev) = __scsi_iterate_devices((shost), NULL); \
 (sdev); \
 (sdev) = __scsi_iterate_devices((shost), (sdev)))

>From drivers/scsi/scsi.c:

struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost,
   struct scsi_device *prev)
{
struct list_head *list = (prev ? >siblings : >__devices);
struct scsi_device *next = NULL;
unsigned long flags;

spin_lock_irqsave(shost->host_lock, flags);
while (list->next != >__devices) {
next = list_entry(list->next, struct scsi_device, siblings);
/* skip devices that we can't get a reference to */
if (!scsi_device_get(next))
break;
next = NULL;
list = list->next;
}
spin_unlock_irqrestore(shost->host_lock, flags);

if (prev)
scsi_device_put(prev);
return next;
}

Bart.

Re: [PATCH 09/31] block: Avoid that blk_exit_rl() triggers a use-after-free

[Tejun Heo]

On Tue, May 23, 2017 at 05:33:58PM -0700, Bart Van Assche wrote:
> Since the introduction of the .init_rq_fn() and .exit_rq_fn() it
> is essential that the memory allocated for struct request_queue
> stays around until all blk_exit_rl() calls have finished. Hence
> make blk_init_rl() take a reference on struct request_queue.
> 
> This patch fixes the following crash:
> 
> general protection fault:  [#2] SMP
> CPU: 3 PID: 28 Comm: ksoftirqd/3 Tainted: G  D 4.12.0-rc2-dbg+ #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.0.0-prebuilt.qemu-project.org 04/01/2014
> task: 88013a108040 task.stack: c971c000
> RIP: 0010:free_request_size+0x1a/0x30
> RSP: 0018:c971fd38 EFLAGS: 00010202
> RAX: 6b6b6b6b6b6b6b6b RBX: 880067362a88 RCX: 0003
> RDX: 880067464178 RSI: 880067362a88 RDI: 880135ea4418
> RBP: c971fd40 R08:  R09: 000100180009
> R10: c971fd38 R11: 81110800 R12: 88006752d3d8
> R13: 88006752d3d8 R14: 88013a108040 R15: 000a
> FS:  () GS:88013fd8() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 7fa8ec1edb00 CR3: 000138ee8000 CR4: 001406e0
> Call Trace:
>  mempool_destroy.part.10+0x21/0x40
>  mempool_destroy+0xe/0x10
>  blk_exit_rl+0x12/0x20
>  blkg_free+0x4d/0xa0
>  __blkg_release_rcu+0x59/0x170
>  rcu_process_callbacks+0x260/0x4e0
>  __do_softirq+0x116/0x250
>  smpboot_thread_fn+0x123/0x1e0
>  kthread+0x109/0x140
>  ret_from_fork+0x31/0x40
> 
> Fixes: commit e9c787e65c0c ("scsi: allocate scsi_cmnd structures as part of 
> struct request")
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Tejun Heo 
> Cc: Jan Kara 
> Cc: Hannes Reinecke 
> Cc:  # v4.11+

Acked-by: Tejun Heo 

Thanks.

-- 
tejun

Re: [PATCH 11/31] block: Introduce queue flag QUEUE_FLAG_SCSI_SUP

[Bart Van Assche]

On Wed, 2017-05-24 at 08:01 +0200, Hannes Reinecke wrote:
> On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> > From the context where a SCSI command is submitted it is not always
> > possible to figure out whether or not the queue the command is
> > submitted to has struct scsi_request as the first member of its
> > private data. Hence introduce the flag QUEUE_FLAG_SCSI_SUP.
> > 
> > Signed-off-by: Bart Van Assche 
> > Cc: Jens Axboe 
> > Cc: Christoph Hellwig 
> > Cc: Omar Sandoval 
> > Cc: Hannes Reinecke 
> > ---
> >  block/bsg-lib.c   | 1 +
> >  drivers/block/cciss.c | 1 +
> >  drivers/ide/ide-probe.c   | 1 +
> >  drivers/scsi/scsi_lib.c   | 2 ++
> >  drivers/scsi/scsi_transport_sas.c | 1 +
> >  include/linux/blkdev.h| 2 ++
> >  6 files changed, 8 insertions(+)
> > 
> 
> Bit of an odd name; what about QUEUE_FLAG_SCSI_PDU?

Hello Hannes,

That sounds like a good idea to me. I will rename the flag when I repost this
series.

Bart.

[PATCH] bnx2fc: fix race condition in bnx2fc_get_host_stats()

[Maurizio Lombardi]

If multiple tasks attempt to read the stats, it may happen
that the start_req_done completion is re-initialized while
still being used by another task, causing a list corruption.

This patch fixes the bug by adding a mutex to serialize the
calls to bnx2fc_get_host_stats().

WARNING: at lib/list_debug.c:48 list_del+0x6e/0xa0() (Not tainted)
Hardware name: PowerEdge R820
list_del corruption. prev->next should be 882035627d90, but was 
884069541588

Pid: 40267, comm: perl Not tainted 2.6.32-642.3.1.el6.x86_64 #1
Call Trace:
 [] ? warn_slowpath_common+0x91/0xe0
 [] ? warn_slowpath_fmt+0x46/0x60
 [] ? list_del+0x6e/0xa0
 [] ? wait_for_common+0x14d/0x180
 [] ? default_wake_function+0x0/0x20
 [] ? wait_for_completion_timeout+0x13/0x20
 [] ? bnx2fc_get_host_stats+0xa1/0x280 [bnx2fc]
 [] ? fc_stat_show+0x90/0xc0 [scsi_transport_fc]
 [] ? show_fcstat_tx_frames+0x16/0x20 [scsi_transport_fc]
 [] ? dev_attr_show+0x27/0x50
 [] ? __get_free_pages+0xe/0x50
 [] ? sysfs_read_file+0x111/0x200
 [] ? vfs_read+0xb5/0x1a0
 [] ? fget_light_pos+0x16/0x50
 [] ? sys_read+0x51/0xb0
 [] ? __audit_syscall_exit+0x25e/0x290
 [] ? system_call_fastpath+0x16/0x1b

Signed-off-by: Maurizio Lombardi 
---
 drivers/scsi/bnx2fc/bnx2fc.h  |  1 +
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 10 --
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 4fc8ed5..1f424e4 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -191,6 +191,7 @@ struct bnx2fc_hba {
struct bnx2fc_cmd_mgr *cmd_mgr;
spinlock_t hba_lock;
struct mutex hba_mutex;
+   struct mutex hba_stats_mutex;
unsigned long adapter_state;
#define ADAPTER_STATE_UP0
#define ADAPTER_STATE_GOING_DOWN1
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 93b5a00..902722d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -663,15 +663,17 @@ static struct fc_host_statistics 
*bnx2fc_get_host_stats(struct Scsi_Host *shost)
if (!fw_stats)
return NULL;
 
+   mutex_lock(>hba_stats_mutex);
+
bnx2fc_stats = fc_get_host_stats(shost);
 
init_completion(>stat_req_done);
if (bnx2fc_send_stat_req(hba))
-   return bnx2fc_stats;
+   goto unlock_stats_mutex;
rc = wait_for_completion_timeout(>stat_req_done, (2 * HZ));
if (!rc) {
BNX2FC_HBA_DBG(lport, "FW stat req timed out\n");
-   return bnx2fc_stats;
+   goto unlock_stats_mutex;
}
BNX2FC_STATS(hba, rx_stat2, fc_crc_cnt);
bnx2fc_stats->invalid_crc_count += hba->bfw_stats.fc_crc_cnt;
@@ -693,6 +695,9 @@ static struct fc_host_statistics 
*bnx2fc_get_host_stats(struct Scsi_Host *shost)
 
memcpy(>prev_stats, hba->stats_buffer,
   sizeof(struct fcoe_statistics_params));
+
+unlock_stats_mutex:
+   mutex_unlock(>hba_stats_mutex);
return bnx2fc_stats;
 }
 
@@ -1340,6 +1345,7 @@ static struct bnx2fc_hba *bnx2fc_hba_create(struct 
cnic_dev *cnic)
}
spin_lock_init(>hba_lock);
mutex_init(>hba_mutex);
+   mutex_init(>hba_stats_mutex);
 
hba->cnic = cnic;
 
-- 
Maurizio Lombardi

Re: [PATCH 08/31] sd, sr: Convert two assignments into warning statements

[Johannes Thumshirn]

On 05/24/2017 02:33 AM, Bart Van Assche wrote:
> Before scsi_prep_fn() calls the ULP .init_command() callback
> function it stores the SCSI command pointer in request.special.
> This means that the SCpnt = rq->special assignments in the sd
> and sr drivers assign a pointer to itself. Hence convert these
> two assignment statements into warning statements.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> ---

Looks good,
Reviewed-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 07/31] scsi: Use blk_mq_rq_to_pdu() to convert a request to a SCSI command pointer

[Johannes Thumshirn]

On 05/24/2017 02:33 AM, Bart Van Assche wrote:
> Since commit e9c787e65c0c ("scsi: allocate scsi_cmnd structures as
> part of struct request") struct request and struct scsi_cmnd are
> adjacent. This means that there is now an alternative to reading
> req->special to convert a pointer to a prepared request into a
> SCSI command pointer, namely by using blk_mq_rq_to_pdu(). Make
> this change where appropriate. Although this patch does not
> change any functionality, it slightly improves performance and
> slightly improves readability.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> ---

Looks good,
Reviewed-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 02/31] Create two versions of scsi_internal_device_unblock()

[Johannes Thumshirn]

On 05/24/2017 02:33 AM, Bart Van Assche wrote:
> This will make it easier to serialize SCSI device state changes
> through a mutex.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: Sreekanth Reddy 
> ---

Looks good,
Reviewed-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 01/31] Split scsi_internal_device_block()

[Johannes Thumshirn]

On 05/24/2017 02:33 AM, Bart Van Assche wrote:
> 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 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: Sreekanth Reddy 
> ---

Looks good,
Reviewed-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 31/31] xen/scsifront: Remove code that zeroes driver-private command data

[Juergen Gross]

On 24/05/17 02:34, Bart Van Assche wrote:
> Since the SCSI core zeroes driver-private command data, remove
> that code from the xen-scsifront driver.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Juergen Gross 
> Cc: xen-de...@lists.xenproject.org

Reviewed-by: Juergen Gross 

Thanks,

Juergen

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

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since the SCSI core zeroes driver-private command data, remove
> that code from the xen-scsifront driver.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Juergen Gross 
> Cc: xen-de...@lists.xenproject.org
> ---
>  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;
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 30/31] scsi: virtio: Remove code that zeroes driver-private command data

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since the SCSI core zeroes driver-private command data, remove
> that code from the virtio driver.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Michael S. Tsirkin 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  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);
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 29/31] scsi: snic: Remove code that zeroes driver-private command data

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since the SCSI core zeroes driver-private command data, remove
> that code from the snic driver.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Narsimhulu Musini 
> Cc: Sesidhar Baddela 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  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);
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 27/31] scsi: Consolidate more initialization code

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Initialize struct scsi_cmnd.request from inside scsi_initialize_rq()
> instead of every time a request is prepared. Note: moving the tag
> initialization into scsi_initialize_rq() is not possible because
> the single-queue block layer only assigns a tag to a request after
> a request has been started.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_error.c | 1 -
>  drivers/scsi/scsi_lib.c   | 4 +---
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 28/31] scsi_setup_fs_cmnd(): Call scsi_req_init() instead of open-coding it

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> The only functional change is that this patch causes scsi_setup_fs_cmnd()
> to clear scsi_request.sense_len.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_lib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 25/31] scsi-mq: Make behavior scsi_mq_prep_fn() closer to that of scsi_prep_fn()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Instead of clearing most of struct scsi_cmnd and reinitializing
> it, rely on scsi_initialize_rq() for initialization of struct
> scsi_cmnd. This patch fixes a bug, namely that it avoids that
> jiffies_at_alloc gets overwritten if a request is requeued.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_lib.c | 12 +---
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 4b24c45fa113..12fd2bb0fe9c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1861,27 +1861,17 @@ 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;
>   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) + shost->hostt->cmd_size);
> + memset(scsi_cmd_priv(cmd), 0, shost->hostt->cmd_size);
>  
>   req->special = cmd;
>  
>   cmd->request = req;
> - cmd->device = sdev;
> - cmd->sense_buffer = sense_buf;
>  
>   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;
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 26/31] scsi: Move the code for clearing private command data into scsi_dispatch_cmd()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> This patch does not change any functionality but avoids duplication
> of the code for clearing driver-private command data.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_lib.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 23/31] scsi: Move sense buffer pointer initialization into scsi_initialize_rq()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> This patch is a preparation for the next patch that will zero
> the struct scsi_request embedded in struct scsi_cmnd before
> calling scsi_req_init().
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_lib.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 24/31] scsi: Make scsi_initialize_rq() zero the entire struct scsi_cmnd

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> This simplifies the memset() call in scsi_initialize_rq() and avoids
> that any stale data is left behind in struct scsi_request.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_lib.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 10c6adb208dc..4b24c45fa113 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1159,12 +1159,10 @@ static void scsi_initialize_rq(struct request *rq)
>   void *buf = cmd->sense_buffer;
>   void *prot = cmd->prot_sdb;
>  
> - /* zero out the cmd, except for the embedded scsi_request */
> - memset((char *)cmd + sizeof(cmd->req), 0,
> -sizeof(*cmd) - sizeof(cmd->req));
> + memset(cmd, 0, sizeof(*cmd));
>   scsi_req_init(>req);
>   cmd->device = dev;
> - cmd->req.sense = cmd->sense_buffer;
> + cmd->req.sense = buf;
>   cmd->sense_buffer = buf;
>   cmd->prot_sdb = prot;
>   INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler);
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 22/31] scsi: Inline scsi_init_command()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> The two drivers that use the per-device command list, namely aacraid
> and dpt_i2o, expect that that list contains only SCSI commands and
> no task management functions. Hence only call scsi_add_cmd_to_list()
> from the block layer prep callback functions and not from
> scsi_ioctl_reset().
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_error.c |  1 -
>  drivers/scsi/scsi_lib.c   | 10 ++
>  drivers/scsi/scsi_priv.h  |  1 -
>  3 files changed, 2 insertions(+), 10 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 21/31] scsi: Move most of scsi_init_command() into scsi_initialize_rq()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Move the initializations that only have to be performed once and
> not every time a request is prepared from scsi_init_command()
> into scsi_initialize_rq(). This patch also moves the
> jiffies_at_alloc assignment such that it gets back the meaning it
> had before commit e9c787e65c0c, namely the value of the jiffies
> counter at request allocation time.
> 
> Fixes: commit e9c787e65c0c ("scsi: allocate scsi_cmnd structures as part of 
> struct request")
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_lib.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 20/31] scsi: Only add commands to the device command list if required by the LLD

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> 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 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi.c  |  9 +
>  drivers/scsi/scsi_lib.c  | 52 
> +---
>  drivers/scsi/scsi_priv.h |  2 ++
>  3 files changed, 35 insertions(+), 28 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 19/31] scsi: Change argument type of scsi_req_init()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since scsi_req_init() works on a struct scsi_request, change the
> argument type into struct scsi_request *.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  block/scsi_ioctl.c| 10 +++---
>  drivers/ide/ide-probe.c   |  2 +-
>  drivers/scsi/scsi_lib.c   |  4 +++-
>  drivers/scsi/scsi_transport_sas.c |  2 +-
>  include/scsi/scsi_request.h   |  2 +-
>  5 files changed, 13 insertions(+), 7 deletions(-)
> 
Ah, right. Now it's clear.

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 18/31] block: Make scsi_req_init() calls implicit

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Instead of explicitly calling scsi_req_init(), let
> blk_get_request() call that function from inside blk_rq_init().
> Add an .initialize_rq_fn() callback function to the block drivers
> that need it. Merge the IDE .init_rq_fn() function into
> .initialize_rq_fn() because it is too small to keep it as a
> separate function.
> 
> References: commit 82ed4db499b8 ("block: split scsi_request out of struct 
> request")
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> Cc: linux-bl...@vger.kernel.org
> ---
>  block/bsg.c|  1 -
>  block/scsi_ioctl.c |  3 ---
>  drivers/block/pktcdvd.c|  1 -
>  drivers/cdrom/cdrom.c  |  1 -
>  drivers/ide/ide-atapi.c|  2 --
>  drivers/ide/ide-cd.c   |  1 -
>  drivers/ide/ide-cd_ioctl.c |  1 -
>  drivers/ide/ide-devsets.c  |  1 -
>  drivers/ide/ide-disk.c |  1 -
>  drivers/ide/ide-ioctls.c   |  2 --
>  drivers/ide/ide-park.c |  2 --
>  drivers/ide/ide-pm.c   |  2 --
>  drivers/ide/ide-probe.c|  6 +++---
>  drivers/ide/ide-tape.c |  1 -
>  drivers/ide/ide-taskfile.c |  1 -
>  drivers/scsi/osd/osd_initiator.c   |  2 --
>  drivers/scsi/osst.c|  1 -
>  drivers/scsi/scsi_error.c  |  1 -
>  drivers/scsi/scsi_lib.c| 10 +-
>  drivers/scsi/scsi_transport_sas.c  |  6 ++
>  drivers/scsi/sg.c  |  2 --
>  drivers/scsi/st.c  |  1 -
>  drivers/target/target_core_pscsi.c |  2 --
>  fs/nfsd/blocklayout.c  |  1 -
>  24 files changed, 18 insertions(+), 34 deletions(-)
> 
> diff --git a/block/bsg.c b/block/bsg.c
> index f7695bb141d9..3ca080be4c70 100644
> --- a/block/bsg.c
> +++ b/block/bsg.c
> @@ -236,7 +236,6 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, 
> fmode_t has_write_perm)
>   rq = blk_get_request(q, op, GFP_KERNEL);
>   if (IS_ERR(rq))
>   return rq;
> - scsi_req_init(rq);
>  
>   ret = blk_fill_sgv4_hdr_rq(q, rq, hdr, bd, has_write_perm);
>   if (ret)
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 4a294a5f7fab..f96c51f5df40 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -326,7 +326,6 @@ static int sg_io(struct request_queue *q, struct gendisk 
> *bd_disk,
>   if (IS_ERR(rq))
>   return PTR_ERR(rq);
>   req = scsi_req(rq);
> - scsi_req_init(rq);
>  
>   if (hdr->cmd_len > BLK_MAX_CDB) {
>   req->cmd = kzalloc(hdr->cmd_len, GFP_KERNEL);
> @@ -456,7 +455,6 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk 
> *disk, fmode_t mode,
>   goto error_free_buffer;
>   }
>   req = scsi_req(rq);
> - scsi_req_init(rq);
>  
>   cmdlen = COMMAND_SIZE(opcode);
>  
> @@ -542,7 +540,6 @@ static int __blk_send_generic(struct request_queue *q, 
> struct gendisk *bd_disk,
>   rq = blk_get_request(q, REQ_OP_SCSI_OUT, __GFP_RECLAIM);
>   if (IS_ERR(rq))
>   return PTR_ERR(rq);
> - scsi_req_init(rq);
>   rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
>   scsi_req(rq)->cmd[0] = cmd;
>   scsi_req(rq)->cmd[4] = data;
> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index b8ce55d7911d..08e3e2fb649b 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -707,7 +707,6 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, 
> struct packet_command *
>REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, __GFP_RECLAIM);
>   if (IS_ERR(rq))
>   return PTR_ERR(rq);
> - scsi_req_init(rq);
>  
>   if (cgc->buflen) {
>   ret = blk_rq_map_kern(q, rq, cgc->buffer, cgc->buflen,
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 070568d496dc..e643c9d7beec 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -2199,7 +2199,6 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info 
> *cdi, __u8 __user *ubuf,
>   break;
>   }
>   req = scsi_req(rq);
> - scsi_req_init(rq);
>  
>   ret = blk_rq_map_user(q, rq, NULL, ubuf, len, GFP_KERNEL);
>   if (ret) {
> diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
> index 5901937284e7..7edebe0fb1eb 100644
> --- a/drivers/ide/ide-atapi.c
> +++ b/drivers/ide/ide-atapi.c
> @@ -93,7 +93,6 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk 
> *disk,
>   int error;
>  
>   rq = blk_get_request(drive->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
> - scsi_req_init(rq);
>   ide_req(rq)->type = ATA_PRIV_MISC;
>   rq->special = (char *)pc;
>  
> @@ -200,7 +199,6 @@ void ide_prep_sense(ide_drive_t *drive, struct 

Re: [PATCH 17/31] block: Introduce request_queue.initialize_rq_fn()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Several block drivers need to initialize the driver-private data
> after having called blk_get_request() and before .prep_rq_fn() is
> called, e.g. when submitting a REQ_OP_SCSI_* request. Avoid that
> that initialization code has to be repeated after every
> blk_get_request() call by adding a new callback function to struct
> request_queue.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> Cc: linux-bl...@vger.kernel.org
> ---
>  block/blk-core.c   | 3 +++
>  block/blk-mq.c | 3 +++
>  include/linux/blkdev.h | 4 
>  3 files changed, 10 insertions(+)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 16/31] scsi: Make scsi_ioctl_reset() pass the request queue pointer to blk_rq_init()

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> A later patch will add a call to a request initialization function
> into blk_rq_init(). Hence make sure that all blk_rq_init() calls
> specify the request queue pointer. Since TMF callback functions in
> SCSI LLD drivers do not use request.q, this patch does not change
> the behavior of any SCSI driver.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> ---
>  drivers/scsi/scsi_error.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 14/31] cdrom: Check private request size before attaching to a queue

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since the cdrom driver only supports request queues for which
> struct scsi_request is the first member of their private request
> data, refuse to register block layer queues for which this is
> not the case.
> 
> References: commit 82ed4db499b8 ("block: split scsi_request out of struct 
> request")
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> Cc: linux-bl...@vger.kernel.org
> ---
>  drivers/cdrom/cdrom.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 76c952fd9ab9..070568d496dc 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -594,6 +594,10 @@ int register_cdrom(struct cdrom_device_info *cdi)
>  
>   if (cdo->open == NULL || cdo->release == NULL)
>   return -EINVAL;
> + if (!blk_queue_scsi_sup(cdi->disk->queue)) {
> + WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
> + return -EINVAL;
> + }
>   if (!banner_printed) {
>   pr_info("Uniform CD-ROM driver " REVISION "\n");
>   banner_printed = 1;
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 13/31] pktcdvd: Check queue type before attaching to a queue

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since the pktcdvd driver only supports request queues for which
> struct scsi_request is the first member of their private request
> data, refuse to register block layer queues for which struct
> scsi_request is not the first member of the private data.
> 
> References: commit 82ed4db499b8 ("block: split scsi_request out of struct 
> request")
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> Cc: linux-bl...@vger.kernel.org
> ---
>  drivers/block/pktcdvd.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index 205b865ebeb9..b8ce55d7911d 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -2583,6 +2583,11 @@ static int pkt_new_dev(struct pktcdvd_device *pd, 
> dev_t dev)
>   bdev = bdget(dev);
>   if (!bdev)
>   return -ENOMEM;
> + if (!blk_queue_scsi_sup(bdev_get_queue(bdev))) {
> + WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
> + bdput(bdev);
> + return -EINVAL;
> + }
>   ret = blkdev_get(bdev, FMODE_READ | FMODE_NDELAY, NULL);
>   if (ret)
>   return ret;
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 15/31] nfsd: Check private request size before submitting a SCSI request

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since using scsi_req() is only allowed against request queues for
> which struct scsi_request is the first member of their private
> request data, refuse to submit SCSI commands against a queue for
> which this is not the case.
> 
> References: commit 82ed4db499b8 ("block: split scsi_request out of struct 
> request")
> Signed-off-by: Bart Van Assche 
> Cc: J. Bruce Fields 
> Cc: Jeff Layton 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> Cc: linux-...@vger.kernel.org
> Cc: linux-bl...@vger.kernel.org
> ---
>  fs/nfsd/blocklayout.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index fb5213afc854..9ca0ca5efbc8 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -219,6 +219,9 @@ static int nfsd4_scsi_identify_device(struct block_device 
> *bdev,
>   u8 *buf, *d, type, assoc;
>   int error;
>  
> + if (WARN_ON_ONCE(!blk_queue_scsi_sup(q)))
> + return -EINVAL;
> +
>   buf = kzalloc(bufflen, GFP_KERNEL);
>   if (!buf)
>   return -ENOMEM;
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 12/31] bsg: Check queue type before attaching to a queue

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> Since BSG only supports request queues for which struct scsi_request
> is the first member of their private request data, refuse to register
> block layer queues for which struct scsi_request is not the first
> member of their private data.
> 
> References: commit bd1599d931ca ("scsi_transport_sas: fix BSG ioctl memory 
> corruption")
> References: commit 82ed4db499b8 ("block: split scsi_request out of struct 
> request")
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> Cc: linux-bl...@vger.kernel.org
> ---
>  block/bsg.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/bsg.c b/block/bsg.c
> index 6fd08544d77e..f7695bb141d9 100644
> --- a/block/bsg.c
> +++ b/block/bsg.c
> @@ -750,6 +750,12 @@ static struct bsg_device *bsg_add_device(struct inode 
> *inode,
>  #ifdef BSG_DEBUG
>   unsigned char buf[32];
>  #endif
> +
> + if (!blk_queue_scsi_sup(rq)) {
> + WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
>   if (!blk_get_queue(rq))
>   return ERR_PTR(-ENXIO);
>  
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 11/31] block: Introduce queue flag QUEUE_FLAG_SCSI_SUP

[Hannes Reinecke]

On 05/24/2017 02:34 AM, Bart Van Assche wrote:
> From the context where a SCSI command is submitted it is not always
> possible to figure out whether or not the queue the command is
> submitted to has struct scsi_request as the first member of its
> private data. Hence introduce the flag QUEUE_FLAG_SCSI_SUP.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Jens Axboe 
> Cc: Christoph Hellwig 
> Cc: Omar Sandoval 
> Cc: Hannes Reinecke 
> ---
>  block/bsg-lib.c   | 1 +
>  drivers/block/cciss.c | 1 +
>  drivers/ide/ide-probe.c   | 1 +
>  drivers/scsi/scsi_lib.c   | 2 ++
>  drivers/scsi/scsi_transport_sas.c | 1 +
>  include/linux/blkdev.h| 2 ++
>  6 files changed, 8 insertions(+)
> 
Bit of an odd name; what about QUEUE_FLAG_SCSI_PDU?

Otherwise:
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)