diff mbox

linux-generic: pktio: ring: add _ring_destroy()

Message ID 1464339211-3770-1-git-send-email-yi.he@linaro.org
State Superseded
Headers show

Commit Message

Yi He May 27, 2016, 8:53 a.m. UTC
Add _ring_destroy() to release shared memory allocated by
_ring_create() calls. previously this was accomplished by
manually odp_shm_lookup() and odp_shm_free().

Signed-off-by: Yi He <yi.he@linaro.org>
---
 .../include/odp_packet_io_ring_internal.h          |  8 +++++
 platform/linux-generic/pktio/ipc.c                 | 34 +++++++++++++---------
 platform/linux-generic/pktio/ring.c                | 16 ++++++++++
 platform/linux-generic/test/ring/ring_basic.c      |  3 ++
 platform/linux-generic/test/ring/ring_stress.c     |  2 ++
 5 files changed, 49 insertions(+), 14 deletions(-)

Comments

Maxim Uvarov May 27, 2016, 3:44 p.m. UTC | #1
On 05/27/16 11:53, Yi He wrote:
> Add _ring_destroy() to release shared memory allocated by
> _ring_create() calls. previously this was accomplished by
> manually odp_shm_lookup() and odp_shm_free().
>
> Signed-off-by: Yi He <yi.he@linaro.org>
> ---
>   .../include/odp_packet_io_ring_internal.h          |  8 +++++
>   platform/linux-generic/pktio/ipc.c                 | 34 +++++++++++++---------
>   platform/linux-generic/pktio/ring.c                | 16 ++++++++++
>   platform/linux-generic/test/ring/ring_basic.c      |  3 ++
>   platform/linux-generic/test/ring/ring_stress.c     |  2 ++
>   5 files changed, 49 insertions(+), 14 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_packet_io_ring_internal.h b/platform/linux-generic/include/odp_packet_io_ring_internal.h
> index 413787a..a599a26 100644
> --- a/platform/linux-generic/include/odp_packet_io_ring_internal.h
> +++ b/platform/linux-generic/include/odp_packet_io_ring_internal.h
> @@ -201,6 +201,14 @@ _ring_t *_ring_create(const char *name, unsigned count,
>   		      unsigned flags);
>   
>   /**
> + * Destroy the ring created with *name*.
> + *
> + * @param name  name of the ring to be destroyed.
> + * @return      0 on success and negative value on error.
> + */
> +int _ring_destroy(const char *name);
There is already:

_ring_t *_ring_lookup(const char *name);

so I think we just need

int _ring_destroy(_rint_t r);

