2017-04-05 13:52:18

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 0/5] IB/qib: Fine-tuning for four function implementations

From: Markus Elfring <[email protected]>
Date: Wed, 5 Apr 2017 15:34:32 +0200

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

Markus Elfring (5):
Use kcalloc() in qib_init_iba7322_funcs()
Use kmalloc_array() in qib_init_7322_variables()
Use kcalloc() in qib_alloc_devdata()
Use kcalloc() in qib_init_pportdata()
Adjust two size determinations in qib_init_pportdata()

drivers/infiniband/hw/qib/qib_iba7322.c | 20 ++++++++++++--------
drivers/infiniband/hw/qib/qib_init.c | 27 +++++++++++++--------------
2 files changed, 25 insertions(+), 22 deletions(-)

--
2.12.2


2017-04-05 13:53:57

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 2/5] IB/qib: Use kmalloc_array() in qib_init_7322_variables()

From: Markus Elfring <[email protected]>
Date: Wed, 5 Apr 2017 09:51:33 +0200

Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/qib/qib_iba7322.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
index e4e4e675c89c..490f432809b8 100644
--- a/drivers/infiniband/hw/qib/qib_iba7322.c
+++ b/drivers/infiniband/hw/qib/qib_iba7322.c
@@ -6442,12 +6442,15 @@ static int qib_init_7322_variables(struct qib_devdata *dd)
sbufcnt = dd->piobcnt2k + dd->piobcnt4k +
NUM_VL15_BUFS + BITS_PER_LONG - 1;
sbufcnt /= BITS_PER_LONG;
- dd->cspec->sendchkenable = kmalloc(sbufcnt *
- sizeof(*dd->cspec->sendchkenable), GFP_KERNEL);
- dd->cspec->sendgrhchk = kmalloc(sbufcnt *
- sizeof(*dd->cspec->sendgrhchk), GFP_KERNEL);
- dd->cspec->sendibchk = kmalloc(sbufcnt *
- sizeof(*dd->cspec->sendibchk), GFP_KERNEL);
+ dd->cspec->sendchkenable = kmalloc_array(sbufcnt,
+ sizeof(*dd->cspec->sendchkenable),
+ GFP_KERNEL);
+ dd->cspec->sendgrhchk = kmalloc_array(sbufcnt,
+ sizeof(*dd->cspec->sendgrhchk),
+ GFP_KERNEL);
+ dd->cspec->sendibchk = kmalloc_array(sbufcnt,
+ sizeof(*dd->cspec->sendibchk),
+ GFP_KERNEL);
if (!dd->cspec->sendchkenable || !dd->cspec->sendgrhchk ||
!dd->cspec->sendibchk) {
ret = -ENOMEM;
--
2.12.2

2017-04-05 13:54:38

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 3/5] IB/qib: Use kcalloc() in qib_alloc_devdata()

From: Markus Elfring <[email protected]>
Date: Wed, 5 Apr 2017 14:15:45 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/qib/qib_init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index b50240b1d5a4..9e680ca971e3 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1140,8 +1140,8 @@ struct qib_devdata *qib_alloc_devdata(struct pci_dev *pdev, size_t extra)
if (!qib_cpulist_count) {
u32 count = num_online_cpus();

- qib_cpulist = kzalloc(BITS_TO_LONGS(count) *
- sizeof(long), GFP_KERNEL);
+ qib_cpulist = kcalloc(BITS_TO_LONGS(count), sizeof(long),
+ GFP_KERNEL);
if (qib_cpulist)
qib_cpulist_count = count;
}
--
2.12.2

2017-04-05 13:55:19

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 4/5] IB/qib: Use kcalloc() in qib_init_pportdata()

From: Markus Elfring <[email protected]>
Date: Wed, 5 Apr 2017 14:20:10 +0200

* Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

* Replace the specification of data structures by pointer dereferences
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/qib/qib_init.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index 9e680ca971e3..101580f0460a 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -258,15 +258,15 @@ int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,

