Re: [PATCH 00/31] SCSI patches for kernel v4.13.
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
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.
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.
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
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, &vha->dpc_flags);
set_bit(REGISTER_FDMI_NEEDED, &vha->dpc_flags);
} else {
--
2.12.0
[PATCH v2 1/7] qla2xxx: Fix recursive loop during target mode configuration for ISP25XX leaving system unresponsive.
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.
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
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(&ha->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(&ha->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(&ha->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(&ha->mq_lock);
ha->queue_pair_map[qpair->id] = NULL;
clear_bit(qpair->id, ha->qpair_qid_map);
+ ha->num_qpairs--;
list_del(&qpair->qp_list_elem);
if (list_empty(&vha->qp_list))
vha->flags.qpairs_available = 0;
--
2.12.0
[PATCH v2 6/7] qla2xxx: Fix mailbox pointer error in fwdump capture.
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.
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,
- &ctx->dsd_list, list) {
- dma_pool_free(ha->dl_dma_pool, dsd_ptr->dsd_addr,
- dsd_ptr->dsd_list_dma);
- list_del(&dsd_ptr->list);
- kfree(dsd_ptr);
+ list_for_each_entry_safe(dsd, tdsd, &ctx->dsd_list, list) {
+ dma_pool_free(ha->dl_dma_pool, dsd->dsd_addr,
+ dsd->dsd_list_dma);
+ list_del(&dsd->list);
+ kfree(dsd);
}
INIT_LIST_HEAD(&ctx->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(&ctx1->dsd_list, &ha->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(&ctx1->dsd_list, &ha->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 a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index 0e0
[PATCH v2 5/7] qla2xxx: Set bit 15 for DIAG_ECHO_TEST MBC.
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(&elreq, 0, sizeof(elreq));
+
elreq.req_sg_cnt = dma_map_sg(&ha->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(&vha->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
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()
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()
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.
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.
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
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
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.
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().
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.
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.
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.
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.
On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> + if (ntoh24(&dest_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.
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.
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.
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.
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.
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.
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.
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.
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
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
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(&sdev->state_mutex);
> > if (sdev->sdev_state == SDEV_OFFLINE)
> > sdev->sdev_state = SDEV_RUNNING;
> > - spin_unlock_irq(shost->host_lock);
> > + mutex_unlock(&sdev->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 ? &prev->siblings : &shost->__devices);
struct scsi_device *next = NULL;
unsigned long flags;
spin_lock_irqsave(shost->host_lock, flags);
while (list->next != &shost->__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
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
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.
Re: [PATCH] bnx2fc: fix race condition in bnx2fc_get_host_stats()
On Wed, 24 May 2017, 8:09am, Maurizio Lombardi wrote: > 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(-) > Thanks Maurizio. Acked-by: Chad Dupuis
[PATCH] bnx2fc: fix race condition in bnx2fc_get_host_stats()
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->hba_stats_mutex);
+
bnx2fc_stats = fc_get_host_stats(shost);
init_completion(&hba->stat_req_done);
if (bnx2fc_send_stat_req(hba))
- return bnx2fc_stats;
+ goto unlock_stats_mutex;
rc = wait_for_completion_timeout(&hba->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(&hba->prev_stats, hba->stats_buffer,
sizeof(struct fcoe_statistics_params));
+
+unlock_stats_mutex:
+ mutex_unlock(&hba->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->hba_lock);
mutex_init(&hba->hba_mutex);
+ mutex_init(&hba->hba_stats_mutex);
hba->cnic = cnic;
--
Maurizio Lombardi
Re: [PATCH 08/31] sd, sr: Convert two assignments into warning statements
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
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()
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()
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
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
