2020-08-10 02:39:31

by Xu Wang

[permalink] [raw]
Subject: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

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

Signed-off-by: Xu Wang <[email protected]>
---
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 1944bf5264db..26988ad7ec97 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -412,7 +412,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,

new->flags = flags;

- new->q.info = devm_kzalloc(dev, sizeof(*new->q.info) * num_descs,
+ new->q.info = devm_kcalloc(dev, num_descs, sizeof(*new->q.info),
GFP_KERNEL);
if (!new->q.info) {
netdev_err(lif->netdev, "Cannot allocate queue info\n");
@@ -462,7 +462,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
new->intr.index = IONIC_INTR_INDEX_NOT_ASSIGNED;
}

- new->cq.info = devm_kzalloc(dev, sizeof(*new->cq.info) * num_descs,
+ new->cq.info = devm_kcalloc(dev, num_descs, sizeof(*new->cq.info),
GFP_KERNEL);
if (!new->cq.info) {
netdev_err(lif->netdev, "Cannot allocate completion queue info\n");
--
2.17.1


2020-08-10 03:21:21

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

On Mon, 2020-08-10 at 02:38 +0000, Xu Wang wrote:
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "devm_kcalloc".
[]
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
[]
> @@ -412,7 +412,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
>
> new->flags = flags;
>
> - new->q.info = devm_kzalloc(dev, sizeof(*new->q.info) * num_descs,
> + new->q.info = devm_kcalloc(dev, num_descs, sizeof(*new->q.info),
> GFP_KERNEL);
> if (!new->q.info) {
> netdev_err(lif->netdev, "Cannot allocate queue info\n");

You could also remove these unnecessary allocation error messages.
There is an existing dump_stack() on allocation failure.

> @@ -462,7 +462,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> new->intr.index = IONIC_INTR_INDEX_NOT_ASSIGNED;
> }
>
> - new->cq.info = devm_kzalloc(dev, sizeof(*new->cq.info) * num_descs,
> + new->cq.info = devm_kcalloc(dev, num_descs, sizeof(*new->cq.info),
> GFP_KERNEL);
> if (!new->cq.info) {
> netdev_err(lif->netdev, "Cannot allocate completion queue info\n");

2020-08-10 03:51:18

by Shannon Nelson

[permalink] [raw]
Subject: Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

On 8/9/20 8:20 PM, Joe Perches wrote:
> On Mon, 2020-08-10 at 02:38 +0000, Xu Wang wrote:
>> A multiplication for the size determination of a memory allocation
>> indicated that an array data structure should be processed.
>> Thus use the corresponding function "devm_kcalloc".
> []
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> []
>> @@ -412,7 +412,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
>>
>> new->flags = flags;
>>
>> - new->q.info = devm_kzalloc(dev, sizeof(*new->q.info) * num_descs,
>> + new->q.info = devm_kcalloc(dev, num_descs, sizeof(*new->q.info),
>> GFP_KERNEL);
>> if (!new->q.info) {
>> netdev_err(lif->netdev, "Cannot allocate queue info\n");
> You could also remove these unnecessary allocation error messages.
> There is an existing dump_stack() on allocation failure.
>
Yes, the dump_stack() tells you which function had the allocation
failure, but since there are multiple allocation operations in this same
function, I find these helpful in knowing quickly which one of the
allocations failed, without having to track down the symbols and source
for whatever distro's kernel this might have happened in.

sln

2020-08-10 03:55:19

by Shannon Nelson

[permalink] [raw]
Subject: Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

On 8/9/20 7:38 PM, Xu Wang wrote:
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "devm_kcalloc".
>
> Signed-off-by: Xu Wang <[email protected]>

Acked-by: Shannon Nelson <[email protected]>

> ---
> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 1944bf5264db..26988ad7ec97 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -412,7 +412,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
>
> new->flags = flags;
>
> - new->q.info = devm_kzalloc(dev, sizeof(*new->q.info) * num_descs,
> + new->q.info = devm_kcalloc(dev, num_descs, sizeof(*new->q.info),
> GFP_KERNEL);
> if (!new->q.info) {
> netdev_err(lif->netdev, "Cannot allocate queue info\n");
> @@ -462,7 +462,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> new->intr.index = IONIC_INTR_INDEX_NOT_ASSIGNED;
> }
>
> - new->cq.info = devm_kzalloc(dev, sizeof(*new->cq.info) * num_descs,
> + new->cq.info = devm_kcalloc(dev, num_descs, sizeof(*new->cq.info),
> GFP_KERNEL);
> if (!new->cq.info) {
> netdev_err(lif->netdev, "Cannot allocate completion queue info\n");

2020-08-10 03:56:26

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

On Sun, 2020-08-09 at 20:50 -0700, Shannon Nelson wrote:
> On 8/9/20 8:20 PM, Joe Perches wrote:
> > On Mon, 2020-08-10 at 02:38 +0000, Xu Wang wrote:
> > > A multiplication for the size determination of a memory allocation
> > > indicated that an array data structure should be processed.
> > > Thus use the corresponding function "devm_kcalloc".
> > []
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > []
> > > @@ -412,7 +412,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> > >
> > > new->flags = flags;
> > >
> > > - new->q.info = devm_kzalloc(dev, sizeof(*new->q.info) * num_descs,
> > > + new->q.info = devm_kcalloc(dev, num_descs, sizeof(*new->q.info),
> > > GFP_KERNEL);
> > > if (!new->q.info) {
> > > netdev_err(lif->netdev, "Cannot allocate queue info\n");
> > You could also remove these unnecessary allocation error messages.
> > There is an existing dump_stack() on allocation failure.
> >
> Yes, the dump_stack() tells you which function had the allocation
> failure, but since there are multiple allocation operations in this same
> function, I find these helpful in knowing quickly which one of the
> allocations failed, without having to track down the symbols and source
> for whatever distro's kernel this might have happened in.

If you do chose the message, might as well add __GFP_NOWARN
to the allocation to avoid the dump_stack().

But honestly, if any allocation fails, you're OOM anyway.

2020-08-11 17:37:43

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

From: Xu Wang <[email protected]>
Date: Mon, 10 Aug 2020 02:38:07 +0000

> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "devm_kcalloc".
>
> Signed-off-by: Xu Wang <[email protected]>

Applied, thanks.