diff mbox series

[net] octeontx2-af: Initialize bitmap arrays.

Message ID 20240123051245.3801246-1-rkannoth@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] octeontx2-af: Initialize bitmap arrays. | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1095 this patch: 1095
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1095 this patch: 1095
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 108 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-01-23--21-00 (tests: 494)

Commit Message

Ratheesh Kannoth Jan. 23, 2024, 5:12 a.m. UTC
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(-)

Comments

Simon Horman Jan. 23, 2024, 6:17 p.m. UTC | #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>
Ratheesh Kannoth Jan. 24, 2024, 3:01 a.m. UTC | #2
> From: Simon Horman <horms@kernel.org>
> Subject: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> 
> 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() ?
This was a comment from @Subbaraya Sundeep Bhatta during our internal review.  
Could you please help with below questions/doubts ?
1. why devm_kfree() API  is available if it is done independently 
2. I could see instances of devm_kfree() usage in current kernel where it does explicit calls.

-Ratheesh
Simon Horman Jan. 24, 2024, 10:37 a.m. UTC | #3
On Wed, Jan 24, 2024 at 03:01:15AM +0000, Ratheesh Kannoth wrote:
> > From: Simon Horman <horms@kernel.org>
> > Subject: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> > 
> > 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() ?
> This was a comment from @Subbaraya Sundeep Bhatta during our internal review.  
> Could you please help with below questions/doubts ?
> 1. why devm_kfree() API  is available if it is done independently 

Hi Ratheesh,

I think the question is: if the devm_kfree() calls are removed,
then is the lifecycle of the objects in question managed correctly?

> 2. I could see instances of devm_kfree() usage in current kernel where it does explicit calls.

Sure. But in this case the use of devm_* doesn't seem to be adding
anything if the memory is _always_ freed by explicit calls to
devm_kfree().
Ratheesh Kannoth Jan. 24, 2024, 10:43 a.m. UTC | #4
> From: Simon Horman <horms@kernel.org>
> Subject: Re: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> 
> I think the question is: if the devm_kfree() calls are removed, then is the
> lifecycle of the objects in question managed correctly?
If lifecycle of the objects are managed correctly without devm_kfree(), why this API is 
Provided and exported in kernel ?

> 
> > 2. I could see instances of devm_kfree() usage in current kernel where it
> does explicit calls.
> 
> Sure. But in this case the use of devm_* doesn't seem to be adding anything
> if the memory is _always_ freed by explicit calls to devm_kfree().
I got it.  I would like to keep the diff minimal (rather than deleting lines diff). would this be okay ?
Simon Horman Jan. 24, 2024, 9:15 p.m. UTC | #5
On Wed, Jan 24, 2024 at 10:43:26AM +0000, Ratheesh Kannoth wrote:
> > From: Simon Horman <horms@kernel.org>
> > Subject: Re: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> > 
> > I think the question is: if the devm_kfree() calls are removed, then is the
> > lifecycle of the objects in question managed correctly?
> If lifecycle of the objects are managed correctly without devm_kfree(), why this API is 
> Provided and exported in kernel ?

When the lifecycle of an object is such that it is freed when
the device is detached and at no other time, then devm_* can be helpful
because there is no need for devm_free calls.

I do understand that devm_free() exists, and there are cases where
it makes sense. But I don't think devm_ is buying us anything here.

> 
> > 
> > > 2. I could see instances of devm_kfree() usage in current kernel where it
> > does explicit calls.
> > 
> > Sure. But in this case the use of devm_* doesn't seem to be adding anything
> > if the memory is _always_ freed by explicit calls to devm_kfree().
> I got it.  I would like to keep the diff minimal (rather than deleting lines diff). would this be okay ?

My feeling is that if you change your patch to:

1. Use kcalloc() instead of devm_kcalloc()
2. Not change kfree() calls to devm_kfree()

