[dpdk-dev] i40e: fix the issue of not freeing memzone
Commit Message
This fixes the issue of not freeing memzone in a call to free the
memory for adminq DMA.
Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
doc/guides/rel_notes/release_2_2.rst | 5 +++++
drivers/net/i40e/base/i40e_osdep.h | 2 +-
drivers/net/i40e/i40e_ethdev.c | 12 +++++++-----
3 files changed, 13 insertions(+), 6 deletions(-)
Comments
> - static uint64_t id = 0;
> const struct rte_memzone *mz = NULL;
> char z_name[RTE_MEMZONE_NAMESIZE];
>
> if (!mem)
> return I40E_ERR_PARAM;
>
> - id++;
> - snprintf(z_name, sizeof(z_name), "i40e_dma_%"PRIu64, id);
> + snprintf(z_name, sizeof(z_name), "i40e_dma_%"PRIu64, rte_rand());
Why change id++ to rte_rand() ?
> #ifdef RTE_LIBRTE_XEN_DOM0
> mz = rte_memzone_reserve_bounded(z_name, size,
> SOCKET_ID_ANY, 0,
> alignment, RTE_PGSIZE_2M);
> @@ -2929,7 +2927,6 @@
> i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
> if (!mz)
> return I40E_ERR_NO_MEMORY;
>
> - mem->id = id;
> mem->size = size;
> mem->va = mz->addr;
> #ifdef RTE_LIBRTE_XEN_DOM0
> @@ -2937,6 +2934,8 @@
> i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
> #else
> mem->pa = mz->phys_addr;
> #endif
> + mem->zone = (const void *)mz;
> + PMD_DRV_LOG(DEBUG, "memzone allocated: %p", mem->zone);
>
Why not print the name of mem_zone instead of pointer?
Thanks
Jingjing
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, November 6, 2015 1:40 PM
> To: Zhang, Helin; dev@dpdk.org
> Subject: RE: [PATCH] i40e: fix the issue of not freeing memzone
>
> > - static uint64_t id = 0;
> > const struct rte_memzone *mz = NULL;
> > char z_name[RTE_MEMZONE_NAMESIZE];
> >
> > if (!mem)
> > return I40E_ERR_PARAM;
> >
> > - id++;
> > - snprintf(z_name, sizeof(z_name), "i40e_dma_%"PRIu64, id);
> > + snprintf(z_name, sizeof(z_name), "i40e_dma_%"PRIu64, rte_rand());
>
> Why change id++ to rte_rand() ?
Don't need to maintain the ID, which may have race condition issue.
Get a random data is good enough. Some other PMDs are using tsc count for the similar purpose.
>
> > #ifdef RTE_LIBRTE_XEN_DOM0
> > mz = rte_memzone_reserve_bounded(z_name, size, SOCKET_ID_ANY, 0,
> > alignment, RTE_PGSIZE_2M);
> > @@ -2929,7 +2927,6 @@
> > i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
> > if (!mz)
> > return I40E_ERR_NO_MEMORY;
> >
> > - mem->id = id;
> > mem->size = size;
> > mem->va = mz->addr;
> > #ifdef RTE_LIBRTE_XEN_DOM0
> > @@ -2937,6 +2934,8 @@
> > i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
> > #else
> > mem->pa = mz->phys_addr;
> > #endif
> > + mem->zone = (const void *)mz;
> > + PMD_DRV_LOG(DEBUG, "memzone allocated: %p", mem->zone);
> >
> Why not print the name of mem_zone instead of pointer?
Good idea to print the name instead, and possible physical address, virtual address, etc.
Regards,
Helin
>
>
> Thanks
> Jingjing
@@ -150,6 +150,11 @@ Drivers
Added discarding packets on VSI to the stats and rectify the old statistics.
+* **i40e: Fixed issue of not freeing memzone.**
+
+ Fixed the issue of not freeing memzone in the call to free the memory for
+ adminq DMA.
+
* **vhost: Fixed Qemu shutdown.**
Fixed issue with libvirt ``virsh destroy`` not killing the VM.
@@ -146,7 +146,7 @@ struct i40e_dma_mem {
void *va;
u64 pa;
u32 size;
- u64 id;
+ const void *zone;
} __attribute__((packed));
#define i40e_allocate_dma_mem(h, m, unused, s, a) \
@@ -2910,15 +2910,13 @@ i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
u64 size,
u32 alignment)
{
- static uint64_t id = 0;
const struct rte_memzone *mz = NULL;
char z_name[RTE_MEMZONE_NAMESIZE];
if (!mem)
return I40E_ERR_PARAM;
- id++;
- snprintf(z_name, sizeof(z_name), "i40e_dma_%"PRIu64, id);
+ snprintf(z_name, sizeof(z_name), "i40e_dma_%"PRIu64, rte_rand());
#ifdef RTE_LIBRTE_XEN_DOM0
mz = rte_memzone_reserve_bounded(z_name, size, SOCKET_ID_ANY, 0,
alignment, RTE_PGSIZE_2M);
@@ -2929,7 +2927,6 @@ i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
if (!mz)
return I40E_ERR_NO_MEMORY;
- mem->id = id;
mem->size = size;
mem->va = mz->addr;
#ifdef RTE_LIBRTE_XEN_DOM0
@@ -2937,6 +2934,8 @@ i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
#else
mem->pa = mz->phys_addr;
#endif
+ mem->zone = (const void *)mz;
+ PMD_DRV_LOG(DEBUG, "memzone allocated: %p", mem->zone);
return I40E_SUCCESS;
}
@@ -2950,9 +2949,12 @@ enum i40e_status_code
i40e_free_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
struct i40e_dma_mem *mem)
{
- if (!mem || !mem->va)
+ if (!mem)
return I40E_ERR_PARAM;
+ PMD_DRV_LOG(DEBUG, "memzone to be freed: %p", mem->zone);
+ rte_memzone_free((const struct rte_memzone *)mem->zone);
+ mem->zone = NULL;
mem->va = NULL;
mem->pa = (u64)0;