[PATCH 0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[PATCH 0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 27 Jan 2018 21:10:12 +0100

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete two error messages for a failed memory allocation
  Use common error handling code

 drivers/infiniband/hw/bnxt_re/qplib_res.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

-- 
2.16.1

[PATCH 1/2] RDMA/bnxt_re: Delete two error messages for a failed memory allocation in bnxt_qplib_alloc_dpi_tbl()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 27 Jan 2018 20:40:11 +0100

Omit extra messages for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/bnxt_re/qplib_res.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.c b/drivers/infiniband/hw/bnxt_re/qplib_res.c
index ad37d54affcc..9dacfd24869b 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.c
@@ -707,8 +707,6 @@ static int bnxt_qplib_alloc_dpi_tbl(struct bnxt_qplib_res     *res,
 	dpit->app_tbl = kcalloc(dpit->max, sizeof(void *), GFP_KERNEL);
 	if (!dpit->app_tbl) {
 		pci_iounmap(res->pdev, dpit->dbr_bar_reg_iomem);
-		dev_err(&res->pdev->dev,
-			"QPLIB: DPI app tbl allocation failed");
 		return -ENOMEM;
 	}
 
@@ -721,9 +719,6 @@ static int bnxt_qplib_alloc_dpi_tbl(struct bnxt_qplib_res     *res,
 		pci_iounmap(res->pdev, dpit->dbr_bar_reg_iomem);
 		kfree(dpit->app_tbl);
 		dpit->app_tbl = NULL;
-		dev_err(&res->pdev->dev,
-			"QPLIB: DPI tbl allocation failed for size = %d",
-			bytes);
 		return -ENOMEM;
 	}
 
-- 
2.16.1

[PATCH 2/2] RDMA/bnxt_re: Use common error handling code in bnxt_qplib_alloc_dpi_tbl()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 27 Jan 2018 20:56:56 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/bnxt_re/qplib_res.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.c b/drivers/infiniband/hw/bnxt_re/qplib_res.c
index 9dacfd24869b..539a5d44e6db 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.c
@@ -705,10 +705,8 @@ static int bnxt_qplib_alloc_dpi_tbl(struct bnxt_qplib_res     *res,
 	dpit->max = dbr_len / PAGE_SIZE;
 
 	dpit->app_tbl = kcalloc(dpit->max, sizeof(void *), GFP_KERNEL);
-	if (!dpit->app_tbl) {
-		pci_iounmap(res->pdev, dpit->dbr_bar_reg_iomem);
-		return -ENOMEM;
-	}
+	if (!dpit->app_tbl)
+		goto unmap_io;
 
 	bytes = dpit->max >> 3;
 	if (!bytes)
@@ -716,15 +714,18 @@ static int bnxt_qplib_alloc_dpi_tbl(struct bnxt_qplib_res     *res,
 
 	dpit->tbl = kmalloc(bytes, GFP_KERNEL);
 	if (!dpit->tbl) {
-		pci_iounmap(res->pdev, dpit->dbr_bar_reg_iomem);
 		kfree(dpit->app_tbl);
 		dpit->app_tbl = NULL;
-		return -ENOMEM;
+		goto unmap_io;
 	}
 
 	memset((u8 *)dpit->tbl, 0xFF, bytes);
 
 	return 0;
+
+unmap_io:
+	pci_iounmap(res->pdev, dpit->dbr_bar_reg_iomem);
+	return -ENOMEM;
 }
 
 /* PKEYs */
-- 
2.16.1

Re: [PATCH 0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[Devesh Sharma]

On Sun, Jan 28, 2018 at 1:45 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 27 Jan 2018 21:10:12 +0100
>
> Two update suggestions were taken into account
> from static source code analysis.

You don't need 2 patches when changing same lines of code. Could
you squash both and send your changes in a single patch.
The patches look good to me otherwise.
-Thanks

Reviewed-By: Devesh Sharma <devesh.sharma@broadcom.com>

>
> Markus Elfring (2):
>   Delete two error messages for a failed memory allocation
>   Use common error handling code
>
>  drivers/infiniband/hw/bnxt_re/qplib_res.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> --
> 2.16.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[SF Markus Elfring]

> You don't need 2 patches when changing same lines of code.

Are these really the same?


> Could you squash both and send your changes in a single patch.

I prefer to keep the deletion of questionable error messages separate
from the refactoring for a bit of exception handling.


> The patches look good to me otherwise.

Thanks for your constructive feedback.

Regards,
Markus

Re: [0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[Devesh Sharma]

On Mon, Jan 29, 2018 at 3:01 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> You don't need 2 patches when changing same lines of code.
>
> Are these really the same?
>
>
>> Could you squash both and send your changes in a single patch.
>
> I prefer to keep the deletion of questionable error messages separate
> from the refactoring for a bit of exception handling.
>

Okay fair enough
Series Acked-By: Devesh Sharma <devesh.sharma@broadcom.com>

>
>> The patches look good to me otherwise.
>
> Thanks for your constructive feedback.
>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[Jonathan Toppins]

On 01/27/2018 03:15 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 27 Jan 2018 21:10:12 +0100
> 
> Two update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (2):
>   Delete two error messages for a failed memory allocation
>   Use common error handling code
> 
>  drivers/infiniband/hw/bnxt_re/qplib_res.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 

I like it.

Acked-by: Jonathan Toppins <jtoppins@redhat.com>

Re: [PATCH 0/2] RDMA/bnxt_re: Adjustments for bnxt_qplib_alloc_dpi_tbl()

[Jason Gunthorpe]

On Sat, Jan 27, 2018 at 09:15:09PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 27 Jan 2018 21:10:12 +0100
> 
> Two update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (2):
>   Delete two error messages for a failed memory allocation
>   Use common error handling code

Applied to for-next with Jonathan and Devesh's ack, thanks all

Jason