Then you will end up with a smaller diff than the current patch.
And it will address the problem described in the patch description.
Brett Creeley Jan. 25, 2024, 2:14 a.m. UTC | #6
On 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
> 
>
Ratheesh Kannoth Jan. 25, 2024, 5:03 a.m. UTC | #7
> From: Simon Horman <horms@kernel.org>
> Subject: Re: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.


> I do understand that devm_free() exists, and there are cases where it makes
> sense. But I don't think devm_ is buying us anything here.
For graceful Error handling, this API makes sense.   

> 1. Use kcalloc() instead of devm_kcalloc() 2. Not change kfree() calls to
> devm_kfree()
> 
> Then you will end up with a smaller diff than the current patch.
> And it will address the problem described in the patch description.
ACK.
Ratheesh Kannoth Jan. 25, 2024, 5:06 a.m. UTC | #8
> From: Brett Creeley <bcreeley@amd.com>
> Subject: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> Is there any reason to not use:
> 
> bitmap_zalloc() and bitmap_free()?
Will follow simon's suggestion to keep patch diff minimal. As bitmap_zalloc() does not give any advantage over the other.

> 
> This is pretty much bitmap_zalloc(), except with devm. As Simon is asking, is
> devm really necessary? 
Will use kcalloc.
Brett Creeley Jan. 25, 2024, 3:56 p.m. UTC | #9
On 1/24/2024 9:06 PM, Ratheesh Kannoth wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
>> From: Brett Creeley <bcreeley@amd.com>
>> Subject: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
>> Is there any reason to not use:
>>
>> bitmap_zalloc() and bitmap_free()?
> Will follow simon's suggestion to keep patch diff minimal. As bitmap_zalloc() does not give any advantage over the other.

It does make some sense because in multiple places you are open coding 
bitmap_zalloc()->bitmap_alloc() in multiple places.

For example:

         mcam->bmap = kmalloc_array(BITS_TO_LONGS(mcam->bmap_entries),
+                                  sizeof(long), GFP_KERNEL | __GFP_ZERO);

This is exactly what bitmap_zalloc()->bitmap_alloc() are doing.

> 
>>
>> This is pretty much bitmap_zalloc(), except with devm. As Simon is asking, is
>> devm really necessary?
> Will use kcalloc.
Simon Horman Jan. 26, 2024, 9:01 p.m. UTC | #10
On Thu, Jan 25, 2024 at 07:56:22AM -0800, Brett Creeley wrote:
> 
> 
> On 1/24/2024 9:06 PM, Ratheesh Kannoth wrote:
> > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> > 
> > 
> > > From: Brett Creeley <bcreeley@amd.com>
> > > Subject: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> > > Is there any reason to not use:
> > > 
> > > bitmap_zalloc() and bitmap_free()?
> > Will follow simon's suggestion to keep patch diff minimal. As bitmap_zalloc() does not give any advantage over the other.
> 
> It does make some sense because in multiple places you are open coding
> bitmap_zalloc()->bitmap_alloc() in multiple places.
> 
> For example:
> 
>         mcam->bmap = kmalloc_array(BITS_TO_LONGS(mcam->bmap_entries),
> +                                  sizeof(long), GFP_KERNEL | __GFP_ZERO);
> 
> This is exactly what bitmap_zalloc()->bitmap_alloc() are doing.

Yes, I agree and I should have suggested using
bitmap_zalloc() and bitmap_free().
Ratheesh Kannoth Jan. 29, 2024, 5:24 a.m. UTC | #11
> From: Simon Horman <horms@kernel.org>
> Subject: Re: [EXT] Re: [PATCH net] octeontx2-af: Initialize bitmap arrays.
> >         mcam->bmap = kmalloc_array(BITS_TO_LONGS(mcam-
> >bmap_entries),
> > +                                  sizeof(long), GFP_KERNEL | __GFP_ZERO);
> >
> > This is exactly what bitmap_zalloc()->bitmap_alloc() are doing.
> 
> Yes, I agree and I should have suggested using
> bitmap_zalloc() and bitmap_free().
> 
ACK.
diff mbox series

Patch

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