On Sat, Mar 10, 2018 at 03:39:34PM +0000, Andrew Rybchenko wrote:
> Size of memory chunk required to populate mempool objects depends
> on how objects are stored in the memory. Different mempool drivers
> may have different requirements and a new operation allows to
> calculate memory size in accordance with driver requirements and
> advertise requirements on minimum memory chunk size and alignment
> in a generic way.
>
> Bump ABI version since the patch breaks it.
>
> Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
Looks good to me. Just see below for few minor comments.
> ---
> RFCv2 -> v1:
> - move default calc_mem_size callback to rte_mempool_ops_default.c
> - add ABI changes to release notes
> - name default callback consistently: rte_mempool_op_<callback>_default()
> - bump ABI version since it is the first patch which breaks ABI
> - describe default callback behaviour in details
> - avoid introduction of internal function to cope with depration
typo (depration)
> (keep it to deprecation patch)
> - move cache-line or page boundary chunk alignment to default callback
> - highlight that min_chunk_size and align parameters are output only
[...]
> --- a/lib/librte_mempool/Makefile
> +++ b/lib/librte_mempool/Makefile
> @@ -11,11 +11,12 @@ LDLIBS += -lrte_eal -lrte_ring
>
> EXPORT_MAP := rte_mempool_version.map
>
> -LIBABIVER := 3
> +LIBABIVER := 4
>
> # all source are stored in SRCS-y
> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool.c
> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_ops.c
> +SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_ops_default.c
> # install includes
> SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
>
> diff --git a/lib/librte_mempool/meson.build b/lib/librte_mempool/meson.build
> index 7a4f3da..9e3b527 100644
> --- a/lib/librte_mempool/meson.build
> +++ b/lib/librte_mempool/meson.build
> @@ -1,7 +1,8 @@
> # SPDX-License-Identifier: BSD-3-Clause
> # Copyright(c) 2017 Intel Corporation
>
> -version = 2
> -sources = files('rte_mempool.c', 'rte_mempool_ops.c')
> +version = 4
> +sources = files('rte_mempool.c', 'rte_mempool_ops.c',
> + 'rte_mempool_ops_default.c')
> headers = files('rte_mempool.h')
> deps += ['ring']
It's strange to see that meson does not have the same
.so version than the legacy build system.
+CC Bruce in case he wants to fix this issue separately.
[...]
> --- a/lib/librte_mempool/rte_mempool_version.map
> +++ b/lib/librte_mempool/rte_mempool_version.map
> @@ -51,3 +51,11 @@ DPDK_17.11 {
> rte_mempool_populate_iova_tab;
>
> } DPDK_16.07;
> +
> +DPDK_18.05 {
> + global:
> +
> + rte_mempool_op_calc_mem_size_default;
> +
> +} DPDK_17.11;
> +
Another minor comment. When applying the patch with git am:
Applying: mempool: add op to calculate memory size to be allocated
.git/rebase-apply/patch:399: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