Maxim.
> +
> +/**
>    * Change the high water mark.
>    *
>    * If *count* is 0, water marking is disabled. Otherwise, it is set to the
> diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c
> index b8a075c..23e89c4 100644
> --- a/platform/linux-generic/pktio/ipc.c
> +++ b/platform/linux-generic/pktio/ipc.c
> @@ -141,7 +141,6 @@ static int _ipc_init_master(pktio_entry_t *pktio_entry,
>   	uint32_t pool_id;
>   	struct pktio_info *pinfo;
>   	const char *pool_name;
> -	odp_shm_t shm;
>   
>   	pool_id = pool_handle_to_index(pool);
>   	pool_entry    = get_pool_entry(pool_id);
> @@ -230,16 +229,13 @@ static int _ipc_init_master(pktio_entry_t *pktio_entry,
>   
>   free_s_prod:
>   	snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod", dev);
> -	shm = odp_shm_lookup(ipc_shm_name);
> -	odp_shm_free(shm);
> +	_ring_destroy(ipc_shm_name);
>   free_m_cons:
>   	snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons", dev);
> -	shm = odp_shm_lookup(ipc_shm_name);
> -	odp_shm_free(shm);
> +	_ring_destroy(ipc_shm_name);
>   free_m_prod:
>   	snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod", dev);
> -	shm = odp_shm_lookup(ipc_shm_name);
> -	odp_shm_free(shm);
> +	_ring_destroy(ipc_shm_name);
>   	return -1;
>   }
>   
> @@ -709,16 +705,26 @@ static int ipc_stop(pktio_entry_t *pktio_entry)
>   
>   static int ipc_close(pktio_entry_t *pktio_entry)
>   {
> -	ipc_stop(pktio_entry);
> +	odp_shm_t shm;
> +	char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
> +	char *dev = pktio_entry->s.name;
>   
> -	if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
> -		char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
> -		char *dev = pktio_entry->s.name;
> -		odp_shm_t shm;
> +	ipc_stop(pktio_entry);
>   
> -		/* unlink this pktio info */
> -		odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
> +	/* unlink this pktio info for both master and slave */
> +	odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
>   
> +	if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
> +		/* destroy rings */
> +		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons", dev);
> +		_ring_destroy(ipc_shm_name);
> +		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod", dev);
> +		_ring_destroy(ipc_shm_name);
> +		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons", dev);
> +		_ring_destroy(ipc_shm_name);
> +		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod", dev);
> +		_ring_destroy(ipc_shm_name);
> +	} else {
>   		/* unlink rings */
>   		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons", dev);
>   		shm = odp_shm_lookup(ipc_shm_name);
> diff --git a/platform/linux-generic/pktio/ring.c b/platform/linux-generic/pktio/ring.c
> index d2f16a5..af24779 100644
> --- a/platform/linux-generic/pktio/ring.c
> +++ b/platform/linux-generic/pktio/ring.c
> @@ -209,6 +209,22 @@ _ring_create(const char *name, unsigned count, unsigned flags)
>   	return r;
>   }
>   
> +int _ring_destroy(const char *name)
> +{
> +	odp_shm_t shm = odp_shm_lookup(name);
> +
> +	if (shm != ODP_SHM_INVALID) {
> +		_ring_t *r = odp_shm_addr(shm);
> +
> +		odp_rwlock_write_lock(&qlock);
> +		TAILQ_REMOVE(&odp_ring_list, r, next);
> +		odp_rwlock_write_unlock(&qlock);
> +
> +		return odp_shm_free(shm);
> +	}
> +	return 0;
> +}
> +
>   /*
>    * change the high water mark. If *count* is 0, water marking is
>    * disabled
> diff --git a/platform/linux-generic/test/ring/ring_basic.c b/platform/linux-generic/test/ring/ring_basic.c
> index 19e24f1..926dc46 100644
> --- a/platform/linux-generic/test/ring/ring_basic.c
> +++ b/platform/linux-generic/test/ring/ring_basic.c
> @@ -66,6 +66,9 @@ int ring_test_basic_start(void)
>   
>   int ring_test_basic_end(void)
>   {
> +	_ring_destroy(st_ring_name);
> +	_ring_destroy(mt_ring_name);
> +
>   	free(test_enq_data);
>   	free(test_deq_data);
>   	return 0;
> diff --git a/platform/linux-generic/test/ring/ring_stress.c b/platform/linux-generic/test/ring/ring_stress.c
> index c68419f..af7d23d 100644
> --- a/platform/linux-generic/test/ring/ring_stress.c
> +++ b/platform/linux-generic/test/ring/ring_stress.c
> @@ -84,6 +84,8 @@ int ring_test_stress_end(void)
>   {
>   	odp_shm_t shared;
>   
> +	_ring_destroy(ring_name);
> +
>   	/* release consume atomic count */
>   	shared = odp_shm_lookup(consume_count_name);
>   	if (shared != ODP_SHM_INVALID)
Maxim Uvarov May 27, 2016, 4:03 p.m. UTC | #2
On 05/27/16 18:44, Maxim Uvarov wrote:
> On 05/27/16 11:53, Yi He wrote:
>> Add _ring_destroy() to release shared memory allocated by
>> _ring_create() calls. previously this was accomplished by
>> manually odp_shm_lookup() and odp_shm_free().
>>
>> Signed-off-by: Yi He <yi.he@linaro.org>
>> ---
>>   .../include/odp_packet_io_ring_internal.h          |  8 +++++
>>   platform/linux-generic/pktio/ipc.c                 | 34 
>> +++++++++++++---------
>>   platform/linux-generic/pktio/ring.c                | 16 ++++++++++
>>   platform/linux-generic/test/ring/ring_basic.c      |  3 ++
>>   platform/linux-generic/test/ring/ring_stress.c     |  2 ++
>>   5 files changed, 49 insertions(+), 14 deletions(-)
>>
>> diff --git 
>> a/platform/linux-generic/include/odp_packet_io_ring_internal.h 
>> b/platform/linux-generic/include/odp_packet_io_ring_internal.h
>> index 413787a..a599a26 100644
>> --- a/platform/linux-generic/include/odp_packet_io_ring_internal.h
>> +++ b/platform/linux-generic/include/odp_packet_io_ring_internal.h
>> @@ -201,6 +201,14 @@ _ring_t *_ring_create(const char *name, unsigned 
>> count,
>>                 unsigned flags);
>>     /**
>> + * Destroy the ring created with *name*.
>> + *
>> + * @param name  name of the ring to be destroyed.
>> + * @return      0 on success and negative value on error.
>> + */
>> +int _ring_destroy(const char *name);
> There is already:
>
> _ring_t *_ring_lookup(const char *name);
>
> so I think we just need
>
> int _ring_destroy(_rint_t r);
>
> Maxim.

After a little bit thinking I se ethat _ring_lookup() does not look to 
shared memory.
So you fix looks good for me.

Maxim.


>> +
>> +/**
>>    * Change the high water mark.
>>    *
>>    * If *count* is 0, water marking is disabled. Otherwise, it is set 
>> to the
>> diff --git a/platform/linux-generic/pktio/ipc.c 
>> b/platform/linux-generic/pktio/ipc.c
>> index b8a075c..23e89c4 100644
>> --- a/platform/linux-generic/pktio/ipc.c
>> +++ b/platform/linux-generic/pktio/ipc.c
>> @@ -141,7 +141,6 @@ static int _ipc_init_master(pktio_entry_t 
>> *pktio_entry,
>>       uint32_t pool_id;
>>       struct pktio_info *pinfo;
>>       const char *pool_name;
>> -    odp_shm_t shm;
>>         pool_id = pool_handle_to_index(pool);
>>       pool_entry    = get_pool_entry(pool_id);
>> @@ -230,16 +229,13 @@ static int _ipc_init_master(pktio_entry_t 
>> *pktio_entry,
>>     free_s_prod:
>>       snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod", dev);
>> -    shm = odp_shm_lookup(ipc_shm_name);
>> -    odp_shm_free(shm);
>> +    _ring_destroy(ipc_shm_name);
>>   free_m_cons:
>>       snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons", dev);
>> -    shm = odp_shm_lookup(ipc_shm_name);
>> -    odp_shm_free(shm);
>> +    _ring_destroy(ipc_shm_name);
>>   free_m_prod:
>>       snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod", dev);
>> -    shm = odp_shm_lookup(ipc_shm_name);
>> -    odp_shm_free(shm);
>> +    _ring_destroy(ipc_shm_name);
>>       return -1;
>>   }
>>   @@ -709,16 +705,26 @@ static int ipc_stop(pktio_entry_t *pktio_entry)
>>     static int ipc_close(pktio_entry_t *pktio_entry)
>>   {
>> -    ipc_stop(pktio_entry);
>> +    odp_shm_t shm;
>> +    char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
>> +    char *dev = pktio_entry->s.name;
>>   -    if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
>> -        char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
>> -        char *dev = pktio_entry->s.name;
>> -        odp_shm_t shm;
>> +    ipc_stop(pktio_entry);
>>   -        /* unlink this pktio info */
>> -        odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
>> +    /* unlink this pktio info for both master and slave */
>> +    odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
>>   +    if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
>> +        /* destroy rings */
>> +        snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons", dev);
>> +        _ring_destroy(ipc_shm_name);
>> +        snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod", dev);
>> +        _ring_destroy(ipc_shm_name);
>> +        snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons", dev);
>> +        _ring_destroy(ipc_shm_name);
>> +        snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod", dev);
>> +        _ring_destroy(ipc_shm_name);
>> +    } else {
>>           /* unlink rings */
>>           snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons", 
>> dev);
>>           shm = odp_shm_lookup(ipc_shm_name);
>> diff --git a/platform/linux-generic/pktio/ring.c 
>> b/platform/linux-generic/pktio/ring.c
>> index d2f16a5..af24779 100644
>> --- a/platform/linux-generic/pktio/ring.c
>> +++ b/platform/linux-generic/pktio/ring.c
>> @@ -209,6 +209,22 @@ _ring_create(const char *name, unsigned count, 
>> unsigned flags)
>>       return r;
>>   }
>>   +int _ring_destroy(const char *name)
>> +{
>> +    odp_shm_t shm = odp_shm_lookup(name);
>> +
>> +    if (shm != ODP_SHM_INVALID) {
>> +        _ring_t *r = odp_shm_addr(shm);
>> +
>> +        odp_rwlock_write_lock(&qlock);
>> +        TAILQ_REMOVE(&odp_ring_list, r, next);
>> +        odp_rwlock_write_unlock(&qlock);
>> +
>> +        return odp_shm_free(shm);
>> +    }
>> +    return 0;
>> +}
>> +
>>   /*
>>    * change the high water mark. If *count* is 0, water marking is
>>    * disabled
>> diff --git a/platform/linux-generic/test/ring/ring_basic.c 
>> b/platform/linux-generic/test/ring/ring_basic.c
>> index 19e24f1..926dc46 100644
>> --- a/platform/linux-generic/test/ring/ring_basic.c
>> +++ b/platform/linux-generic/test/ring/ring_basic.c
>> @@ -66,6 +66,9 @@ int ring_test_basic_start(void)
>>     int ring_test_basic_end(void)
>>   {
>> +    _ring_destroy(st_ring_name);
>> +    _ring_destroy(mt_ring_name);
>> +
>>       free(test_enq_data);
>>       free(test_deq_data);
>>       return 0;
>> diff --git a/platform/linux-generic/test/ring/ring_stress.c 
>> b/platform/linux-generic/test/ring/ring_stress.c
>> index c68419f..af7d23d 100644
>> --- a/platform/linux-generic/test/ring/ring_stress.c
>> +++ b/platform/linux-generic/test/ring/ring_stress.c
>> @@ -84,6 +84,8 @@ int ring_test_stress_end(void)
>>   {
>>       odp_shm_t shared;
>>   +    _ring_destroy(ring_name);
>> +
>>       /* release consume atomic count */
>>       shared = odp_shm_lookup(consume_count_name);
>>       if (shared != ODP_SHM_INVALID)
>
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp_packet_io_ring_internal.h b/platform/linux-generic/include/odp_packet_io_ring_internal.h
index 413787a..a599a26 100644
--- a/platform/linux-generic/include/odp_packet_io_ring_internal.h
+++ b/platform/linux-generic/include/odp_packet_io_ring_internal.h
@@ -201,6 +201,14 @@  _ring_t *_ring_create(const char *name, unsigned count,
 		      unsigned flags);
 
 /**
+ * Destroy the ring created with *name*.
+ *
+ * @param name  name of the ring to be destroyed.
+ * @return      0 on success and negative value on error.
+ */
+int _ring_destroy(const char *name);
+
+/**
  * Change the high water mark.
  *
  * If *count* is 0, water marking is disabled. Otherwise, it is set to the
diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c
index b8a075c..23e89c4 100644
--- a/platform/linux-generic/pktio/ipc.c
+++ b/platform/linux-generic/pktio/ipc.c
@@ -141,7 +141,6 @@  static int _ipc_init_master(pktio_entry_t *pktio_entry,
 	uint32_t pool_id;
 	struct pktio_info *pinfo;
 	const char *pool_name;
-	odp_shm_t shm;
 
 	pool_id = pool_handle_to_index(pool);
 	pool_entry    = get_pool_entry(pool_id);
@@ -230,16 +229,13 @@  static int _ipc_init_master(pktio_entry_t *pktio_entry,
 
 free_s_prod:
 	snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod", dev);
-	shm = odp_shm_lookup(ipc_shm_name);
-	odp_shm_free(shm);
+	_ring_destroy(ipc_shm_name);
 free_m_cons:
 	snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons", dev);
-	shm = odp_shm_lookup(ipc_shm_name);
-	odp_shm_free(shm);
+	_ring_destroy(ipc_shm_name);
 free_m_prod:
 	snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod", dev);
-	shm = odp_shm_lookup(ipc_shm_name);
-	odp_shm_free(shm);
+	_ring_destroy(ipc_shm_name);
 	return -1;
 }
 
@@ -709,16 +705,26 @@  static int ipc_stop(pktio_entry_t *pktio_entry)
 
 static int ipc_close(pktio_entry_t *pktio_entry)
 {
-	ipc_stop(pktio_entry);
+	odp_shm_t shm;
+	char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
+	char *dev = pktio_entry->s.name;
 
-	if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
-		char ipc_shm_name[ODP_POOL_NAME_LEN + sizeof("_m_prod")];
-		char *dev = pktio_entry->s.name;
-		odp_shm_t shm;
+	ipc_stop(pktio_entry);
 
-		/* unlink this pktio info */
-		odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
+	/* unlink this pktio info for both master and slave */
+	odp_shm_free(pktio_entry->s.ipc.pinfo_shm);
 
+	if (pktio_entry->s.ipc.type == PKTIO_TYPE_IPC_MASTER) {
+		/* destroy rings */
+		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons", dev);
+		_ring_destroy(ipc_shm_name);
+		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_prod", dev);
+		_ring_destroy(ipc_shm_name);
+		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_cons", dev);
+		_ring_destroy(ipc_shm_name);
+		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_m_prod", dev);
+		_ring_destroy(ipc_shm_name);
+	} else {
 		/* unlink rings */
 		snprintf(ipc_shm_name, sizeof(ipc_shm_name), "%s_s_cons", dev);
 		shm = odp_shm_lookup(ipc_shm_name);
diff --git a/platform/linux-generic/pktio/ring.c b/platform/linux-generic/pktio/ring.c
index d2f16a5..af24779 100644
--- a/platform/linux-generic/pktio/ring.c
+++ b/platform/linux-generic/pktio/ring.c
@@ -209,6 +209,22 @@  _ring_create(const char *name, unsigned count, unsigned flags)
 	return r;
 }
 
+int _ring_destroy(const char *name)
+{
+	odp_shm_t shm = odp_shm_lookup(name);
+
+	if (shm != ODP_SHM_INVALID) {
+		_ring_t *r = odp_shm_addr(shm);
+
+		odp_rwlock_write_lock(&qlock);
+		TAILQ_REMOVE(&odp_ring_list, r, next);
+		odp_rwlock_write_unlock(&qlock);
+
+		return odp_shm_free(shm);
+	}
+	return 0;
+}
+
 /*
  * change the high water mark. If *count* is 0, water marking is
  * disabled
diff --git a/platform/linux-generic/test/ring/ring_basic.c b/platform/linux-generic/test/ring/ring_basic.c
index 19e24f1..926dc46 100644
--- a/platform/linux-generic/test/ring/ring_basic.c
+++ b/platform/linux-generic/test/ring/ring_basic.c
@@ -66,6 +66,9 @@  int ring_test_basic_start(void)
 
 int ring_test_basic_end(void)
 {
+	_ring_destroy(st_ring_name);
+	_ring_destroy(mt_ring_name);
+
 	free(test_enq_data);
 	free(test_deq_data);
 	return 0;
diff --git a/platform/linux-generic/test/ring/ring_stress.c b/platform/linux-generic/test/ring/ring_stress.c
index c68419f..af7d23d 100644
--- a/platform/linux-generic/test/ring/ring_stress.c
+++ b/platform/linux-generic/test/ring/ring_stress.c
@@ -84,6 +84,8 @@  int ring_test_stress_end(void)
 {
 	odp_shm_t shared;
 
+	_ring_destroy(ring_name);
+
 	/* release consume atomic count */
 	shared = odp_shm_lookup(consume_count_name);
 	if (shared != ODP_SHM_INVALID)