rte_sched: correctly free allocated subport memory
diff mbox series

Message ID 20200526172455.6457-1-hrvoje.habjanic@zg.ht.hr
State Accepted
Headers show
Series
  • rte_sched: correctly free allocated subport memory
Related show

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-testing warning Testing issues
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Hrvoje Habjanic May 26, 2020, 5:24 p.m. UTC
In function rte_sched_subport_free (lib/librte_sched/rte_sched.c,
line 865), there is code to free all allocated stuff related to
scheduler subport. First there are some checks, and in the end,
rte_bitmap_free is called.

Now, rte_bitmap_free is a dummy function, and it just checks if
provided pointer to bitmap is valid or not. So, actual memory for
subport is not freed.

This patch fixes this by removing call to rte_bitmap_free, and
instead calling rte_free.

Signed-off-by: Hrvoje Habjanic <hrvoje.habjanic@zg.ht.hr>
---
 lib/librte_sched/rte_sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Singh, Jasvinder May 27, 2020, 11:48 a.m. UTC | #1
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Hrvoje Habjanic
> Sent: Tuesday, May 26, 2020 6:25 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] rte_sched: correctly free allocated subport
> memory
> 
> In function rte_sched_subport_free (lib/librte_sched/rte_sched.c, line 865),
> there is code to free all allocated stuff related to scheduler subport. First
> there are some checks, and in the end, rte_bitmap_free is called.
> 
> Now, rte_bitmap_free is a dummy function, and it just checks if provided
> pointer to bitmap is valid or not. So, actual memory for subport is not freed.
> 
> This patch fixes this by removing call to rte_bitmap_free, and instead calling
> rte_free.
> 
> Signed-off-by: Hrvoje Habjanic <hrvoje.habjanic@zg.ht.hr>
> ---
>  lib/librte_sched/rte_sched.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c index
> c0983ddda..f15a3b515 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -888,7 +888,7 @@ rte_sched_subport_free(struct rte_sched_port *port,
>  		}
>  	}
> 
> -	rte_bitmap_free(subport->bmp);
> +	rte_free(subport);
>  }
> 
>  void
> --
> 2.17.1
Hi Hrvoje;

I guess this is your first patch to dpdk.org, here are some suggestions when you send bug fixes;

- When sending fixes, please use "fix" word in the subject line, e.g- rte_sched: fix subport memory leak
- The commit message should include commit id corresponding to the line that you fixes as shown below for this case. 
   Fixes: d9213b829a31 ("sched: remove pipe params config from port level")

Patch looks good to me.

Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>
Stephen Hemminger May 29, 2020, 5:48 a.m. UTC | #2
On Tue, 26 May 2020 19:24:55 +0200
Hrvoje Habjanic <hrvoje.habjanic@zg.ht.hr> wrote:

> In function rte_sched_subport_free (lib/librte_sched/rte_sched.c,
> line 865), there is code to free all allocated stuff related to
> scheduler subport. First there are some checks, and in the end,
> rte_bitmap_free is called.
> 
> Now, rte_bitmap_free is a dummy function, and it just checks if
> provided pointer to bitmap is valid or not. So, actual memory for
> subport is not freed.
> 
> This patch fixes this by removing call to rte_bitmap_free, and
> instead calling rte_free.
> 
> Signed-off-by: Hrvoje Habjanic <hrvoje.habjanic@zg.ht.hr>


Fixes: ce7c4fd7c2ac ("sched: add pipe config to subport level")

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Hrvoje Habjanic May 30, 2020, 8:05 a.m. UTC | #3
On 27. 05. 2020. 13:48, Singh, Jasvinder wrote:
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Hrvoje Habjanic
>> Sent: Tuesday, May 26, 2020 6:25 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] rte_sched: correctly free allocated subport
>> memory
>>
>> In function rte_sched_subport_free (lib/librte_sched/rte_sched.c, line 865),
>> there is code to free all allocated stuff related to scheduler subport. First
>> there are some checks, and in the end, rte_bitmap_free is called.
>>
>> Now, rte_bitmap_free is a dummy function, and it just checks if provided
>> pointer to bitmap is valid or not. So, actual memory for subport is not freed.
>>
>> This patch fixes this by removing call to rte_bitmap_free, and instead calling
>> rte_free.
>>
>> Signed-off-by: Hrvoje Habjanic <hrvoje.habjanic@zg.ht.hr>
>> ---
>>  lib/librte_sched/rte_sched.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c index
>> c0983ddda..f15a3b515 100644
>> --- a/lib/librte_sched/rte_sched.c
>> +++ b/lib/librte_sched/rte_sched.c
>> @@ -888,7 +888,7 @@ rte_sched_subport_free(struct rte_sched_port *port,
>>  		}
>>  	}
>>
>> -	rte_bitmap_free(subport->bmp);
>> +	rte_free(subport);
>>  }
>>
>>  void
>> --
>> 2.17.1
> Hi Hrvoje;
>
> I guess this is your first patch to dpdk.org, here are some suggestions when you send bug fixes;

Yes, it is.

>
> - When sending fixes, please use "fix" word in the subject line, e.g- rte_sched: fix subport memory leak
> - The commit message should include commit id corresponding to the line that you fixes as shown below for this case. 
>    Fixes: d9213b829a31 ("sched: remove pipe params config from port level")

OK, noted, thank you.

> Patch looks good to me.

Great.

As s side note, it would be nice if this could be backported to 19.11 LTS.

Regards,

H.

>
> Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>
>
>
Thomas Monjalon June 24, 2020, 10:47 p.m. UTC | #4
30/05/2020 10:05, Hrvoje Habjanic:
> On 27. 05. 2020. 13:48, Singh, Jasvinder wrote:
> >>
> >> In function rte_sched_subport_free (lib/librte_sched/rte_sched.c, line 865),
> >> there is code to free all allocated stuff related to scheduler subport. First
> >> there are some checks, and in the end, rte_bitmap_free is called.
> >>
> >> Now, rte_bitmap_free is a dummy function, and it just checks if provided
> >> pointer to bitmap is valid or not. So, actual memory for subport is not freed.
> >>
> >> This patch fixes this by removing call to rte_bitmap_free, and instead calling
> >> rte_free.
> >>
> >> Signed-off-by: Hrvoje Habjanic <hrvoje.habjanic@zg.ht.hr>
> >> ---
> > Hi Hrvoje;
> >
> > I guess this is your first patch to dpdk.org, here are some suggestions when you send bug fixes;
> 
> Yes, it is.
> 
> >
> > - When sending fixes, please use "fix" word in the subject line, e.g- rte_sched: fix subport memory leak
> > - The commit message should include commit id corresponding to the line that you fixes as shown below for this case. 
> >    Fixes: d9213b829a31 ("sched: remove pipe params config from port level")
> 
> OK, noted, thank you.
> 
> > Patch looks good to me.
> 
> Great.
> 
> As s side note, it would be nice if this could be backported to 19.11 LTS.

Added Cc: stable@dpdk.org

> > Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>

Applied, thanks

Patch
diff mbox series

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index c0983ddda..f15a3b515 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -888,7 +888,7 @@  rte_sched_subport_free(struct rte_sched_port *port,
 		}
 	}
 
-	rte_bitmap_free(subport->bmp);
+	rte_free(subport);
 }
 
 void