Hi Andrew,
On Saturday 10 March 2018 09:09 PM, 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>
> ---
> 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
> (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
>
[...]
> diff --git a/lib/librte_mempool/rte_mempool_ops_default.c
> b/lib/librte_mempool/rte_mempool_ops_default.c
> new file mode 100644
> index 0000000..57fe79b
> --- /dev/null
> +++ b/lib/librte_mempool/rte_mempool_ops_default.c
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2016 Intel Corporation.
> + * Copyright(c) 2016 6WIND S.A.
> + * Copyright(c) 2018 Solarflare Communications Inc.
> + */
> +
> +#include <rte_mempool.h>
> +
> +ssize_t
> +rte_mempool_op_calc_mem_size_default(const struct rte_mempool *mp,
> + uint32_t obj_num, uint32_t pg_shift,
> + size_t *min_chunk_size, size_t *align)
> +{
> + unsigned int mp_flags;
> + int ret;
> + size_t total_elt_sz;
> + size_t mem_size;
> +
> + /* Get mempool capabilities */
> + mp_flags = 0;
> + ret = rte_mempool_ops_get_capabilities(mp, &mp_flags);
> + if ((ret < 0) && (ret != -ENOTSUP))
> + return ret;
> +
> + total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size;
> +
> + mem_size = rte_mempool_xmem_size(obj_num, total_elt_sz, pg_shift,
> + mp->flags | mp_flags);
> +
Looks ok to me except a nit:
(mp->flags | mp_flags) style expression is to differentiate that
mp_flags holds driver specific flag like BLK_ALIGN and mp->flags
has appl specific flags.. is it so? If not then why not simply
do like:
mp->flags |= mp_flags.
Thanks.
