Re: [PATCH] ring: add function to free a ring
From: Olivier MATZ <hidden>
Date: 2015-09-07 08:45:22
Hi Pablo, Please find some comments below. On 08/18/2015 04:00 PM, Pablo de Lara wrote:
quoted hunk ↗ jump to hunk
When creating a ring, a memzone is created to allocate it in memory, but the ring could not be freed, as memzones could not be. Since memzones can be freed now, then rings can be as well, taking into account if they were initialized using pre-allocated memory (in which case, memory should be freed externally) or using rte_memzone_reserve (with rte_ring_create), freeing the memory with rte_memzone_free. Signed-off-by: Pablo de Lara <redacted> --- lib/librte_ring/rte_ring.c | 43 ++++++++++++++++++++++++++++++++++++ lib/librte_ring/rte_ring.h | 7 ++++++ lib/librte_ring/rte_ring_version.map | 7 ++++++ 3 files changed, 57 insertions(+)diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index c9e59d4..83ce6d3 100644 --- a/lib/librte_ring/rte_ring.c +++ b/lib/librte_ring/rte_ring.c@@ -208,6 +208,49 @@ rte_ring_create(const char *name, unsigned count, int socket_id, return r; } +/* free the ring */ +void +rte_ring_free(struct rte_ring *r) +{ + struct rte_ring_list *ring_list = NULL; + char mz_name[RTE_MEMZONE_NAMESIZE]; + struct rte_tailq_entry *te; + const struct rte_memzone *mz; + + if (r == NULL) + return; + + snprintf(mz_name, sizeof(mz_name), "%s%s", RTE_RING_MZ_PREFIX, r->name); + mz = rte_memzone_lookup(mz_name); + + /* + * Free ring memory if it was allocated with rte_memzone_reserve, + * otherwise it should be freed externally + */ + if (rte_memzone_free(mz) != 0) + return;
Should we have a log here? I think it may hide important bugs if we just return silently here.
+
+ ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+ rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
+
+ /* find out tailq entry */
+ TAILQ_FOREACH(te, ring_list, next) {
+ if (te->data == (void *) r)
+ break;
+ }
+
+ if (te == NULL) {
+ rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
+ return;
+ }If I understand well, a ring is in the tailq only if it was created with rte_ring_create(). A ring that is statically created in memory is not in the tailq. But we already returned in that case. So (te == NULL) should not happen here, right? We could also add an error log then. I'm not sure we should handle the case where the ring is not allocated with rte_ring_create() in this function. If the ring is allocated with another mean (either in a global variable, or with another dynamic memory allocator), this function should not be called. What do you think? Regards, Olivier
quoted hunk ↗ jump to hunk
+ + TAILQ_REMOVE(ring_list, te, next); + + rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); + + rte_free(te); +} + /* * change the high water mark. If *count* is 0, water marking is * disableddiff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index af68888..e75566f 100644 --- a/lib/librte_ring/rte_ring.h +++ b/lib/librte_ring/rte_ring.h@@ -300,6 +300,13 @@ int rte_ring_init(struct rte_ring *r, const char *name, unsigned count, */ struct rte_ring *rte_ring_create(const char *name, unsigned count, int socket_id, unsigned flags); +/** + * De-allocate all memory used by the ring. + * + * @param r + * Ring to free + */ +void rte_ring_free(struct rte_ring *r); /** * Change the high water mark.diff --git a/lib/librte_ring/rte_ring_version.map b/lib/librte_ring/rte_ring_version.map index 982fdd1..5474b98 100644 --- a/lib/librte_ring/rte_ring_version.map +++ b/lib/librte_ring/rte_ring_version.map@@ -11,3 +11,10 @@ DPDK_2.0 { local: *; }; + +DPDK_2.2 { + global: + + rte_ring_free; + +} DPDK_2.0;