[dpdk-dev] [PATCH v1 1/9] mempool: add op to calculate memory size to be allocated

[Bruce Richardson]

On Mon, Mar 19, 2018 at 06:03:52PM +0100, Olivier Matz wrote:
> 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 <arybchenko at 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.
>
The so version drift occurred during the development of the next-build
tree, sadly. While initially all version were correct, as the patches
flowed into mainline I wasn't able to keep up with all the version changed.
:-(
Since nobody is actually using meson for packaging (yet), I'm not sure this
is critical, so I don't mind whether it's fixed in a separate patch or not.

/Bruce