From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com
 [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 6C483568D
 for <dev@dpdk.org>; Wed, 24 Aug 2016 18:15:27 +0200 (CEST)
Received: from [10.16.0.195] (unknown [10.16.0.195])
 by proxy.6wind.com (Postfix) with ESMTP id 557812B20E;
 Wed, 24 Aug 2016 18:15:27 +0200 (CEST)
To: "Sanford, Robert" <rsanford@akamai.com>, "dev@dpdk.org" <dev@dpdk.org>
References: <1470084176-79932-1-git-send-email-rsanford@akamai.com>
 <1470084176-79932-3-git-send-email-rsanford@akamai.com>
 <57BC6721.5090805@6wind.com>
 <D6827E65-62EF-494B-83B6-BBCF9649D9F3@akamai.com>
Cc: "declan.doherty@intel.com" <declan.doherty@intel.com>,
 "pablo.de.lara.guarch@intel.com" <pablo.de.lara.guarch@intel.com>
From: Olivier MATZ <olivier.matz@6wind.com>
Message-ID: <57BDC81D.2000902@6wind.com>
Date: Wed, 24 Aug 2016 18:15:25 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Icedove/38.6.0
MIME-Version: 1.0
In-Reply-To: <D6827E65-62EF-494B-83B6-BBCF9649D9F3@akamai.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH 2/4] mempool: make cache flush threshold
 macro public
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 24 Aug 2016 16:15:27 -0000

Hi Robert,

On 08/23/2016 06:07 PM, Sanford, Robert wrote:
> Hi Olivier,
>
> On 8/23/16, 11:09 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>
>      Hi Robert,
>
>      On 08/01/2016 10:42 PM, Robert Sanford wrote:
>      > Rename macros that calculate a mempool cache flush threshold, and
>      > move them from rte_mempool.c to rte_mempool.h, so that the bonding
>      > driver can accurately calculate its mbuf requirements.
>      >
>      > Signed-off-by: Robert Sanford <rsanford@akamai.com>
>      > ---
>      >   lib/librte_mempool/rte_mempool.c |    8 ++------
>      >   lib/librte_mempool/rte_mempool.h |    7 +++++++
>      >   2 files changed, 9 insertions(+), 6 deletions(-)
>      >
>      > [...]
>      >
>      > --- a/lib/librte_mempool/rte_mempool.h
>      > +++ b/lib/librte_mempool/rte_mempool.h
>      > @@ -263,6 +263,13 @@ struct rte_mempool {
>      >   #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
>      >
>      >   /**
>      > + * Calculate the threshold before we flush excess elements.
>      > + */
>      > +#define RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER 1.5
>      > +#define RTE_MEMPOOL_CALC_CACHE_FLUSHTHRESH(c)	\
>      > +	((typeof(c))((c) * RTE_MEMPOOL_CACHE_FLUSHTHRESH_MULTIPLIER))
>      > +
>      > +/**
>      >    * @internal When debug is enabled, store some statistics.
>      >    *
>      >    * @param mp
>      >
>
>      What do you think of using a static inline function instead of a macro ?
>
>
>      Regards,
>      Olivier
> --
>
> Yes, an inline function is better than a macro. We still need to move the #define MULTIPLIER from rte_mempool.c to rte_mempool.h.
>
> How is this for the prototype?
> static inline unsigned rte_mempool_calc_cache_flushthresh(unsigned cache_size)
>
> Where in the .h should we place the function, right below the MULTIPLIER definition?

Yep, looks good, thanks.

Maybe "unsigned" -> "unsigned int", because checkpatch may complain...


Regards,
Olivier