.../ethernet/marvell/octeontx2/af/rvu_npc.c | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 deletions(-)
kmalloc_array() does not initializes memory to zero.
This causes issues with bitmap. Use devm_kcalloc()
to fix the issue.
Fixes: dd7842878633 ("octeontx2-af: Add new devlink param to configure maximum usable NIX block LFs")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
.../ethernet/marvell/octeontx2/af/rvu_npc.c | 55 ++++++++++---------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
index 167145bdcb75..7539e6f0290a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
@@ -1850,13 +1850,13 @@ void npc_mcam_rsrcs_deinit(struct rvu *rvu)
{
struct npc_mcam *mcam = &rvu->hw->mcam;
- kfree(mcam->bmap);
- kfree(mcam->bmap_reverse);
- kfree(mcam->entry2pfvf_map);
- kfree(mcam->cntr2pfvf_map);
- kfree(mcam->entry2cntr_map);
- kfree(mcam->cntr_refcnt);
- kfree(mcam->entry2target_pffunc);
+ devm_kfree(rvu->dev, mcam->bmap);
+ devm_kfree(rvu->dev, mcam->bmap_reverse);
+ devm_kfree(rvu->dev, mcam->entry2pfvf_map);
+ devm_kfree(rvu->dev, mcam->cntr2pfvf_map);
+ devm_kfree(rvu->dev, mcam->entry2cntr_map);
+ devm_kfree(rvu->dev, mcam->cntr_refcnt);
+ devm_kfree(rvu->dev, mcam->entry2target_pffunc);
kfree(mcam->counters.bmap);
}
@@ -1904,21 +1904,22 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
mcam->pf_offset = mcam->nixlf_offset + nixlf_count;
/* Allocate bitmaps for managing MCAM entries */
- mcam->bmap = kmalloc_array(BITS_TO_LONGS(mcam->bmap_entries),
- sizeof(long), GFP_KERNEL);
+ mcam->bmap = devm_kcalloc(rvu->dev, BITS_TO_LONGS(mcam->bmap_entries),
+ sizeof(long), GFP_KERNEL);
if (!mcam->bmap)
return -ENOMEM;
- mcam->bmap_reverse = kmalloc_array(BITS_TO_LONGS(mcam->bmap_entries),
- sizeof(long), GFP_KERNEL);
+ mcam->bmap_reverse = devm_kcalloc(rvu->dev,
+ BITS_TO_LONGS(mcam->bmap_entries),
+ sizeof(long), GFP_KERNEL);
if (!mcam->bmap_reverse)
goto free_bmap;
mcam->bmap_fcnt = mcam->bmap_entries;
/* Alloc memory for saving entry to RVU PFFUNC allocation mapping */
- mcam->entry2pfvf_map = kmalloc_array(mcam->bmap_entries,
- sizeof(u16), GFP_KERNEL);
+ mcam->entry2pfvf_map = devm_kcalloc(rvu->dev, mcam->bmap_entries,
+ sizeof(u16), GFP_KERNEL);
if (!mcam->entry2pfvf_map)
goto free_bmap_reverse;
@@ -1941,27 +1942,27 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
if (err)
goto free_entry_map;
- mcam->cntr2pfvf_map = kmalloc_array(mcam->counters.max,
- sizeof(u16), GFP_KERNEL);
+ mcam->cntr2pfvf_map = devm_kcalloc(rvu->dev, mcam->counters.max,
+ sizeof(u16), GFP_KERNEL);
if (!mcam->cntr2pfvf_map)
goto free_cntr_bmap;
/* Alloc memory for MCAM entry to counter mapping and for tracking
* counter's reference count.
*/
- mcam->entry2cntr_map = kmalloc_array(mcam->bmap_entries,
- sizeof(u16), GFP_KERNEL);
+ mcam->entry2cntr_map = devm_kcalloc(rvu->dev, mcam->bmap_entries,
+ sizeof(u16), GFP_KERNEL);
if (!mcam->entry2cntr_map)
goto free_cntr_map;
- mcam->cntr_refcnt = kmalloc_array(mcam->counters.max,
- sizeof(u16), GFP_KERNEL);
+ mcam->cntr_refcnt = devm_kcalloc(rvu->dev, mcam->counters.max,
+ sizeof(u16), GFP_KERNEL);
if (!mcam->cntr_refcnt)
goto free_entry_cntr_map;
/* Alloc memory for saving target device of mcam rule */
- mcam->entry2target_pffunc = kmalloc_array(mcam->total_entries,
- sizeof(u16), GFP_KERNEL);
+ mcam->entry2target_pffunc = devm_kcalloc(rvu->dev, mcam->total_entries,
+ sizeof(u16), GFP_KERNEL);
if (!mcam->entry2target_pffunc)
goto free_cntr_refcnt;
@@ -1978,19 +1979,19 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
return 0;
free_cntr_refcnt:
- kfree(mcam->cntr_refcnt);
+ devm_kfree(rvu->dev, mcam->cntr_refcnt);
free_entry_cntr_map:
- kfree(mcam->entry2cntr_map);
+ devm_kfree(rvu->dev, mcam->entry2cntr_map);
free_cntr_map:
- kfree(mcam->cntr2pfvf_map);
+ devm_kfree(rvu->dev, mcam->cntr2pfvf_map);
free_cntr_bmap:
kfree(mcam->counters.bmap);
free_entry_map:
- kfree(mcam->entry2pfvf_map);
+ devm_kfree(rvu->dev, mcam->entry2pfvf_map);
free_bmap_reverse:
- kfree(mcam->bmap_reverse);
+ devm_kfree(rvu->dev, mcam->bmap_reverse);
free_bmap:
- kfree(mcam->bmap);
+ devm_kfree(rvu->dev, mcam->bmap);
return -ENOMEM;
}
--
2.25.1On 1/22/2024 9:12 PM, Ratheesh Kannoth wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> kmalloc_array() does not initializes memory to zero.
> This causes issues with bitmap. Use devm_kcalloc()
> to fix the issue.
>
Is there any reason to not use:
bitmap_zalloc() and bitmap_free()?
> Fixes: dd7842878633 ("octeontx2-af: Add new devlink param to configure maximum usable NIX block LFs")
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
> .../ethernet/marvell/octeontx2/af/rvu_npc.c | 55 ++++++++++---------
> 1 file changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index 167145bdcb75..7539e6f0290a 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -1850,13 +1850,13 @@ void npc_mcam_rsrcs_deinit(struct rvu *rvu)
> {
> struct npc_mcam *mcam = &rvu->hw->mcam;
>
> - kfree(mcam->bmap);
> - kfree(mcam->bmap_reverse);
> - kfree(mcam->entry2pfvf_map);
> - kfree(mcam->cntr2pfvf_map);
> - kfree(mcam->entry2cntr_map);
> - kfree(mcam->cntr_refcnt);
> - kfree(mcam->entry2target_pffunc);
> + devm_kfree(rvu->dev, mcam->bmap);
> + devm_kfree(rvu->dev, mcam->bmap_reverse);
> + devm_kfree(rvu->dev, mcam->entry2pfvf_map);
> + devm_kfree(rvu->dev, mcam->cntr2pfvf_map);
> + devm_kfree(rvu->dev, mcam->entry2cntr_map);
> + devm_kfree(rvu->dev, mcam->cntr_refcnt);
> + devm_kfree(rvu->dev, mcam->entry2target_pffunc);
> kfree(mcam->counters.bmap);
> }
>
> @@ -1904,21 +1904,22 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
> mcam->pf_offset = mcam->nixlf_offset + nixlf_count;
>
> /* Allocate bitmaps for managing MCAM entries */
> - mcam->bmap = kmalloc_array(BITS_TO_LONGS(mcam->bmap_entries),
> - sizeof(long), GFP_KERNEL);
> + mcam->bmap = devm_kcalloc(rvu->dev, BITS_TO_LONGS(mcam->bmap_entries),
> + sizeof(long), GFP_KERNEL);
This is pretty much bitmap_zalloc(), except with devm. As Simon is
asking, is devm really necessary? If not bitmap_zalloc() and
bitmap_free() seem to fit your use well if I'm not mistaken.
Thanks,
Brett
> if (!mcam->bmap)
> return -ENOMEM;
>
> - mcam->bmap_reverse = kmalloc_array(BITS_TO_LONGS(mcam->bmap_entries),
> - sizeof(long), GFP_KERNEL);
> + mcam->bmap_reverse = devm_kcalloc(rvu->dev,
> + BITS_TO_LONGS(mcam->bmap_entries),
> + sizeof(long), GFP_KERNEL);
> if (!mcam->bmap_reverse)
> goto free_bmap;
>
> mcam->bmap_fcnt = mcam->bmap_entries;
>
> /* Alloc memory for saving entry to RVU PFFUNC allocation mapping */
> - mcam->entry2pfvf_map = kmalloc_array(mcam->bmap_entries,
> - sizeof(u16), GFP_KERNEL);
> + mcam->entry2pfvf_map = devm_kcalloc(rvu->dev, mcam->bmap_entries,
> + sizeof(u16), GFP_KERNEL);
> if (!mcam->entry2pfvf_map)
> goto free_bmap_reverse;
>
> @@ -1941,27 +1942,27 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
> if (err)
> goto free_entry_map;
>
> - mcam->cntr2pfvf_map = kmalloc_array(mcam->counters.max,
> - sizeof(u16), GFP_KERNEL);
> + mcam->cntr2pfvf_map = devm_kcalloc(rvu->dev, mcam->counters.max,
> + sizeof(u16), GFP_KERNEL);
> if (!mcam->cntr2pfvf_map)
> goto free_cntr_bmap;
>
> /* Alloc memory for MCAM entry to counter mapping and for tracking
> * counter's reference count.
> */
> - mcam->entry2cntr_map = kmalloc_array(mcam->bmap_entries,
> - sizeof(u16), GFP_KERNEL);
> + mcam->entry2cntr_map = devm_kcalloc(rvu->dev, mcam->bmap_entries,
> + sizeof(u16), GFP_KERNEL);
> if (!mcam->entry2cntr_map)
> goto free_cntr_map;
>
> - mcam->cntr_refcnt = kmalloc_array(mcam->counters.max,
> - sizeof(u16), GFP_KERNEL);
> + mcam->cntr_refcnt = devm_kcalloc(rvu->dev, mcam->counters.max,
> + sizeof(u16), GFP_KERNEL);
> if (!mcam->cntr_refcnt)
> goto free_entry_cntr_map;
>
> /* Alloc memory for saving target device of mcam rule */
> - mcam->entry2target_pffunc = kmalloc_array(mcam->total_entries,
> - sizeof(u16), GFP_KERNEL);
> + mcam->entry2target_pffunc = devm_kcalloc(rvu->dev, mcam->total_entries,
> + sizeof(u16), GFP_KERNEL);
> if (!mcam->entry2target_pffunc)
> goto free_cntr_refcnt;
>
> @@ -1978,19 +1979,19 @@ int npc_mcam_rsrcs_init(struct rvu *rvu, int blkaddr)
> return 0;
>
> free_cntr_refcnt:
> - kfree(mcam->cntr_refcnt);
> + devm_kfree(rvu->dev, mcam->cntr_refcnt);
> free_entry_cntr_map:
> - kfree(mcam->entry2cntr_map);
> + devm_kfree(rvu->dev, mcam->entry2cntr_map);
> free_cntr_map:
> - kfree(mcam->cntr2pfvf_map);
> + devm_kfree(rvu->dev, mcam->cntr2pfvf_map);
> free_cntr_bmap:
> kfree(mcam->counters.bmap);
> free_entry_map:
> - kfree(mcam->entry2pfvf_map);
> + devm_kfree(rvu->dev, mcam->entry2pfvf_map);
> free_bmap_reverse:
> - kfree(mcam->bmap_reverse);
> + devm_kfree(rvu->dev, mcam->bmap_reverse);
> free_bmap:
> - kfree(mcam->bmap);
> + devm_kfree(rvu->dev, mcam->bmap);
>
> return -ENOMEM;
> }
> --
> 2.25.1
>
>
+ Suman Ghosh <sumang@marvell.com>
On Tue, Jan 23, 2024 at 10:42:45AM +0530, Ratheesh Kannoth wrote:
> kmalloc_array() does not initializes memory to zero.
> This causes issues with bitmap. Use devm_kcalloc()
> to fix the issue.
Hi Ratheesh,
I assume that the reason that the cited commit moved away from devm_
allocations was to allow more natural management of the resources
independently of the life cycle of the driver instance. Or in other words,
the need to free the bitmaps in npc_mcam_rsrcs_deinit() probably indicates
that devm_ allocations of them aren't giving us anything.
So, perhaps kcalloc() is more appropriate than devm_kcalloc() ?
> Fixes: dd7842878633 ("octeontx2-af: Add new devlink param to configure maximum usable NIX block LFs")
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
© 2016 - 2025 Red Hat, Inc.