[dpdk-dev] [PATCH] mem: fix freeing of memzone used by ivshmem

[dpdk-dev] [PATCH] mem: fix freeing of memzone used by ivshmem

[Mauricio Vasquez B]

although previous implementation returned an error when trying to release a
memzone assigned to an ivshmem device, it stills freed it.

Fixes: cd10c42eb5bc ("mem: fix ivshmem freeing")

Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
---
 lib/librte_eal/common/eal_common_memzone.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
index 711c845..1fce906 100644
--- a/lib/librte_eal/common/eal_common_memzone.c
+++ b/lib/librte_eal/common/eal_common_memzone.c
@@ -321,15 +321,19 @@ rte_memzone_free(const struct rte_memzone *mz)
 	idx = ((uintptr_t)mz - (uintptr_t)mcfg->memzone);
 	idx = idx / sizeof(struct rte_memzone);
 
-	addr = mcfg->memzone[idx].addr;
 #ifdef RTE_LIBRTE_IVSHMEM
 	/*
 	 * If ioremap_addr is set, it's an IVSHMEM memzone and we cannot
 	 * free it.
 	 */
-	if (mcfg->memzone[idx].ioremap_addr != 0)
+	if (mcfg->memzone[idx].ioremap_addr != 0) {
 		ret = -EINVAL;
+		goto error;
+	}
 #endif
+
+	addr = mcfg->memzone[idx].addr;
+
 	if (addr == NULL)
 		ret = -EINVAL;
 	else if (mcfg->memzone_cnt == 0) {
@@ -345,6 +349,10 @@ rte_memzone_free(const struct rte_memzone *mz)
 	rte_free(addr);
 
 	return ret;
+
+error:
+	rte_rwlock_write_unlock(&mcfg->mlock);
+	return ret;
 }
 
 /*
-- 
1.9.1

Re: [dpdk-dev] [PATCH] mem: fix freeing of memzone used by ivshmem

[Sergio Gonzalez Monroy]

On 14/04/2016 14:48, Mauricio Vasquez B wrote:
> although previous implementation returned an error when trying to release a
> memzone assigned to an ivshmem device, it stills freed it.
>
> Fixes: cd10c42eb5bc ("mem: fix ivshmem freeing")
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
> ---
>   lib/librte_eal/common/eal_common_memzone.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
>

Thanks for the fix (not sure what I was thinking at the time).

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Re: [dpdk-dev] [PATCH] mem: fix freeing of memzone used by ivshmem

[Mauricio Vásquez]

This patch does not compile when ivshmem is disabled:

compile error:
  CC malloc_heap.o

/home/patchWorkOrg/compilation/lib/librte_eal/common/eal_common_memzone.c(353):
error #177: label "error" was declared but never referenced
    error:
  ^

I'll send a v2 solving this issue


On Thu, Apr 14, 2016 at 4:48 PM, Sergio Gonzalez Monroy <
sergio.gonzalez.monroy@intel.com> wrote:

> On 14/04/2016 14:48, Mauricio Vasquez B wrote:
>
>> although previous implementation returned an error when trying to release
>> a
>> memzone assigned to an ivshmem device, it stills freed it.
>>
>> Fixes: cd10c42eb5bc ("mem: fix ivshmem freeing")
>>
>> Signed-off-by: Mauricio Vasquez B <
>> mauricio.vasquezbernal@studenti.polito.it>
>> ---
>>   lib/librte_eal/common/eal_common_memzone.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>>
>>
> Thanks for the fix (not sure what I was thinking at the time).
>
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>
>

[dpdk-dev] [PATCH v2] mem: fix freeing of memzone used by ivshmem

[Mauricio Vasquez B]

although previous implementation returned an error when trying to release a
memzone assigned to an ivshmem device, it stills freed it.

Fixes: cd10c42eb5bc ("mem: fix ivshmem freeing")

Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
---
v2: 
solved compilation problem when ivshmem is disabled
 lib/librte_eal/common/eal_common_memzone.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
index 711c845..a8f804c 100644
--- a/lib/librte_eal/common/eal_common_memzone.c
+++ b/lib/librte_eal/common/eal_common_memzone.c
@@ -321,15 +321,19 @@ rte_memzone_free(const struct rte_memzone *mz)
 	idx = ((uintptr_t)mz - (uintptr_t)mcfg->memzone);
 	idx = idx / sizeof(struct rte_memzone);
 
-	addr = mcfg->memzone[idx].addr;
 #ifdef RTE_LIBRTE_IVSHMEM
 	/*
 	 * If ioremap_addr is set, it's an IVSHMEM memzone and we cannot
 	 * free it.
 	 */
-	if (mcfg->memzone[idx].ioremap_addr != 0)
-		ret = -EINVAL;
+	if (mcfg->memzone[idx].ioremap_addr != 0) {
+		rte_rwlock_write_unlock(&mcfg->mlock);
+		return -EINVAL;
+	}
 #endif
+
+	addr = mcfg->memzone[idx].addr;
+
 	if (addr == NULL)
 		ret = -EINVAL;
 	else if (mcfg->memzone_cnt == 0) {
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2] mem: fix freeing of memzone used by ivshmem

[Sergio Gonzalez Monroy]

On 15/04/2016 09:29, Mauricio Vasquez B wrote:
> although previous implementation returned an error when trying to release a
> memzone assigned to an ivshmem device, it stills freed it.
>
> Fixes: cd10c42eb5bc ("mem: fix ivshmem freeing")
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
> ---
> v2:
> solved compilation problem when ivshmem is disabled
>   lib/librte_eal/common/eal_common_memzone.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)

This time I have waited to see the test-report (which I should have done 
for the v1).

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Re: [dpdk-dev] [PATCH v2] mem: fix freeing of memzone used by ivshmem

[Thomas Monjalon]

2016-04-21 12:21, Sergio Gonzalez Monroy:
> On 15/04/2016 09:29, Mauricio Vasquez B wrote:
> > although previous implementation returned an error when trying to release a
> > memzone assigned to an ivshmem device, it stills freed it.
> >
> > Fixes: cd10c42eb5bc ("mem: fix ivshmem freeing")
> >
> > Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>
> > ---
> > v2:
> > solved compilation problem when ivshmem is disabled
> >   lib/librte_eal/common/eal_common_memzone.c | 10 +++++++---
> >   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> This time I have waited to see the test-report (which I should have done 
> for the v1).
> 
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Applied, thanks