ppd->cc_max_table_entries =
ppd->cc_supported_table_entries/IB_CCT_ENTRIES;
-
- size = IB_CC_TABLE_CAP_DEFAULT * sizeof(struct ib_cc_table_entry)
- * IB_CCT_ENTRIES;
- ppd->ccti_entries = kzalloc(size, GFP_KERNEL);
+ ppd->ccti_entries = kcalloc(IB_CC_TABLE_CAP_DEFAULT * IB_CCT_ENTRIES,
+ sizeof(*ppd->ccti_entries),
+ GFP_KERNEL);
if (!ppd->ccti_entries)
goto bail;

- size = IB_CC_CCS_ENTRIES * sizeof(struct ib_cc_congestion_entry);
- ppd->congestion_entries = kzalloc(size, GFP_KERNEL);
+ ppd->congestion_entries = kcalloc(IB_CC_CCS_ENTRIES,
+ sizeof(*ppd->congestion_entries),
+ GFP_KERNEL);
if (!ppd->congestion_entries)
goto bail_1;

--
2.12.2

2017-04-05 13:52:17

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 1/5] IB/qib: Use kcalloc() in qib_init_iba7322_funcs()

From: Markus Elfring <[email protected]>
Date: Wed, 5 Apr 2017 09:43:54 +0200

* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/qib/qib_iba7322.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
index af9f596bb68b..e4e4e675c89c 100644
--- a/drivers/infiniband/hw/qib/qib_iba7322.c
+++ b/drivers/infiniband/hw/qib/qib_iba7322.c
@@ -7324,8 +7324,9 @@ struct qib_devdata *qib_init_iba7322_funcs(struct pci_dev *pdev,
actual_cnt -= dd->num_pports;

tabsize = actual_cnt;
- dd->cspec->msix_entries = kzalloc(tabsize *
- sizeof(struct qib_msix_entry), GFP_KERNEL);
+ dd->cspec->msix_entries = kcalloc(tabsize,
+ sizeof(*dd->cspec->msix_entries),
+ GFP_KERNEL);
if (!dd->cspec->msix_entries)
tabsize = 0;

--
2.12.2

2017-04-05 13:57:42

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 5/5] IB/qib: Adjust two size determinations in qib_init_pportdata()

From: Markus Elfring <[email protected]>
Date: Wed, 5 Apr 2017 15:00:44 +0200

* Replace the specification of two data structures by pointer dereferences
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

