Re: [dpdk-dev] [PATCH v1 0/9] mempool: prepare to add bucket driver

[Olivier Matz <olivier.matz@6wind.com>]

Hi Andrew,

Thank you for this nice rework.
Globally, the patchset looks good to me. I'm sending some comments
as reply to specific patches.

On Sat, Mar 10, 2018 at 03:39:33PM +0000, Andrew Rybchenko wrote:
> The initial patch series [1] is split into two to simplify processing.
> The second series relies on this one and will add bucket mempool driver
> and related ops.
> 
> The patch series has generic enhancements suggested by Olivier.
> Basically it adds driver callbacks to calculate required memory size and
> to populate objects using provided memory area. It allows to remove
> so-called capability flags used before to tell generic code how to
> allocate and slice allocated memory into mempool objects.
> Clean up which removes get_capabilities and register_memory_area is
> not strictly required, but I think right thing to do.
> Existing mempool drivers are updated.
> 
> I've kept rte_mempool_populate_iova_tab() intact since it seems to
> be not directly related XMEM API functions.

The function rte_mempool_populate_iova_tab() (actually, it was
rte_mempool_populate_phys_tab()) was introduced to support XMEM
API. In my opinion, it can also be deprecated.

> It breaks ABI since changes rte_mempool_ops. Also it removes
> rte_mempool_ops_register_memory_area() and
> rte_mempool_ops_get_capabilities() since corresponding callbacks are
> removed.
> 
> Internal global functions are not listed in map file since it is not
> a part of external API.
> 
> [1] http://dpdk.org/ml/archives/dev/2018-January/088698.html
> 
> RFCv1 -> RFCv2:
>   - add driver ops to calculate required memory size and populate
>     mempool objects, remove extra flags which were required before
>     to control it
>   - transition of octeontx and dpaa drivers to the new callbacks
>   - change info API to get information from driver required to
>     API user to know contiguous block size
>   - remove get_capabilities (not required any more and may be
>     substituted with more in info get API)
>   - remove register_memory_area since it is substituted with
>     populate callback which can do more
>   - use SPDX tags
>   - avoid all objects affinity to single lcore
>   - fix bucket get_count
>   - deprecate XMEM API
>   - avoid introduction of a new function to flush cache
>   - fix NO_CACHE_ALIGN case in bucket mempool
> 
> RFCv2 -> v1:
>   - split the series in two
>   - squash octeontx patches which implement calc_mem_size and populate
>     callbacks into the patch which removes get_capabilities since it is
>     the easiest way to untangle the tangle of tightly related library
>     functions and flags advertised by the driver
>   - consistently name default callbacks
>   - move default callbacks to dedicated file
>   - see detailed description in patches
> 
> Andrew Rybchenko (7):
>   mempool: add op to calculate memory size to be allocated
>   mempool: add op to populate objects using provided memory
>   mempool: remove callback to get capabilities
>   mempool: deprecate xmem functions
>   mempool/octeontx: prepare to remove register memory area op
>   mempool/dpaa: prepare to remove register memory area op
>   mempool: remove callback to register memory area
> 
> Artem V. Andreev (2):
>   mempool: ensure the mempool is initialized before populating
>   mempool: support flushing the default cache of the mempool
> 
>  doc/guides/rel_notes/deprecation.rst            |  12 +-
>  doc/guides/rel_notes/release_18_05.rst          |  32 ++-
>  drivers/mempool/dpaa/dpaa_mempool.c             |  13 +-
>  drivers/mempool/octeontx/rte_mempool_octeontx.c |  64 ++++--
>  lib/librte_mempool/Makefile                     |   3 +-
>  lib/librte_mempool/meson.build                  |   5 +-
>  lib/librte_mempool/rte_mempool.c                | 159 +++++++--------
>  lib/librte_mempool/rte_mempool.h                | 260 +++++++++++++++++-------
>  lib/librte_mempool/rte_mempool_ops.c            |  37 ++--
>  lib/librte_mempool/rte_mempool_ops_default.c    |  51 +++++
>  lib/librte_mempool/rte_mempool_version.map      |  11 +-
>  test/test/test_mempool.c                        |  31 ---
>  12 files changed, 437 insertions(+), 241 deletions(-)
>  create mode 100644 lib/librte_mempool/rte_mempool_ops_default.c
> 
> -- 
> 2.7.4
>