[dpdk-dev,RFC,4/6] mempool: add a function to flush default cache

  From: "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>

Mempool get/put API cares about cache itself, but sometimes it is
required to flush the cache explicitly.

Also dedicated API allows to decouple it from block get API (to be
added) and provides more fine-grained control.

Signed-off-by: Artem V. Andreev <Artem.Andreev@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_mempool/rte_mempool.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
  

On Fri, Nov 24, 2017 at 04:06:29PM +0000, Andrew Rybchenko wrote:
> From: "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
> 
> Mempool get/put API cares about cache itself, but sometimes it is
> required to flush the cache explicitly.

I don't disagree, but do you have some use-case in mind?


> Also dedicated API allows to decouple it from block get API (to be
> added) and provides more fine-grained control.
> 
> Signed-off-by: Artem V. Andreev <Artem.Andreev@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  lib/librte_mempool/rte_mempool.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index 9bcb8b7..3a52b93 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -1161,6 +1161,22 @@ rte_mempool_default_cache(struct rte_mempool *mp, unsigned lcore_id)
>  }
>  
>  /**
> + * Ensure that a default per-lcore mempool cache is flushed, if it is present
> + *
> + * @param mp
> + *   A pointer to the mempool structure.
> + */
> +static __rte_always_inline void
> +rte_mempool_ensure_cache_flushed(struct rte_mempool *mp)
> +{
> +	struct rte_mempool_cache *cache;
> +	cache = rte_mempool_default_cache(mp, rte_lcore_id());
> +	if (cache != NULL && cache->len > 0)
> +		rte_mempool_cache_flush(cache, mp);
> +}
> +

We already have rte_mempool_cache_flush().
Why not just extending it instead of adding a new function?

I mean:

    static __rte_always_inline void
    rte_mempool_cache_flush(struct rte_mempool_cache *cache,
    			struct rte_mempool *mp)
    {
   +	if (cache == NULL)
   +		cache = rte_mempool_default_cache(mp, rte_lcore_id());
   +	if (cache == NULL || cache->len == 0)
   +		return;
    	rte_mempool_ops_enqueue_bulk(mp, cache->objs, cache->len);
    	cache->len = 0;
    }
  

On 12/14/2017 04:38 PM, Olivier MATZ wrote:
> On Fri, Nov 24, 2017 at 04:06:29PM +0000, Andrew Rybchenko wrote:
>> From: "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
>>
>> Mempool get/put API cares about cache itself, but sometimes it is
>> required to flush the cache explicitly.
> I don't disagree, but do you have some use-case in mind?

Ideally mempool objects should be reused ASAP. Block/bucket dequeue
bypasses cache, since cache is not block-aware. So, cache should be
flushed before block dequeue. Initially we had cache flush inside block
dequeue wrapper, but decoupling it gives more freedom for optimizations.

>> Also dedicated API allows to decouple it from block get API (to be
>> added) and provides more fine-grained control.
>>
>> Signed-off-by: Artem V. Andreev <Artem.Andreev@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   lib/librte_mempool/rte_mempool.h | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
>> index 9bcb8b7..3a52b93 100644
>> --- a/lib/librte_mempool/rte_mempool.h
>> +++ b/lib/librte_mempool/rte_mempool.h
>> @@ -1161,6 +1161,22 @@ rte_mempool_default_cache(struct rte_mempool *mp, unsigned lcore_id)
>>   }
>>   
>>   /**
>> + * Ensure that a default per-lcore mempool cache is flushed, if it is present
>> + *
>> + * @param mp
>> + *   A pointer to the mempool structure.
>> + */
>> +static __rte_always_inline void
>> +rte_mempool_ensure_cache_flushed(struct rte_mempool *mp)
>> +{
>> +	struct rte_mempool_cache *cache;
>> +	cache = rte_mempool_default_cache(mp, rte_lcore_id());
>> +	if (cache != NULL && cache->len > 0)
>> +		rte_mempool_cache_flush(cache, mp);
>> +}
>> +
> We already have rte_mempool_cache_flush().
> Why not just extending it instead of adding a new function?
>
> I mean:
>
>      static __rte_always_inline void
>      rte_mempool_cache_flush(struct rte_mempool_cache *cache,
>      			struct rte_mempool *mp)
>      {
>     +	if (cache == NULL)
>     +		cache = rte_mempool_default_cache(mp, rte_lcore_id());
>     +	if (cache == NULL || cache->len == 0)
>     +		return;
>      	rte_mempool_ops_enqueue_bulk(mp, cache->objs, cache->len);
>      	cache->len = 0;
>      }

Thanks, good idea.