From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 85D9B1B026 for ; Mon, 19 Mar 2018 18:03:58 +0100 (CET) Received: from lfbn-lil-1-702-109.w81-254.abo.wanadoo.fr ([81.254.39.109] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1exyCv-00073g-2s; Mon, 19 Mar 2018 18:04:26 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 19 Mar 2018 18:03:52 +0100 Date: Mon, 19 Mar 2018 18:03:52 +0100 From: Olivier Matz To: Andrew Rybchenko Cc: dev@dpdk.org, Bruce Richardson Message-ID: <20180319170352.ddwhimxr6dzkpqea@platinum> References: <1516713372-10572-1-git-send-email-arybchenko@solarflare.com> <1520696382-16400-1-git-send-email-arybchenko@solarflare.com> <1520696382-16400-2-git-send-email-arybchenko@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1520696382-16400-2-git-send-email-arybchenko@solarflare.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v1 1/9] mempool: add op to calculate memory size to be allocated X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Mar 2018 17:03:58 -0000 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 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__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.