* [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers
@ 2019-08-14 13:28 Martin Wilck
2019-08-14 13:28 ` [PATCH v2 1/2] scsi: qla2xxx: qla2x00_alloc_fw_dump: set ha->eft Martin Wilck
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Martin Wilck @ 2019-08-14 13:28 UTC (permalink / raw)
To: Martin K. Petersen, Himanshu Madhani
Cc: Bart Van Assche, Joe Carnuccio, Quinn Tran, Hannes Reinecke,
Martin Wilck, linux-scsi
From: Martin Wilck <mwilck@suse.com>
Hi Himanshu, hi Martin,
Please consider this series for merging.
The first patch of the series is a fix for a memory corruption we
saw in a test where qla2xxx was loaded/unloaded repeatedly under
memory pressure. The second one is a cleanup/consistency fix.
Regards,
Martin
Changes in v2: rather then keeping the patches small, cleanup the
buffer allocation code properly, following (and going beyond) Hannes'
review suggestions for the v1 set.
Martin Wilck (2):
scsi: qla2xxx: qla2x00_alloc_fw_dump: set ha->eft
scsi: qla2xxx: cleanup trace buffer initialization
drivers/scsi/qla2xxx/qla_init.c | 218 +++++++++++++++-----------------
drivers/scsi/qla2xxx/qla_os.c | 1 +
2 files changed, 102 insertions(+), 117 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] scsi: qla2xxx: qla2x00_alloc_fw_dump: set ha->eft
2019-08-14 13:28 [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin Wilck
@ 2019-08-14 13:28 ` Martin Wilck
2019-08-14 13:28 ` [PATCH v2 2/2] scsi: qla2xxx: cleanup trace buffer initialization Martin Wilck
2019-08-15 2:08 ` [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin Wilck @ 2019-08-14 13:28 UTC (permalink / raw)
To: Martin K. Petersen, Himanshu Madhani
Cc: Bart Van Assche, Joe Carnuccio, Quinn Tran, Hannes Reinecke,
Martin Wilck, linux-scsi, Bart Van Assche
From: Martin Wilck <mwilck@suse.com>
In qla2x00_alloc_fw_dump(), an existing EFT buffer (e.g. from
previous invocation of qla2x00_alloc_offload_mem()) is freed.
The buffer is then re-allocated, but without setting the eft and
eft_dma fields to the new values.
Fixes: a28d9e4ef997 "scsi: qla2xxx: Add support for multiple fwdump
templates/segments"
Cc: Joe Carnuccio <joe.carnuccio@cavium.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
drivers/scsi/qla2xxx/qla_init.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 535dc21..6dd68be 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3197,6 +3197,8 @@ qla2x00_alloc_fw_dump(scsi_qla_host_t *vha)
ql_dbg(ql_dbg_init, vha, 0x00c3,
"Allocated (%d KB) EFT ...\n", EFT_SIZE / 1024);
eft_size = EFT_SIZE;
+ ha->eft_dma = tc_dma;
+ ha->eft = tc;
}
if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
--
2.22.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] scsi: qla2xxx: cleanup trace buffer initialization
2019-08-14 13:28 [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin Wilck
2019-08-14 13:28 ` [PATCH v2 1/2] scsi: qla2xxx: qla2x00_alloc_fw_dump: set ha->eft Martin Wilck
@ 2019-08-14 13:28 ` Martin Wilck
2019-08-15 2:08 ` [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin Wilck @ 2019-08-14 13:28 UTC (permalink / raw)
To: Martin K. Petersen, Himanshu Madhani
Cc: Bart Van Assche, Joe Carnuccio, Quinn Tran, Hannes Reinecke,
Martin Wilck, linux-scsi, Bart Van Assche
From: Martin Wilck <mwilck@suse.com>
Avoid code duplication between qla2x00_alloc_offload_mem() and
qla2x00_alloc_fw_dump() by moving the FCE and EFT buffer allocation
and initialization to separate functions. Cleanly track failure
and success by making sure that the ha->eft, ha->fce and respective
eft_dma, fce_dma members are set if and only if the buffers are properly
allocated and initialized. Avoid pointless buffer reallocation.
Eliminate some goto statements. Make sure the fce_enabled flag
is cleared when the FCE buffer is freed.
Fixes: ad0a0b01f088 "scsi: qla2xxx: Fix Firmware dump size for Extended
login and Exchange Offload"
Fixes: a28d9e4ef997 "scsi: qla2xxx: Add support for multiple fwdump
templates/segments"
Cc: Joe Carnuccio <joe.carnuccio@cavium.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <hmadhani@marvell.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
drivers/scsi/qla2xxx/qla_init.c | 220 +++++++++++++++-----------------
drivers/scsi/qla2xxx/qla_os.c | 1 +
2 files changed, 102 insertions(+), 119 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 6dd68be..4a89ec5 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3032,103 +3032,113 @@ qla24xx_chip_diag(scsi_qla_host_t *vha)
}
static void
-qla2x00_alloc_offload_mem(scsi_qla_host_t *vha)
+qla2x00_init_fce_trace(scsi_qla_host_t *vha)
{
int rval;
dma_addr_t tc_dma;
void *tc;
struct qla_hw_data *ha = vha->hw;
+ if (!IS_FWI2_CAPABLE(ha))
+ return;
+
+ if (!IS_QLA25XX(ha) && !IS_QLA81XX(ha) && !IS_QLA83XX(ha) &&
+ !IS_QLA27XX(ha) && !IS_QLA28XX(ha))
+ return;
+
+ if (ha->fce) {
+ ql_dbg(ql_dbg_init, vha, 0x00bd,
+ "%s: FCE Mem is already allocated.\n",
+ __func__);
+ return;
+ }
+
+ /* Allocate memory for Fibre Channel Event Buffer. */
+ tc = dma_alloc_coherent(&ha->pdev->dev, FCE_SIZE, &tc_dma,
+ GFP_KERNEL);
+ if (!tc) {
+ ql_log(ql_log_warn, vha, 0x00be,
+ "Unable to allocate (%d KB) for FCE.\n",
+ FCE_SIZE / 1024);
+ return;
+ }
+
+ rval = qla2x00_enable_fce_trace(vha, tc_dma, FCE_NUM_BUFFERS,
+ ha->fce_mb, &ha->fce_bufs);
+ if (rval) {
+ ql_log(ql_log_warn, vha, 0x00bf,
+ "Unable to initialize FCE (%d).\n", rval);
+ dma_free_coherent(&ha->pdev->dev, FCE_SIZE, tc, tc_dma);
+ return;
+ }
+
+ ql_dbg(ql_dbg_init, vha, 0x00c0,
+ "Allocated (%d KB) for FCE...\n", FCE_SIZE / 1024);
+
+ ha->flags.fce_enabled = 1;
+ ha->fce_dma = tc_dma;
+ ha->fce = tc;
+}
+
+static void
+qla2x00_init_eft_trace(scsi_qla_host_t *vha)
+{
+ int rval;
+ dma_addr_t tc_dma;
+ void *tc;
+ struct qla_hw_data *ha = vha->hw;
+
+ if (!IS_FWI2_CAPABLE(ha))
+ return;
+
if (ha->eft) {
ql_dbg(ql_dbg_init, vha, 0x00bd,
- "%s: Offload Mem is already allocated.\n",
+ "%s: EFT Mem is already allocated.\n",
__func__);
return;
}
- if (IS_FWI2_CAPABLE(ha)) {
- /* Allocate memory for Fibre Channel Event Buffer. */
- if (!IS_QLA25XX(ha) && !IS_QLA81XX(ha) && !IS_QLA83XX(ha) &&
- !IS_QLA27XX(ha) && !IS_QLA28XX(ha))
- goto try_eft;
-
- if (ha->fce)
- dma_free_coherent(&ha->pdev->dev,
- FCE_SIZE, ha->fce, ha->fce_dma);
-
- /* Allocate memory for Fibre Channel Event Buffer. */
- tc = dma_alloc_coherent(&ha->pdev->dev, FCE_SIZE, &tc_dma,
- GFP_KERNEL);
- if (!tc) {
- ql_log(ql_log_warn, vha, 0x00be,
- "Unable to allocate (%d KB) for FCE.\n",
- FCE_SIZE / 1024);
- goto try_eft;
- }
-
- rval = qla2x00_enable_fce_trace(vha, tc_dma, FCE_NUM_BUFFERS,
- ha->fce_mb, &ha->fce_bufs);
- if (rval) {
- ql_log(ql_log_warn, vha, 0x00bf,
- "Unable to initialize FCE (%d).\n", rval);
- dma_free_coherent(&ha->pdev->dev, FCE_SIZE, tc,
- tc_dma);
- ha->flags.fce_enabled = 0;
- goto try_eft;
- }
- ql_dbg(ql_dbg_init, vha, 0x00c0,
- "Allocate (%d KB) for FCE...\n", FCE_SIZE / 1024);
-
- ha->flags.fce_enabled = 1;
- ha->fce_dma = tc_dma;
- ha->fce = tc;
-
-try_eft:
- if (ha->eft)
- dma_free_coherent(&ha->pdev->dev,
- EFT_SIZE, ha->eft, ha->eft_dma);
-
- /* Allocate memory for Extended Trace Buffer. */
- tc = dma_alloc_coherent(&ha->pdev->dev, EFT_SIZE, &tc_dma,
- GFP_KERNEL);
- if (!tc) {
- ql_log(ql_log_warn, vha, 0x00c1,
- "Unable to allocate (%d KB) for EFT.\n",
- EFT_SIZE / 1024);
- goto eft_err;
- }
-
- rval = qla2x00_enable_eft_trace(vha, tc_dma, EFT_NUM_BUFFERS);
- if (rval) {
- ql_log(ql_log_warn, vha, 0x00c2,
- "Unable to initialize EFT (%d).\n", rval);
- dma_free_coherent(&ha->pdev->dev, EFT_SIZE, tc,
- tc_dma);
- goto eft_err;
- }
- ql_dbg(ql_dbg_init, vha, 0x00c3,
- "Allocated (%d KB) EFT ...\n", EFT_SIZE / 1024);
-
- ha->eft_dma = tc_dma;
- ha->eft = tc;
+ /* Allocate memory for Extended Trace Buffer. */
+ tc = dma_alloc_coherent(&ha->pdev->dev, EFT_SIZE, &tc_dma,
+ GFP_KERNEL);
+ if (!tc) {
+ ql_log(ql_log_warn, vha, 0x00c1,
+ "Unable to allocate (%d KB) for EFT.\n",
+ EFT_SIZE / 1024);
+ return;
}
-eft_err:
- return;
+ rval = qla2x00_enable_eft_trace(vha, tc_dma, EFT_NUM_BUFFERS);
+ if (rval) {
+ ql_log(ql_log_warn, vha, 0x00c2,
+ "Unable to initialize EFT (%d).\n", rval);
+ dma_free_coherent(&ha->pdev->dev, EFT_SIZE, tc, tc_dma);
+ return;
+ }
+
+ ql_dbg(ql_dbg_init, vha, 0x00c3,
+ "Allocated (%d KB) EFT ...\n", EFT_SIZE / 1024);
+
+ ha->eft_dma = tc_dma;
+ ha->eft = tc;
+}
+
+static void
+qla2x00_alloc_offload_mem(scsi_qla_host_t *vha)
+{
+ qla2x00_init_fce_trace(vha);
+ qla2x00_init_eft_trace(vha);
}
void
qla2x00_alloc_fw_dump(scsi_qla_host_t *vha)
{
- int rval;
uint32_t dump_size, fixed_size, mem_size, req_q_size, rsp_q_size,
eft_size, fce_size, mq_size;
struct qla_hw_data *ha = vha->hw;
struct req_que *req = ha->req_q_map[0];
struct rsp_que *rsp = ha->rsp_q_map[0];
struct qla2xxx_fw_dump *fw_dump;
- dma_addr_t tc_dma;
- void *tc;
dump_size = fixed_size = mem_size = eft_size = fce_size = mq_size = 0;
req_q_size = rsp_q_size = 0;
@@ -3166,39 +3176,13 @@ qla2x00_alloc_fw_dump(scsi_qla_host_t *vha)
}
if (ha->tgt.atio_ring)
mq_size += ha->tgt.atio_q_length * sizeof(request_t);
- /* Allocate memory for Fibre Channel Event Buffer. */
- if (!IS_QLA25XX(ha) && !IS_QLA81XX(ha) && !IS_QLA83XX(ha) &&
- !IS_QLA27XX(ha) && !IS_QLA28XX(ha))
- goto try_eft;
- fce_size = sizeof(struct qla2xxx_fce_chain) + FCE_SIZE;
-try_eft:
+ qla2x00_init_fce_trace(vha);
+ if (ha->fce)
+ fce_size = sizeof(struct qla2xxx_fce_chain) + FCE_SIZE;
+ qla2x00_init_eft_trace(vha);
if (ha->eft)
- dma_free_coherent(&ha->pdev->dev,
- EFT_SIZE, ha->eft, ha->eft_dma);
-
- /* Allocate memory for Extended Trace Buffer. */
- tc = dma_alloc_coherent(&ha->pdev->dev, EFT_SIZE, &tc_dma,
- GFP_KERNEL);
- if (!tc) {
- ql_log(ql_log_warn, vha, 0x00c1,
- "Unable to allocate (%d KB) for EFT.\n",
- EFT_SIZE / 1024);
- goto allocate;
- }
-
- rval = qla2x00_enable_eft_trace(vha, tc_dma, EFT_NUM_BUFFERS);
- if (rval) {
- ql_log(ql_log_warn, vha, 0x00c2,
- "Unable to initialize EFT (%d).\n", rval);
- dma_free_coherent(&ha->pdev->dev, EFT_SIZE, tc,
- tc_dma);
- }
- ql_dbg(ql_dbg_init, vha, 0x00c3,
- "Allocated (%d KB) EFT ...\n", EFT_SIZE / 1024);
- eft_size = EFT_SIZE;
- ha->eft_dma = tc_dma;
- ha->eft = tc;
+ eft_size = EFT_SIZE;
}
if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
@@ -3220,24 +3204,22 @@ qla2x00_alloc_fw_dump(scsi_qla_host_t *vha)
j, fwdt->dump_size);
dump_size += fwdt->dump_size;
}
- goto allocate;
+ } else {
+ req_q_size = req->length * sizeof(request_t);
+ rsp_q_size = rsp->length * sizeof(response_t);
+ dump_size = offsetof(struct qla2xxx_fw_dump, isp);
+ dump_size += fixed_size + mem_size + req_q_size + rsp_q_size
+ + eft_size;
+ ha->chain_offset = dump_size;
+ dump_size += mq_size + fce_size;
+ if (ha->exchoffld_buf)
+ dump_size += sizeof(struct qla2xxx_offld_chain) +
+ ha->exchoffld_size;
+ if (ha->exlogin_buf)
+ dump_size += sizeof(struct qla2xxx_offld_chain) +
+ ha->exlogin_size;
}
- req_q_size = req->length * sizeof(request_t);
- rsp_q_size = rsp->length * sizeof(response_t);
- dump_size = offsetof(struct qla2xxx_fw_dump, isp);
- dump_size += fixed_size + mem_size + req_q_size + rsp_q_size + eft_size;
- ha->chain_offset = dump_size;
- dump_size += mq_size + fce_size;
-
- if (ha->exchoffld_buf)
- dump_size += sizeof(struct qla2xxx_offld_chain) +
- ha->exchoffld_size;
- if (ha->exlogin_buf)
- dump_size += sizeof(struct qla2xxx_offld_chain) +
- ha->exlogin_size;
-
-allocate:
if (!ha->fw_dump_len || dump_size > ha->fw_dump_alloc_len) {
ql_dbg(ql_dbg_init, vha, 0x00c5,
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 7d73b6a..51154c0 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -4577,6 +4577,7 @@ qla2x00_free_fw_dump(struct qla_hw_data *ha)
ha->fce = NULL;
ha->fce_dma = 0;
+ ha->flags.fce_enabled = 0;
ha->eft = NULL;
ha->eft_dma = 0;
ha->fw_dumped = 0;
--
2.22.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers
2019-08-14 13:28 [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin Wilck
2019-08-14 13:28 ` [PATCH v2 1/2] scsi: qla2xxx: qla2x00_alloc_fw_dump: set ha->eft Martin Wilck
2019-08-14 13:28 ` [PATCH v2 2/2] scsi: qla2xxx: cleanup trace buffer initialization Martin Wilck
@ 2019-08-15 2:08 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2019-08-15 2:08 UTC (permalink / raw)
To: Martin Wilck
Cc: Martin K. Petersen, Himanshu Madhani, Bart Van Assche,
Joe Carnuccio, Quinn Tran, Hannes Reinecke, linux-scsi
Martin,
> The first patch of the series is a fix for a memory corruption we
> saw in a test where qla2xxx was loaded/unloaded repeatedly under
> memory pressure. The second one is a cleanup/consistency fix.
Applied to 5.4/scsi-queue, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-15 2:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14 13:28 [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin Wilck
2019-08-14 13:28 ` [PATCH v2 1/2] scsi: qla2xxx: qla2x00_alloc_fw_dump: set ha->eft Martin Wilck
2019-08-14 13:28 ` [PATCH v2 2/2] scsi: qla2xxx: cleanup trace buffer initialization Martin Wilck
2019-08-15 2:08 ` [PATCH v2 0/2] scsi: qla2xxx: fixes for FW trace/dump buffers Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.