* Delete the local variable "size" which became unnecessary with
this refactoring.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/qib/qib_init.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index 101580f0460a..e223226ed94d 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -222,8 +222,6 @@ struct qib_ctxtdata *qib_create_ctxtdata(struct qib_pportdata *ppd, u32 ctxt,
int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,
u8 hw_pidx, u8 port)
{
- int size;
-
ppd->dd = dd;
ppd->hw_pidx = hw_pidx;
ppd->port = port; /* IB port number, not index */
@@ -270,13 +268,14 @@ int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,
if (!ppd->congestion_entries)
goto bail_1;

- size = sizeof(struct cc_table_shadow);
- ppd->ccti_entries_shadow = kzalloc(size, GFP_KERNEL);
+ ppd->ccti_entries_shadow = kzalloc(sizeof(*ppd->ccti_entries_shadow),
+ GFP_KERNEL);
if (!ppd->ccti_entries_shadow)
goto bail_2;

- size = sizeof(struct ib_cc_congestion_setting_attr);
- ppd->congestion_entries_shadow = kzalloc(size, GFP_KERNEL);
+ ppd->congestion_entries_shadow = kzalloc(sizeof(*ppd
+ ->congestion_entries_shadow),
+ GFP_KERNEL);
if (!ppd->congestion_entries_shadow)
goto bail_3;

--
2.12.2

2017-04-05 14:23:54

by Yuval Shaia

[permalink] [raw]
Subject: Re: [PATCH 2/5] IB/qib: Use kmalloc_array() in qib_init_7322_variables()

On Wed, Apr 05, 2017 at 03:52:48PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 5 Apr 2017 09:51:33 +0200
>
> Multiplications for the size determination of memory allocations
> indicated that array data structures should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/infiniband/hw/qib/qib_iba7322.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
> index e4e4e675c89c..490f432809b8 100644
> --- a/drivers/infiniband/hw/qib/qib_iba7322.c
> +++ b/drivers/infiniband/hw/qib/qib_iba7322.c
> @@ -6442,12 +6442,15 @@ static int qib_init_7322_variables(struct qib_devdata *dd)
> sbufcnt = dd->piobcnt2k + dd->piobcnt4k +
> NUM_VL15_BUFS + BITS_PER_LONG - 1;
> sbufcnt /= BITS_PER_LONG;
> - dd->cspec->sendchkenable = kmalloc(sbufcnt *
> - sizeof(*dd->cspec->sendchkenable), GFP_KERNEL);
> - dd->cspec->sendgrhchk = kmalloc(sbufcnt *
> - sizeof(*dd->cspec->sendgrhchk), GFP_KERNEL);
> - dd->cspec->sendibchk = kmalloc(sbufcnt *
> - sizeof(*dd->cspec->sendibchk), GFP_KERNEL);
> + dd->cspec->sendchkenable = kmalloc_array(sbufcnt,
> + sizeof(*dd->cspec->sendchkenable),
> + GFP_KERNEL);
> + dd->cspec->sendgrhchk = kmalloc_array(sbufcnt,
> + sizeof(*dd->cspec->sendgrhchk),
> + GFP_KERNEL);
> + dd->cspec->sendibchk = kmalloc_array(sbufcnt,
> + sizeof(*dd->cspec->sendibchk),
> + GFP_KERNEL);
> if (!dd->cspec->sendchkenable || !dd->cspec->sendgrhchk ||
> !dd->cspec->sendibchk) {
> ret = -ENOMEM;

Reviewed-by: Yuval Shaia <[email protected]>

> --
> 2.12.2
>
> --
> 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

2017-04-05 14:33:34

by Yuval Shaia

[permalink] [raw]
Subject: Re: [PATCH 5/5] IB/qib: Adjust two size determinations in qib_init_pportdata()

On Wed, Apr 05, 2017 at 03:55:39PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 5 Apr 2017 15:00:44 +0200
>
> * Replace the specification of two data structures by pointer dereferences
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> * Delete the local variable "size" which became unnecessary with
> this refactoring.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/infiniband/hw/qib/qib_init.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
> index 101580f0460a..e223226ed94d 100644
> --- a/drivers/infiniband/hw/qib/qib_init.c
> +++ b/drivers/infiniband/hw/qib/qib_init.c
> @@ -222,8 +222,6 @@ struct qib_ctxtdata *qib_create_ctxtdata(struct qib_pportdata *ppd, u32 ctxt,
> int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,
> u8 hw_pidx, u8 port)
> {
> - int size;
> -
> ppd->dd = dd;
> ppd->hw_pidx = hw_pidx;
> ppd->port = port; /* IB port number, not index */
> @@ -270,13 +268,14 @@ int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,
> if (!ppd->congestion_entries)
> goto bail_1;
>
> - size = sizeof(struct cc_table_shadow);
> - ppd->ccti_entries_shadow = kzalloc(size, GFP_KERNEL);
> + ppd->ccti_entries_shadow = kzalloc(sizeof(*ppd->ccti_entries_shadow),
> + GFP_KERNEL);
> if (!ppd->ccti_entries_shadow)
> goto bail_2;
>
> - size = sizeof(struct ib_cc_congestion_setting_attr);
> - ppd->congestion_entries_shadow = kzalloc(size, GFP_KERNEL);
> + ppd->congestion_entries_shadow = kzalloc(sizeof(*ppd
> + ->congestion_entries_shadow),
> + GFP_KERNEL);

Not related to this patch but is related to your patch-set - can you check
the array allocations in lines 264 and 268?

Besides that:
Reviewed-by: Yuval Shaia <[email protected]>

> if (!ppd->congestion_entries_shadow)
> goto bail_3;
>
> --
> 2.12.2
>
> --
> 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

2017-04-05 14:35:09

by Yuval Shaia

[permalink] [raw]
Subject: Re: [PATCH 1/5] IB/qib: Use kcalloc() in qib_init_iba7322_funcs()

On Wed, Apr 05, 2017 at 03:51:52PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 5 Apr 2017 09:43:54 +0200
>
> * A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kcalloc".
>
> This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data structure by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/infiniband/hw/qib/qib_iba7322.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
> index af9f596bb68b..e4e4e675c89c 100644
> --- a/drivers/infiniband/hw/qib/qib_iba7322.c
> +++ b/drivers/infiniband/hw/qib/qib_iba7322.c
> @@ -7324,8 +7324,9 @@ struct qib_devdata *qib_init_iba7322_funcs(struct pci_dev *pdev,
> actual_cnt -= dd->num_pports;
>
> tabsize = actual_cnt;
> - dd->cspec->msix_entries = kzalloc(tabsize *
> - sizeof(struct qib_msix_entry), GFP_KERNEL);
> + dd->cspec->msix_entries = kcalloc(tabsize,
> + sizeof(*dd->cspec->msix_entries),
> + GFP_KERNEL);

Are we fine with loosing the zeroing of the entries?

> if (!dd->cspec->msix_entries)
> tabsize = 0;
>
> --
> 2.12.2
>
> --
> 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

2017-04-05 14:55:37

by SF Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 1/5] IB/qib: Use kcalloc() in qib_init_iba7322_funcs()

>> @@ -7324,8 +7324,9 @@ struct qib_devdata *qib_init_iba7322_funcs(struct pci_dev *pdev,
>> actual_cnt -= dd->num_pports;
>>
>> tabsize = actual_cnt;
>> - dd->cspec->msix_entries = kzalloc(tabsize *
>> - sizeof(struct qib_msix_entry), GFP_KERNEL);
>> + dd->cspec->msix_entries = kcalloc(tabsize,
>> + sizeof(*dd->cspec->msix_entries),
>> + GFP_KERNEL);
>
> Are we fine with loosing the zeroing of the entries?

How did you get this concern?

Do you really miss such functionality from the other interface?

Regards,
Markus

2017-04-05 15:05:07

by SF Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 5/5] IB/qib: Adjust two size determinations in qib_init_pportdata()

>> @@ -270,13 +268,14 @@ int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,
>> if (!ppd->congestion_entries)
>> goto bail_1;
>>
>> - size = sizeof(struct cc_table_shadow);
>> - ppd->ccti_entries_shadow = kzalloc(size, GFP_KERNEL);
>> + ppd->ccti_entries_shadow = kzalloc(sizeof(*ppd->ccti_entries_shadow),
>> + GFP_KERNEL);
>> if (!ppd->ccti_entries_shadow)
>> goto bail_2;
>>
>> - size = sizeof(struct ib_cc_congestion_setting_attr);
>> - ppd->congestion_entries_shadow = kzalloc(size, GFP_KERNEL);
>> + ppd->congestion_entries_shadow = kzalloc(sizeof(*ppd
>> + ->congestion_entries_shadow),
>> + GFP_KERNEL);
>
> Not related to this patch but is related to your patch-set - can you check
> the array allocations in lines 264 and 268?

Do you refer to source code places here which are affected by the update step
"[PATCH 4/5] IB/qib: Use kcalloc() in qib_init_pportdata()"?


> Besides that:
> Reviewed-by: Yuval Shaia <[email protected]>

Do you find the proposed change for the shown data types really acceptable
in these function calls?

Regards,
Markus

2017-04-05 15:08:06

by Johannes Thumshirn

[permalink] [raw]
Subject: Re: [PATCH 1/5] IB/qib: Use kcalloc() in qib_init_iba7322_funcs()

On Wed, Apr 05, 2017 at 04:54:40PM +0200, SF Markus Elfring wrote:
> >> @@ -7324,8 +7324,9 @@ struct qib_devdata *qib_init_iba7322_funcs(struct pci_dev *pdev,
> >> actual_cnt -= dd->num_pports;
> >>
> >> tabsize = actual_cnt;
> >> - dd->cspec->msix_entries = kzalloc(tabsize *
> >> - sizeof(struct qib_msix_entry), GFP_KERNEL);
> >> + dd->cspec->msix_entries = kcalloc(tabsize,
> >> + sizeof(*dd->cspec->msix_entries),
> >> + GFP_KERNEL);
> >
> > Are we fine with loosing the zeroing of the entries?
>
> How did you get this concern?
>
> Do you really miss such functionality from the other interface?

Ahm... Don't kzalloc() and kcalloc() both pass in __GFP_ZERO?

--
Johannes Thumshirn Storage
[email protected] +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

2017-04-05 15:10:38

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH 5/5] IB/qib: Adjust two size determinations in qib_init_pportdata()

On Wed, 2017-04-05 at 15:55 +0200, SF Markus Elfring wrote:
> - size = sizeof(struct ib_cc_congestion_setting_attr);
> - ppd->congestion_entries_shadow = kzalloc(size, GFP_KERNEL);
> + ppd->congestion_entries_shadow = kzalloc(sizeof(*ppd
> + ->congestion_entries_shadow),
> + GFP_KERNEL);

The way how the above line has been split looks really weird. Please
move the entire kzalloc() call to the next line such that "*ppd" and
"->congestion_entries_shadow" appear on the same line.

Thanks,

Bart.

2017-04-05 15:16:05

by Yuval Shaia

[permalink] [raw]
Subject: Re: [PATCH 5/5] IB/qib: Adjust two size determinations in qib_init_pportdata()

On Wed, Apr 05, 2017 at 05:04:35PM +0200, SF Markus Elfring wrote:
> >> @@ -270,13 +268,14 @@ int qib_init_pportdata(struct qib_pportdata *ppd, struct qib_devdata *dd,
> >> if (!ppd->congestion_entries)
> >> goto bail_1;
> >>
> >> - size = sizeof(struct cc_table_shadow);
> >> - ppd->ccti_entries_shadow = kzalloc(size, GFP_KERNEL);
> >> + ppd->ccti_entries_shadow = kzalloc(sizeof(*ppd->ccti_entries_shadow),
> >> + GFP_KERNEL);
> >> if (!ppd->ccti_entries_shadow)
> >> goto bail_2;
> >>
> >> - size = sizeof(struct ib_cc_congestion_setting_attr);
> >> - ppd->congestion_entries_shadow = kzalloc(size, GFP_KERNEL);
> >> + ppd->congestion_entries_shadow = kzalloc(sizeof(*ppd
> >> + ->congestion_entries_shadow),
> >> + GFP_KERNEL);
> >
> > Not related to this patch but is related to your patch-set - can you check
> > the array allocations in lines 264 and 268?
>
> Do you refer to source code places here which are affected by the update step
> "[PATCH 4/5] IB/qib: Use kcalloc() in qib_init_pportdata()"?

Oops, please ignore.

>
>
> > Besides that:
> > Reviewed-by: Yuval Shaia <[email protected]>
>
> Do you find the proposed change for the shown data types really acceptable
> in these function calls?

I found that the fix brings no harm to the existing code.

>
> Regards,
> Markus

2017-04-05 15:19:20

by Yuval Shaia

[permalink] [raw]
Subject: Re: [PATCH 1/5] IB/qib: Use kcalloc() in qib_init_iba7322_funcs()

On Wed, Apr 05, 2017 at 05:06:58PM +0200, Johannes Thumshirn wrote:
> On Wed, Apr 05, 2017 at 04:54:40PM +0200, SF Markus Elfring wrote:
> > >> @@ -7324,8 +7324,9 @@ struct qib_devdata *qib_init_iba7322_funcs(struct pci_dev *pdev,
> > >> actual_cnt -= dd->num_pports;
> > >>
> > >> tabsize = actual_cnt;
> > >> - dd->cspec->msix_entries = kzalloc(tabsize *
> > >> - sizeof(struct qib_msix_entry), GFP_KERNEL);
> > >> + dd->cspec->msix_entries = kcalloc(tabsize,
> > >> + sizeof(*dd->cspec->msix_entries),
> > >> + GFP_KERNEL);
> > >
> > > Are we fine with loosing the zeroing of the entries?
> >
> > How did you get this concern?
> >
> > Do you really miss such functionality from the other interface?
>
> Ahm... Don't kzalloc() and kcalloc() both pass in __GFP_ZERO?

Yes.
Please ignore my comment.

>
> --
> Johannes Thumshirn Storage
> [email protected] +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
> GF: Felix Imend?rffer, Jane Smithard, Graham Norton
> HRB 21284 (AG N?rnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

2017-04-05 15:22:20

by SF Markus Elfring

[permalink] [raw]
Subject: Re: IB/qib: Adjust two size determinations in qib_init_pportdata()

> I found that the fix brings no harm to the existing code.

How do you think about to take another look at remaining update candidates
at other source code places for this Linux module?

Regards,
Markus

2017-04-06 00:38:01

by Dennis Dalessandro

[permalink] [raw]
Subject: Re: [PATCH 0/5] IB/qib: Fine-tuning for four function implementations

On 04/05/2017 09:50 AM, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 5 Apr 2017 15:34:32 +0200
>
> A few update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (5):
> Use kcalloc() in qib_init_iba7322_funcs()
> Use kmalloc_array() in qib_init_7322_variables()
> Use kcalloc() in qib_alloc_devdata()
> Use kcalloc() in qib_init_pportdata()
> Adjust two size determinations in qib_init_pportdata()
>
> drivers/infiniband/hw/qib/qib_iba7322.c | 20 ++++++++++++--------
> drivers/infiniband/hw/qib/qib_init.c | 27 +++++++++++++--------------
> 2 files changed, 25 insertions(+), 22 deletions(-)
>

Doug,

I haven't reviewed these in detail yet but seems be along the lines of
the patchset you decided not to take for cxgb [1]. I recommend not
picking this up either. If something in here catches our eye Mike or I
one will Ack it.

[1] http://marc.info/?l=linux-rdma&m=149141638506039&w=2

-Denny

2017-04-06 19:48:57

by Yuval Shaia

[permalink] [raw]
Subject: Re: IB/qib: Adjust two size determinations in qib_init_pportdata()

On Wed, Apr 05, 2017 at 05:21:22PM +0200, SF Markus Elfring wrote:
> > I found that the fix brings no harm to the existing code.
>
> How do you think about to take another look at remaining update candidates
> at other source code places for this Linux module?

You mean the other patches in this patch-set?

>
> 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

2017-04-06 20:33:32

by SF Markus Elfring

[permalink] [raw]
Subject: Re: IB/qib: Adjust two size determinations in qib_init_pportdata()

>> How do you think about to take another look at remaining update candidates
>> at other source code places for this Linux module?
>
> You mean the other patches in this patch-set?

Partly, yes. - Do you care for adjustments around numbered jump labels
(and suggestions from the script "checkpatch.pl") for example?

Will it be more important to check return values from all calls of a function
like "kmalloc" in related source files?

Regards,
Markus