scsi: qedi: Fix error codes in qedi_alloc_global_queues()
PATCH 1/2 scsi: qedi: Fix error codes in qedi_alloc_global_queues()
This function had some left over code that returned 1 on error instead negative error codes. Convert everything to use negative error codes. The caller treats all non-zero returns the same so this does not affect run time.
A couple places set "rc" instead of "status" so those error paths ended up returning success by mistake. Get rid of the "rc" variable and use "status" everywhere.
Remove the bogus "status = 0" initialization, as a future proofing measure so the compiler will warn about uninitialized error codes.
--- drivers/scsi/qedi/qedi_main.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/scsi/qedi/qedi_main.c +7 -7
@@ -1621,7 +1621,7 @@ static int qedi_alloc_global_queues(struct qedi_ctx *qedi) {u32 *list;int i; -int status =0 , rc; +int status;u32 *pbl;dma_addr_t page;int num_pages;
@@ -1632,14 +1632,14 @@ static int qedi_alloc_global_queues(struct qedi_ctx *qedi)*/ if (!qedi->num_queues) {QEDI_ERR (&qedi->dbg_ctx,"No MSI-X vectors available!\n" ); -return 1 ; +return -ENOMEM ; }/* Make sure we allocated the PBL that will contain the physical * addresses of our queues */ if (!qedi->p_cpuq) { - status =1 ; + status = -EINVAL ; goto mem_alloc_failure; }
@@ -1654,13 +1654,13 @@ static int qedi_alloc_global_queues(struct qedi_ctx *qedi) "qedi->global_queues=%p.\n", qedi->global_queues);/* Allocate DMA coherent buffers for BDQ */ - rc =qedi_alloc_bdq (qedi); -if (rc) + status =qedi_alloc_bdq (qedi); +if (status) goto mem_alloc_failure;/* Allocate DMA coherent buffers for NVM_ISCSI_CFG */ - rc =qedi_alloc_nvm_iscsi_cfg (qedi); -if (rc) + status =qedi_alloc_nvm_iscsi_cfg (qedi); +if (status) goto mem_alloc_failure;/* Allocate a CQ and an associated PBL for each MSI-X
Signature · 1 line
2.20.1
PATCH 2/2 scsi: qedf: fix error codes in qedf_alloc_global_queues()
This driver has some left over "return 1" on failure style code mixed with "return negative error codes" style code. The caller doesn't care so we should just convert everything to return negative error codes.
Then there was a problem that there were two variables used to store error codes which just resulted in confusion. If qedf_alloc_bdq() returned a negative error code, we accidentally returned success instead of propagating the error code. So get rid of the "rc" variable and use "status" every where.
Also remove the "status = 0" initialization so that these sorts of bugs will be detected by the compiler in the future.
--- drivers/scsi/qedf/qedf_main.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/scsi/qedf/qedf_main.c +5 -5
@@ -3004,7 +3004,7 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf) {u32 *list;int i; -int status =0 , rc; +int status;u32 *pbl;dma_addr_t page;int num_pages;
@@ -3016,7 +3016,7 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf)*/ if (!qedf->num_queues) {QEDF_ERR (&(qedf->dbg_ctx),"No MSI-X vectors available!\n" ); -return 1 ; +return -ENOMEM ; }/*
@@ -3024,7 +3024,7 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf)* addresses of our queues */ if (!qedf->p_cpuq) { - status =1 ; + status = -EINVAL ;QEDF_ERR (&qedf->dbg_ctx,"p_cpuq is NULL.\n" ); goto mem_alloc_failure; }
@@ -3040,8 +3040,8 @@ static int qedf_alloc_global_queues(struct qedf_ctx *qedf) "qedf->global_queues=%p.\n", qedf->global_queues);/* Allocate DMA coherent buffers for BDQ */ - rc =qedf_alloc_bdq (qedf); -if (rc) { + status =qedf_alloc_bdq (qedf); +if (status) {QEDF_ERR (&qedf->dbg_ctx,"Unable to allocate bdq.\n" ); goto mem_alloc_failure; }
Signature · 1 line
2.20.1
Quoted 85 lines
-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: Tuesday, August 10, 2021 2:18 PM
To: Nilesh Javali <[email protected]>; Manish Rangankar <[email protected]>
Cc: GR-QLogic-Storage-Upstream <GR-QLogic-Storage- [email protected]>; James E.J. Bottomley <[email protected]>; Martin K. Petersen <[email protected]>; Hannes Reinecke <[email protected]>; Arun Easi <[email protected]>; Adheer Chandravanshi <[email protected]>; Johannes Thumshirn <[email protected]>; [email protected]; kernel- [email protected]
Subject: [EXT] [PATCH 1/2] scsi: qedi: Fix error codes inqedi_alloc_global_queues()External Email
----------------------------------------------------------------------
This function had some left over code that returned 1 on error instead negative error codes. Convert everything to use negative error codes.
The caller treats all non-zero returns the same so this does not affect run time.A couple places set "rc" instead of "status" so those error paths ended up returning success by mistake. Get rid of the "rc" variable and use "status" everywhere.
Remove the bogus "status = 0" initialization, as a future proofing measure so the compiler will warn about uninitialized error codes.
Fixes ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driverframework.")
Signed-off-by Dan Carpenter <[email protected]>--- drivers/scsi/qedi/qedi_main.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/scsi/qedi/qedi_main.c index renamed from drivers/scsi/qedi/qedi_main.c +0
-0
0b0acb827071..e6dc0b495a82100644drivers/scsi/qedi/qedi_main.c +7
-7@@ -1621,7 +1621,7 @@ static int qedi_alloc_global_queues(struct qedi_ctx *qedi) {u32 *list;int i; -int status =0 , rc; +int status;u32 *pbl;dma_addr_t page;int num_pages;@@ -1632,14 +1632,14 @@ static int qedi_alloc_global_queues(struct qedi_ctx*qedi) */ if (!qedi->num_queues) {QEDI_ERR (&qedi->dbg_ctx,"No MSI-X vectors available!\n" ); -return 1 ; +return -ENOMEM ; }/* Make sure we allocated the PBL that will contain the physical * addresses of our queues */ if (!qedi->p_cpuq) { - status =1 ; + status = -EINVAL ; goto mem_alloc_failure; }@@ -1654,13 +1654,13 @@ static int qedi_alloc_global_queues(struct qedi_ctx *qedi) "qedi->global_queues=%p.\n", qedi->global_queues);/* Allocate DMA coherent buffers for BDQ */ - rc =qedi_alloc_bdq (qedi); -if (rc) + status =qedi_alloc_bdq (qedi); +if (status) goto mem_alloc_failure;/* Allocate DMA coherent buffers for NVM_ISCSI_CFG */ - rc =qedi_alloc_nvm_iscsi_cfg (qedi); -if (rc) + status =qedi_alloc_nvm_iscsi_cfg (qedi); +if (status) goto mem_alloc_failure;
Quoted 3 lines
/* Allocate a CQ and an associated PBL for each MSI-X
--
2.20.1
Thanks,
Dan,
Quoted 4 lines
This function had some left over code that returned 1 on error instead negative error codes. Convert everything to use negative error codes. The caller treats all non-zero returns the same so this does not affect run time.
Applied 1+2 to 5.15/scsi-staging, thanks!
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering