[dpdk-dev] [PATCH] ring: add function to free a ring

[dpdk-dev] [PATCH] ring: add function to free a ring

[Pablo de Lara]

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 <pablo.de.lara.guarch@intel.com>
---
 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;
+
+	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;
+	}
+
+	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
  * disabled
diff --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;
-- 
2.4.2

Re: [dpdk-dev] [PATCH] ring: add function to free a ring

[Olivier MATZ]

Hi Pablo,

Please find some comments below.

On 08/18/2015 04:00 PM, Pablo de Lara wrote:
> 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 <pablo.de.lara.guarch@intel.com>
> ---
>  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


> +
> +	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
>   * disabled
> diff --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;
> 

Re: [dpdk-dev] [PATCH] ring: add function to free a ring

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Monday, September 07, 2015 9:45 AM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ring: add function to free a ring
> 
> Hi Pablo,
> 
> Please find some comments below.
> 
> On 08/18/2015 04:00 PM, Pablo de Lara wrote:
> > 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 <pablo.de.lara.guarch@intel.com>
> > ---
> >  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.

Agree.

> 
> > +
> > +	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.

Yes, (te == NULL) should not happen, but not sure if there is a way
where it can happen, but looking at other libraries, like rte_hash or rte_acl,
we check for this and don't add any error log.

> 
> 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.

You are right that that rte_ring_free should not be called
if the ring was not created with rte_ring_create,
but we should have a way to handle it, just in case, right?

> 
> What do you think?

Thanks for the comments!

Pablo
> 
> 
> Regards,
> Olivier
> 
> 
> > +
> > +	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
> >   * disabled
> > diff --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;
> >

[dpdk-dev] [PATCH v2] ring: add function to free a ring

[Pablo de Lara]

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 <pablo.de.lara.guarch@intel.com>
---
Changes in v2:
- Include note in release notes
- Add error log when ring cannot be freed

 doc/guides/rel_notes/release_2_2.rst |  4 ++++
 lib/librte_ring/rte_ring.c           | 45 ++++++++++++++++++++++++++++++++++++
 lib/librte_ring/rte_ring.h           |  7 ++++++
 lib/librte_ring/rte_ring_version.map |  7 ++++++
 4 files changed, 63 insertions(+)

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index 682f468..f6501a3 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -4,6 +4,10 @@ DPDK Release 2.2
 New Features
 ------------
 
+* **Enabled freeing of rte_ring.**
+
+  New function rte_ring_free() allows the user to free a ring
+  if it was created with rte_ring_create().
 
 Resolved Issues
 ---------------
diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index c9e59d4..2bcfd89 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -208,6 +208,51 @@ 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) {
+		RTE_LOG(ERR, RING, "Cannot free memory\n");
+		return;
+	}
+
+	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;
+	}
+
+	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
  * disabled
diff --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;
-- 
2.4.3

[dpdk-dev] [PATCH v3] ring: add function to free a ring

[Pablo de Lara]

From: "Pablo de Lara" <pablo.de.lara.guarch@intel.com>

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 <pablo.de.lara.guarch@intel.com>
---
Changes in v3:
 - Simplify patch using stored memzone address in ring structure
 - Change copyright date

Changes in v2:
 - Include note in release notes
 - Add error log when ring cannot be freed

This patch depends on patch "rte_ring: store memzone pointer inside ring"

 doc/guides/rel_notes/release_2_2.rst |  4 +++
 lib/librte_ring/rte_ring.c           | 47 +++++++++++++++++++++++++++++++++++-
 lib/librte_ring/rte_ring.h           |  7 ++++++
 lib/librte_ring/rte_ring_version.map |  7 ++++++
 4 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index 5687676..24937ac 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -4,6 +4,10 @@ DPDK Release 2.2
 New Features
 ------------
 
+* **Enabled freeing of rte_ring.**
+
+  New function rte_ring_free() allows the user to free a ring
+  if it was created with rte_ring_create().
 
 Resolved Issues
 ---------------
diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index 4e78e14..d80faf3 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -209,6 +209,51 @@ 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;
+	struct rte_tailq_entry *te;
+
+	if (r == NULL)
+		return;
+
+	/*
+	 * Ring was not created with rte_ring_create,
+	 * therefore, there is no memzone to free.
+	 */
+	if (r->memzone == NULL) {
+		RTE_LOG(ERR, RING, "Cannot free ring (not created with rte_ring_create()");
+		return;
+	}
+
+	if (rte_memzone_free(r->memzone) != 0) {
+		RTE_LOG(ERR, RING, "Cannot free memory\n");
+		return;
+	}
+
+	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;
+	}
+
+	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
  * disabled
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index df45f3f..fb5a626 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -304,6 +304,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;
-- 
2.4.3

Re: [dpdk-dev] [PATCH v3] ring: add function to free a ring

[Bruce Richardson]

On Fri, Oct 02, 2015 at 03:01:25PM +0100, Pablo de Lara wrote:
> From: "Pablo de Lara" <pablo.de.lara.guarch@intel.com>
> 
> 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 <pablo.de.lara.guarch@intel.com>
> ---
> Changes in v3:
>  - Simplify patch using stored memzone address in ring structure
>  - Change copyright date

I think you need to call out that this patch depends upon
http://dpdk.org/dev/patchwork/patch/7308/

> 
> Changes in v2:
>  - Include note in release notes
>  - Add error log when ring cannot be freed
> 
> This patch depends on patch "rte_ring: store memzone pointer inside ring"
> 
>  doc/guides/rel_notes/release_2_2.rst |  4 +++
>  lib/librte_ring/rte_ring.c           | 47 +++++++++++++++++++++++++++++++++++-
>  lib/librte_ring/rte_ring.h           |  7 ++++++
>  lib/librte_ring/rte_ring_version.map |  7 ++++++
>  4 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
> index 5687676..24937ac 100644
> --- a/doc/guides/rel_notes/release_2_2.rst
> +++ b/doc/guides/rel_notes/release_2_2.rst
> @@ -4,6 +4,10 @@ DPDK Release 2.2
>  New Features
>  ------------
>  
> +* **Enabled freeing of rte_ring.**
> +
> +  New function rte_ring_free() allows the user to free a ring
> +  if it was created with rte_ring_create().
>  
>  Resolved Issues
>  ---------------
> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
> index 4e78e14..d80faf3 100644
> --- a/lib/librte_ring/rte_ring.c
> +++ b/lib/librte_ring/rte_ring.c
> @@ -1,7 +1,7 @@
>  /*-
>   *   BSD LICENSE
>   *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -209,6 +209,51 @@ 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;
> +	struct rte_tailq_entry *te;
> +
> +	if (r == NULL)
> +		return;
> +
> +	/*
> +	 * Ring was not created with rte_ring_create,
> +	 * therefore, there is no memzone to free.
> +	 */
> +	if (r->memzone == NULL) {
> +		RTE_LOG(ERR, RING, "Cannot free ring (not created with rte_ring_create()");
> +		return;
> +	}
> +
> +	if (rte_memzone_free(r->memzone) != 0) {
> +		RTE_LOG(ERR, RING, "Cannot free memory\n");
> +		return;
> +	}
> +
> +	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;
> +	}
> +
> +	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
>   * disabled
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> index df45f3f..fb5a626 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -304,6 +304,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;
> -- 
> 2.4.3
> 

Re: [dpdk-dev] [PATCH v3] ring: add function to free a ring

[De Lara Guarch, Pablo]

Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, October 02, 2015 3:10 PM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] ring: add function to free a ring
> 
> On Fri, Oct 02, 2015 at 03:01:25PM +0100, Pablo de Lara wrote:
> > From: "Pablo de Lara" <pablo.de.lara.guarch@intel.com>
> >
> > 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 <pablo.de.lara.guarch@intel.com>
> > ---
> > Changes in v3:
> >  - Simplify patch using stored memzone address in ring structure
> >  - Change copyright date
> 
> I think you need to call out that this patch depends upon
> http://dpdk.org/dev/patchwork/patch/7308/

I did below, probably I should have included the patch ID :S

> 
> >
> > Changes in v2:
> >  - Include note in release notes
> >  - Add error log when ring cannot be freed
> >
> > This patch depends on patch "rte_ring: store memzone pointer inside ring"
> >
> >  doc/guides/rel_notes/release_2_2.rst |  4 +++
> >  lib/librte_ring/rte_ring.c           | 47
> +++++++++++++++++++++++++++++++++++-
> >  lib/librte_ring/rte_ring.h           |  7 ++++++
> >  lib/librte_ring/rte_ring_version.map |  7 ++++++
> >  4 files changed, 64 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/rel_notes/release_2_2.rst
> b/doc/guides/rel_notes/release_2_2.rst
> > index 5687676..24937ac 100644
> > --- a/doc/guides/rel_notes/release_2_2.rst
> > +++ b/doc/guides/rel_notes/release_2_2.rst
> > @@ -4,6 +4,10 @@ DPDK Release 2.2
> >  New Features
> >  ------------
> >
> > +* **Enabled freeing of rte_ring.**
> > +
> > +  New function rte_ring_free() allows the user to free a ring
> > +  if it was created with rte_ring_create().
> >
> >  Resolved Issues
> >  ---------------
> > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
> > index 4e78e14..d80faf3 100644
> > --- a/lib/librte_ring/rte_ring.c
> > +++ b/lib/librte_ring/rte_ring.c
> > @@ -1,7 +1,7 @@
> >  /*-
> >   *   BSD LICENSE
> >   *
> > - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> >   *   All rights reserved.
> >   *
> >   *   Redistribution and use in source and binary forms, with or without
> > @@ -209,6 +209,51 @@ 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;
> > +	struct rte_tailq_entry *te;
> > +
> > +	if (r == NULL)
> > +		return;
> > +
> > +	/*
> > +	 * Ring was not created with rte_ring_create,
> > +	 * therefore, there is no memzone to free.
> > +	 */
> > +	if (r->memzone == NULL) {
> > +		RTE_LOG(ERR, RING, "Cannot free ring (not created with
> rte_ring_create()");
> > +		return;
> > +	}
> > +
> > +	if (rte_memzone_free(r->memzone) != 0) {
> > +		RTE_LOG(ERR, RING, "Cannot free memory\n");
> > +		return;
> > +	}
> > +
> > +	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;
> > +	}
> > +
> > +	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
> >   * disabled
> > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> > index df45f3f..fb5a626 100644
> > --- a/lib/librte_ring/rte_ring.h
> > +++ b/lib/librte_ring/rte_ring.h
> > @@ -304,6 +304,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;
> > --
> > 2.4.3
> >

[dpdk-dev] [PATCH v4] ring: add function to free a ring

[Pablo de Lara]

From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>

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 <pablo.de.lara.guarch@intel.com>
---
Changes in v4:
 - Include below missing patch ID which this patch depends on

Changes in v3:
 - Simplify patch using stored memzone address in ring structure
 - Change copyright date

Changes in v2:
 - Include note in release notes
 - Add error log when ring cannot be freed

This patch depends on patch "rte_ring: store memzone pointer inside ring"
(http://dpdk.org/dev/patchwork/patch/7308)

 doc/guides/rel_notes/release_2_2.rst |  4 +++
 lib/librte_ring/rte_ring.c           | 47 +++++++++++++++++++++++++++++++++++-
 lib/librte_ring/rte_ring.h           |  7 ++++++
 lib/librte_ring/rte_ring_version.map |  7 ++++++
 4 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index 5687676..24937ac 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -4,6 +4,10 @@ DPDK Release 2.2
 New Features
 ------------
 
+* **Enabled freeing of rte_ring.**
+
+  New function rte_ring_free() allows the user to free a ring
+  if it was created with rte_ring_create().
 
 Resolved Issues
 ---------------
diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index 4e78e14..d80faf3 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -209,6 +209,51 @@ 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;
+	struct rte_tailq_entry *te;
+
+	if (r == NULL)
+		return;
+
+	/*
+	 * Ring was not created with rte_ring_create,
+	 * therefore, there is no memzone to free.
+	 */
+	if (r->memzone == NULL) {
+		RTE_LOG(ERR, RING, "Cannot free ring (not created with rte_ring_create()");
+		return;
+	}
+
+	if (rte_memzone_free(r->memzone) != 0) {
+		RTE_LOG(ERR, RING, "Cannot free memory\n");
+		return;
+	}
+
+	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;
+	}
+
+	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
  * disabled
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index df45f3f..fb5a626 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -304,6 +304,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;
-- 
2.4.3

Re: [dpdk-dev] [PATCH v4] ring: add function to free a ring

[Olivier MATZ]

Hi Pablo,

On 10/02/2015 05:53 PM, Pablo de Lara wrote:
> From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
> 
> 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 <pablo.de.lara.guarch@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH v4] ring: add function to free a ring

[Thomas Monjalon]

> > 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 <pablo.de.lara.guarch@intel.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks