[dpdk-dev,v7,2/5] mempool: remove rte_ring from rte_mempool struct

Now that we're moving to an external mempoool handler, which
uses a void *pool_data as a pointer to the pool data, remove the
unneeded ring pointer from the mempool struct.

Signed-off-by: David Hunt <david.hunt@intel.com>
---
 app/test/test_mempool_perf.c     | 1 -
 lib/librte_mempool/rte_mempool.h | 1 -
 2 files changed, 2 deletions(-)
  

On 06/02/2016 03:27 PM, David Hunt wrote:
> Now that we're moving to an external mempoool handler, which
> uses a void *pool_data as a pointer to the pool data, remove the
> unneeded ring pointer from the mempool struct.
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>
> ---
>  app/test/test_mempool_perf.c     | 1 -
>  lib/librte_mempool/rte_mempool.h | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/app/test/test_mempool_perf.c b/app/test/test_mempool_perf.c
> index cdc02a0..091c1df 100644
> --- a/app/test/test_mempool_perf.c
> +++ b/app/test/test_mempool_perf.c
> @@ -161,7 +161,6 @@ per_lcore_mempool_test(__attribute__((unused)) void *arg)
>  							   n_get_bulk);
>  				if (unlikely(ret < 0)) {
>  					rte_mempool_dump(stdout, mp);
> -					rte_ring_dump(stdout, mp->ring);
>  					/* in this case, objects are lost... */
>  					return -1;
>  				}
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index a6b28b0..c33eeb8 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -204,7 +204,6 @@ struct rte_mempool_memhdr {
>   */
>  struct rte_mempool {
>  	char name[RTE_MEMPOOL_NAMESIZE]; /**< Name of mempool. */
> -	struct rte_ring *ring;           /**< Ring to store objects. */
>  	union {
>  		void *pool_data;         /**< Ring or pool to store objects */
>  		uint64_t pool_id;        /**< External mempool identifier */
> 

Sorry if I missed it in previous discussions, but I don't really
see the point of having this in a separate commit, as the goal
of the previous commit is to replace the ring by configurable ops.

Moreover, after applying only the previous commit, the
call to rte_ring_dump(stdout, mp->ring) would probably crash
sine ring is NULL.

I think this comment also applies to the next commit. Splitting
between functionalities is good, but in this case I think the 3
commits are linked together, and it should not break compilation
or tests to facilitate the git bisect.


Regards,
Olivier
  

On 6/3/2016 1:28 PM, Olivier MATZ wrote:
>
> On 06/02/2016 03:27 PM, David Hunt wrote:
>> Now that we're moving to an external mempoool handler, which
>> uses a void *pool_data as a pointer to the pool data, remove the
>> unneeded ring pointer from the mempool struct.
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> ---
>>   app/test/test_mempool_perf.c     | 1 -
>>   lib/librte_mempool/rte_mempool.h | 1 -
>>   2 files changed, 2 deletions(-)
>>
>> diff --git a/app/test/test_mempool_perf.c b/app/test/test_mempool_perf.c
>> index cdc02a0..091c1df 100644
>> --- a/app/test/test_mempool_perf.c
>> +++ b/app/test/test_mempool_perf.c
>> @@ -161,7 +161,6 @@ per_lcore_mempool_test(__attribute__((unused)) void *arg)
>>   							   n_get_bulk);
>>   				if (unlikely(ret < 0)) {
>>   					rte_mempool_dump(stdout, mp);
>> -					rte_ring_dump(stdout, mp->ring);
>>   					/* in this case, objects are lost... */
>>   					return -1;
>>   				}
>> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
>> index a6b28b0..c33eeb8 100644
>> --- a/lib/librte_mempool/rte_mempool.h
>> +++ b/lib/librte_mempool/rte_mempool.h
>> @@ -204,7 +204,6 @@ struct rte_mempool_memhdr {
>>    */
>>   struct rte_mempool {
>>   	char name[RTE_MEMPOOL_NAMESIZE]; /**< Name of mempool. */
>> -	struct rte_ring *ring;           /**< Ring to store objects. */
>>   	union {
>>   		void *pool_data;         /**< Ring or pool to store objects */
>>   		uint64_t pool_id;        /**< External mempool identifier */
>>
> Sorry if I missed it in previous discussions, but I don't really
> see the point of having this in a separate commit, as the goal
> of the previous commit is to replace the ring by configurable ops.
>
> Moreover, after applying only the previous commit, the
> call to rte_ring_dump(stdout, mp->ring) would probably crash
> sine ring is NULL.
>
> I think this comment also applies to the next commit. Splitting
> between functionalities is good, but in this case I think the 3
> commits are linked together, and it should not break compilation
> or tests to facilitate the git bisect.
>
>
> Regards,
> Olivier

Yes. Originally there was a lot of discussion to split out the bigger 
patch, which I
did, and it was easier to review, but I think that now we're (very) 
close to final
revision, I can merge those three back into one.

Thanks,
Dave.