From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 39EF95F1A for ; Tue, 20 Mar 2018 15:41:47 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Mar 2018 07:41:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,335,1517904000"; d="scan'208";a="25526833" Received: from dddjenna-mobl1.ger.corp.intel.com ([10.252.24.63]) by fmsmga007.fm.intel.com with SMTP; 20 Mar 2018 07:41:44 -0700 Received: by (sSMTP sendmail emulation); Tue, 20 Mar 2018 14:41:42 +0000 Date: Tue, 20 Mar 2018 14:41:42 +0000 From: Bruce Richardson To: Olivier Matz Cc: Andrew Rybchenko , dev@dpdk.org Message-ID: <20180320144142.GA11732@bricha3-MOBL.ger.corp.intel.com> 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> <20180319170352.ddwhimxr6dzkpqea@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180319170352.ddwhimxr6dzkpqea@platinum> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.9.4 (2018-02-28) 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: Tue, 20 Mar 2018 14:41:47 -0000 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 > > 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. > 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