* [PATCH 0/3] scsi: qedf: Fix an error handling path + cleanups
@ 2021-05-23 11:01 Christophe JAILLET
2021-05-23 11:02 ` [PATCH 1/3] scsi: qedf: Fix an error handling path in 'qedf_alloc_global_queues()' Christophe JAILLET
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Christophe JAILLET @ 2021-05-23 11:01 UTC (permalink / raw)
To: skashyap, jhasan, GR-QLogic-Storage-Upstream, jejb,
martin.petersen, chad.dupuis, arun.easi, nilesh.javali
Cc: linux-scsi, linux-kernel, kernel-janitors, Christophe JAILLET
Patch 1/3 fixes a case where success is un-intentionaly returned.
The 2 other patches are clean-up.
Patch 2/3 is a clean-up in the same function, to improve its readability.
Pacth 3/3 is a proposal to both remove some lines of code and improve readability.
Christophe JAILLET (3):
scsi: qedf: Fix an error handling path in 'qedf_alloc_global_queues()'
scsi: qedf: Simplify 'qedf_alloc_global_queues()'
scsi: qedf: Axe a few useless lines of code
drivers/scsi/qedf/qedf_main.c | 102 +++++++++++++---------------------
1 file changed, 38 insertions(+), 64 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] scsi: qedf: Fix an error handling path in 'qedf_alloc_global_queues()'
2021-05-23 11:01 [PATCH 0/3] scsi: qedf: Fix an error handling path + cleanups Christophe JAILLET
@ 2021-05-23 11:02 ` Christophe JAILLET
2021-05-23 11:02 ` [PATCH 2/3] scsi: qedf: Simplify 'qedf_alloc_global_queues()' Christophe JAILLET
2021-05-23 11:02 ` [PATCH 3/3] scsi: qedf: Axe a few useless lines of code Christophe JAILLET
2 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2021-05-23 11:02 UTC (permalink / raw)
To: skashyap, jhasan, GR-QLogic-Storage-Upstream, jejb,
martin.petersen, chad.dupuis, arun.easi, nilesh.javali
Cc: linux-scsi, linux-kernel, kernel-janitors, Christophe JAILLET
If 'qedf_alloc_bdq()' fails, 'status' is known to be 0, so we report
success to the caller.
In fact going to 'mem_alloc_failure' is pointless here, because we try to
free some resources that have not been allocated yet.
This is however harmless because 'qedf_free_global_queues()' is robust
enough.
So make a direct return instead, as already done just a few lines above.
While at it, also do a direct return if '!qedf->p_cpuq'. Going to
'mem_alloc_failure' is also pointless here and mixing goto and direct
return is spurious. So make it clear that nothing has to be undone.
Fixes: 61d8658b4a43 ("scsi: qedf: Add QLogic FastLinQ offload FCoE driver framework.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/scsi/qedf/qedf_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 7368a40ba649..ff3a1b183a7c 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -3022,9 +3022,8 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf)
* addresses of our queues
*/
if (!qedf->p_cpuq) {
- status = 1;
QEDF_ERR(&qedf->dbg_ctx, "p_cpuq is NULL.\n");
- goto mem_alloc_failure;
+ return 1;
}
qedf->global_queues = kzalloc((sizeof(struct global_queue *)
@@ -3041,7 +3040,7 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf)
rc = qedf_alloc_bdq(qedf);
if (rc) {
QEDF_ERR(&qedf->dbg_ctx, "Unable to allocate bdq.\n");
- goto mem_alloc_failure;
+ return -ENOMEM;
}
/* Allocate a CQ and an associated PBL for each MSI-X vector */
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] scsi: qedf: Simplify 'qedf_alloc_global_queues()'
2021-05-23 11:01 [PATCH 0/3] scsi: qedf: Fix an error handling path + cleanups Christophe JAILLET
2021-05-23 11:02 ` [PATCH 1/3] scsi: qedf: Fix an error handling path in 'qedf_alloc_global_queues()' Christophe JAILLET
@ 2021-05-23 11:02 ` Christophe JAILLET
2021-05-23 11:02 ` [PATCH 3/3] scsi: qedf: Axe a few useless lines of code Christophe JAILLET
2 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2021-05-23 11:02 UTC (permalink / raw)
To: skashyap, jhasan, GR-QLogic-Storage-Upstream, jejb,
martin.petersen, chad.dupuis, arun.easi, nilesh.javali
Cc: linux-scsi, linux-kernel, kernel-janitors, Christophe JAILLET
In 'qedf_alloc_global_queues()', 'qedf->global_queues[i]', is used many
times. Use an intermediate variable with a much shorter name, 'q', to
simplify and make the code more readable.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/scsi/qedf/qedf_main.c | 53 +++++++++++++++--------------------
1 file changed, 22 insertions(+), 31 deletions(-)
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index ff3a1b183a7c..3b80d4298f15 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -3045,55 +3045,46 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf)
/* Allocate a CQ and an associated PBL for each MSI-X vector */
for (i = 0; i < qedf->num_queues; i++) {
- qedf->global_queues[i] = kzalloc(sizeof(struct global_queue),
- GFP_KERNEL);
- if (!qedf->global_queues[i]) {
+ struct global_queue *q;
+
+ q = kzalloc(sizeof(*q), GFP_KERNEL);
+ if (!q) {
QEDF_WARN(&(qedf->dbg_ctx), "Unable to allocate "
"global queue %d.\n", i);
status = -ENOMEM;
goto mem_alloc_failure;
}
+ qedf->global_queues[i] = q;
- qedf->global_queues[i]->cq_mem_size =
+ q->cq_mem_size =
FCOE_PARAMS_CQ_NUM_ENTRIES * sizeof(struct fcoe_cqe);
- qedf->global_queues[i]->cq_mem_size =
- ALIGN(qedf->global_queues[i]->cq_mem_size, QEDF_PAGE_SIZE);
-
- qedf->global_queues[i]->cq_pbl_size =
- (qedf->global_queues[i]->cq_mem_size /
- PAGE_SIZE) * sizeof(void *);
- qedf->global_queues[i]->cq_pbl_size =
- ALIGN(qedf->global_queues[i]->cq_pbl_size, QEDF_PAGE_SIZE);
-
- qedf->global_queues[i]->cq =
- dma_alloc_coherent(&qedf->pdev->dev,
- qedf->global_queues[i]->cq_mem_size,
- &qedf->global_queues[i]->cq_dma,
- GFP_KERNEL);
-
- if (!qedf->global_queues[i]->cq) {
+ q->cq_mem_size = ALIGN(q->cq_mem_size, QEDF_PAGE_SIZE);
+
+ q->cq_pbl_size = (q->cq_mem_size / PAGE_SIZE) * sizeof(void *);
+ q->cq_pbl_size = ALIGN(q->cq_pbl_size, QEDF_PAGE_SIZE);
+
+ q->cq = dma_alloc_coherent(&qedf->pdev->dev, q->cq_mem_size,
+ &q->cq_dma, GFP_KERNEL);
+
+ if (!q->cq) {
QEDF_WARN(&(qedf->dbg_ctx), "Could not allocate cq.\n");
status = -ENOMEM;
goto mem_alloc_failure;
}
- qedf->global_queues[i]->cq_pbl =
- dma_alloc_coherent(&qedf->pdev->dev,
- qedf->global_queues[i]->cq_pbl_size,
- &qedf->global_queues[i]->cq_pbl_dma,
- GFP_KERNEL);
+ q->cq_pbl = dma_alloc_coherent(&qedf->pdev->dev, q->cq_pbl_size,
+ &q->cq_pbl_dma, GFP_KERNEL);
- if (!qedf->global_queues[i]->cq_pbl) {
+ if (!q->cq_pbl) {
QEDF_WARN(&(qedf->dbg_ctx), "Could not allocate cq PBL.\n");
status = -ENOMEM;
goto mem_alloc_failure;
}
/* Create PBL */
- num_pages = qedf->global_queues[i]->cq_mem_size /
- QEDF_PAGE_SIZE;
- page = qedf->global_queues[i]->cq_dma;
- pbl = (u32 *)qedf->global_queues[i]->cq_pbl;
+ num_pages = q->cq_mem_size / QEDF_PAGE_SIZE;
+ page = q->cq_dma;
+ pbl = (u32 *)q->cq_pbl;
while (num_pages--) {
*pbl = U64_LO(page);
@@ -3103,7 +3094,7 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf)
page += QEDF_PAGE_SIZE;
}
/* Set the initial consumer index for cq */
- qedf->global_queues[i]->cq_cons_idx = 0;
+ q->cq_cons_idx = 0;
}
list = (u32 *)qedf->p_cpuq;
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] scsi: qedf: Axe a few useless lines of code
2021-05-23 11:01 [PATCH 0/3] scsi: qedf: Fix an error handling path + cleanups Christophe JAILLET
2021-05-23 11:02 ` [PATCH 1/3] scsi: qedf: Fix an error handling path in 'qedf_alloc_global_queues()' Christophe JAILLET
2021-05-23 11:02 ` [PATCH 2/3] scsi: qedf: Simplify 'qedf_alloc_global_queues()' Christophe JAILLET
@ 2021-05-23 11:02 ` Christophe JAILLET
2 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2021-05-23 11:02 UTC (permalink / raw)
To: skashyap, jhasan, GR-QLogic-Storage-Upstream, jejb,
martin.petersen, chad.dupuis, arun.easi, nilesh.javali
Cc: linux-scsi, linux-kernel, kernel-janitors, Christophe JAILLET
Simplify a bit this file a remove a few lines of code:
- keep function parameters on the same line
- remove unneeded return at the end of a function that return void
- remove duplicated and useless empty lines
- remove unneeded {} when there is only one statement
- remove a few () around 'qedf->dbg_ctx' so that we can...
- ... merge a few messages that were split on 2 lines
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Many more clean-up could be done to improve readability in this file.
The above few steps have the advantage to also reduce the number of LoC.
Anyway, that's mostly a matter of taste.
---
drivers/scsi/qedf/qedf_main.c | 44 +++++++++++------------------------
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 3b80d4298f15..0fb5b89fa4a2 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -304,9 +304,7 @@ static void qedf_flogi_resp(struct fc_seq *seq, struct fc_frame *fp,
static struct fc_seq *qedf_elsct_send(struct fc_lport *lport, u32 did,
struct fc_frame *fp, unsigned int op,
- void (*resp)(struct fc_seq *,
- struct fc_frame *,
- void *),
+ void (*resp)(struct fc_seq *, struct fc_frame *, void *),
void *arg, u32 timeout)
{
struct qedf_ctx *qedf = lport_priv(lport);
@@ -630,7 +628,6 @@ static void qedf_link_update(void *dev, struct qed_link_output *link)
}
}
-
static void qedf_dcbx_handler(void *dev, struct qed_dcbx_get *get, u32 mib_type)
{
struct qedf_ctx *qedf = (struct qedf_ctx *)dev;
@@ -739,7 +736,6 @@ static int qedf_eh_abort(struct scsi_cmnd *sc_cmd)
goto out;
}
-
io_req = (struct qedf_ioreq *)sc_cmd->SCp.ptr;
if (!io_req) {
QEDF_ERR(&qedf->dbg_ctx,
@@ -972,9 +968,8 @@ static int qedf_eh_host_reset(struct scsi_cmnd *sc_cmd)
static int qedf_slave_configure(struct scsi_device *sdev)
{
- if (qedf_queue_depth) {
+ if (qedf_queue_depth)
scsi_change_queue_depth(sdev, qedf_queue_depth);
- }
return 0;
}
@@ -1181,7 +1176,6 @@ static int qedf_xmit(struct fc_lport *lport, struct fc_frame *fp)
cp = NULL;
}
-
/* adjust skb network/transport offsets to match mac/fcoe/port */
skb_push(skb, elen + hlen);
skb_reset_mac_header(skb);
@@ -1899,7 +1893,6 @@ static int qedf_vport_create(struct fc_vport *vport, bool disabled)
fc_disc_init(vn_port);
fc_disc_config(vn_port, vn_port);
-
/* Allocate the exchange manager */
shost = vport_to_shost(vport);
n_port = shost_priv(shost);
@@ -2000,8 +1993,7 @@ static void qedf_wait_for_vport_destroy(struct qedf_ctx *qedf)
{
struct fc_host_attrs *fc_host = shost_to_fc_host(qedf->lport->host);
- QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_NPIV,
- "Entered.\n");
+ QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_NPIV, "Entered.\n");
while (fc_host->npiv_vports_inuse > 0) {
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_NPIV,
"Waiting for all vports to be reaped.\n");
@@ -2289,7 +2281,6 @@ static bool qedf_process_completions(struct qedf_fastpath *fp)
return true;
}
-
/* MSI-X fastpath handler code */
static irqreturn_t qedf_msix_handler(int irq, void *dev_id)
{
@@ -2623,7 +2614,6 @@ static void qedf_ll2_process_skb(struct work_struct *work)
kfree_skb(skb);
out:
kfree(skb_work);
- return;
}
static int qedf_ll2_rx(void *cookie, struct sk_buff *skb,
@@ -2746,8 +2736,7 @@ static int qedf_prepare_sb(struct qedf_ctx *qedf)
GFP_KERNEL);
if (!qedf->fp_array) {
- QEDF_ERR(&(qedf->dbg_ctx), "fastpath array allocation "
- "failed.\n");
+ QEDF_ERR(&qedf->dbg_ctx, "fastpath array allocation failed.\n");
return -ENOMEM;
}
@@ -2768,9 +2757,8 @@ static int qedf_prepare_sb(struct qedf_ctx *qedf)
}
fp->sb_id = id;
fp->qedf = qedf;
- fp->cq_num_entries =
- qedf->global_queues[id]->cq_mem_size /
- sizeof(struct fcoe_cqe);
+ fp->cq_num_entries = qedf->global_queues[id]->cq_mem_size /
+ sizeof(struct fcoe_cqe);
}
err:
return 0;
@@ -2815,7 +2803,6 @@ void qedf_process_cqe(struct qedf_ctx *qedf, struct fcoe_cqe *cqe)
return;
}
-
switch (comp_type) {
case FCOE_GOOD_COMPLETION_CQE_TYPE:
atomic_inc(&fcport->free_sqes);
@@ -3154,8 +3141,7 @@ static int qedf_set_fcoe_pf_param(struct qedf_ctx *qedf)
rval = qedf_alloc_global_queues(qedf);
if (rval) {
- QEDF_ERR(&(qedf->dbg_ctx), "Global queue allocation "
- "failed.\n");
+ QEDF_ERR(&qedf->dbg_ctx, "Global queue allocation failed.\n");
return 1;
}
@@ -3326,8 +3312,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_INFO, "qedf->io_mempool=%p.\n",
qedf->io_mempool);
- sprintf(host_buf, "qedf_%u_link",
- qedf->lport->host->host_no);
+ sprintf(host_buf, "qedf_%u_link", qedf->lport->host->host_no);
qedf->link_update_wq = create_workqueue(host_buf);
INIT_DELAYED_WORK(&qedf->link_update, qedf_handle_link_update);
INIT_DELAYED_WORK(&qedf->link_recovery, qedf_link_recovery);
@@ -3581,8 +3566,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
qedf->timer_work_queue =
create_workqueue(host_buf);
if (!qedf->timer_work_queue) {
- QEDF_ERR(&(qedf->dbg_ctx), "Failed to start timer "
- "workqueue.\n");
+ QEDF_ERR(&qedf->dbg_ctx, "Failed to start timer workqueue.\n");
rc = -ENOMEM;
goto err7;
}
@@ -3826,13 +3810,13 @@ void qedf_schedule_hw_err_handler(void *dev, enum qed_hw_err_type err_type)
{
struct qedf_ctx *qedf = dev;
- QEDF_ERR(&(qedf->dbg_ctx),
- "Hardware error handler scheduled, event=%d.\n",
- err_type);
+ QEDF_ERR(&qedf->dbg_ctx,
+ "Hardware error handler scheduled, event=%d.\n",
+ err_type);
if (test_bit(QEDF_IN_RECOVERY, &qedf->flags)) {
- QEDF_ERR(&(qedf->dbg_ctx),
- "Already in recovery, not scheduling board disable work.\n");
+ QEDF_ERR(&qedf->dbg_ctx,
+ "Already in recovery, not scheduling board disable work.\n");
return;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-23 11:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-23 11:01 [PATCH 0/3] scsi: qedf: Fix an error handling path + cleanups Christophe JAILLET
2021-05-23 11:02 ` [PATCH 1/3] scsi: qedf: Fix an error handling path in 'qedf_alloc_global_queues()' Christophe JAILLET
2021-05-23 11:02 ` [PATCH 2/3] scsi: qedf: Simplify 'qedf_alloc_global_queues()' Christophe JAILLET
2021-05-23 11:02 ` [PATCH 3/3] scsi: qedf: Axe a few useless lines of code Christophe JAILLET
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).