2018-01-27 20:15:56

by SF Markus Elfring

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

From: Markus Elfring <[email protected]>
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



2018-01-27 20:17:22

by SF Markus Elfring

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

From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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


2018-01-27 20:18:39

by SF Markus Elfring

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

From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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


2018-01-29 03:45:41

by Devesh Sharma

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

On Sun, Jan 28, 2018 at 1:45 AM, SF Markus Elfring
<[email protected]> wrote:
> From: Markus Elfring <[email protected]>
> 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 <[email protected]>

>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2018-01-29 09:32:45

by SF Markus Elfring

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

> 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

2018-01-29 12:10:10

by Devesh Sharma

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

On Mon, Jan 29, 2018 at 3:01 PM, SF Markus Elfring
<[email protected]> 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 <[email protected]>

>
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2018-01-29 14:21:46

by Jonathan Toppins

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

On 01/27/2018 03:15 PM, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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 <[email protected]>

2018-02-01 22:51:24

by Jason Gunthorpe

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

On Sat, Jan 27, 2018 at 09:15:09PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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