From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id 7F8CD3195
 for <dev@dpdk.org>; Mon, 25 Sep 2017 13:33:06 +0200 (CEST)
Received: from lfbn-lil-1-182-75.w90-45.abo.wanadoo.fr ([90.45.31.75]
 helo=droids-corp.org)
 by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.84_2) (envelope-from <olivier.matz@6wind.com>)
 id 1dwRiq-0004Gd-BB; Mon, 25 Sep 2017 13:38:49 +0200
Received: by droids-corp.org (sSMTP sendmail emulation);
 Mon, 25 Sep 2017 13:32:58 +0200
Date: Mon, 25 Sep 2017 13:32:58 +0200
From: Olivier MATZ <olivier.matz@6wind.com>
To: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Cc: dev@dpdk.org, thomas@monjalon.net, jerin.jacob@caviumnetworks.com,
 hemant.agrawal@nxp.com
Message-ID: <20170925113257.kopzsgzxhqiecgnr@platinum>
References: <20170906112834.32378-1-santosh.shukla@caviumnetworks.com>
 <20170907153042.30890-1-santosh.shukla@caviumnetworks.com>
 <20170907153042.30890-8-santosh.shukla@caviumnetworks.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170907153042.30890-8-santosh.shukla@caviumnetworks.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH v6 7/8] mempool: introduce block size align
	flag
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <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: Mon, 25 Sep 2017 11:33:06 -0000

On Thu, Sep 07, 2017 at 09:00:41PM +0530, Santosh Shukla wrote:
> Some mempool hw like octeontx/fpa block, demands block size
> (/total_elem_sz) aligned object start address.
> 
> Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag.
> If this flag is set:
> - Align object start address(vaddr) to a multiple of total_elt_sz.
> - Allocate one additional object. Additional object is needed to make
>   sure that requested 'n' object gets correctly populated.
> 
> Example:
> - Let's say that we get 'x' size of memory chunk from memzone.
> - And application has requested 'n' object from mempool.
> - Ideally, we start using objects at start address 0 to...(x-block_sz)
>   for n obj.
> - Not necessarily first object address i.e. 0 is aligned to block_sz.
> - So we derive 'offset' value for block_sz alignment purpose i.e..'off'.
> - That 'off' makes sure that start address of object is blk_sz aligned.
> - Calculating 'off' may end up sacrificing first block_sz area of
>   memzone area x. So total number of the object which can fit in the
>   pool area is n-1, Which is incorrect behavior.
> 
> Therefore we request one additional object (/block_sz area) from memzone
> when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set.
> 
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>
> [...]
>
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>   */
>  size_t
>  rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
> -		      __rte_unused unsigned int flags)
> +		      unsigned int flags)
>  {
>  	size_t obj_per_page, pg_num, pg_sz;
>  
> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
> +		/* alignment need one additional object */
> +		elt_num += 1;
> +

In previous version, we agreed to test both _BLK_ALIGNED_OBJECTS
and _PHYS_CONTIG in _xmem_size()/_usage(). Here, the test will
also be true if only MEMPOOL_F_CAPA_PHYS_CONTIG is set.

If we want to test both, the test should be:

    mask = MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | MEMPOOL_F_CAPA_PHYS_CONTIG;
    if ((flags & mask) == mask)

> @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>  ssize_t
>  rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num,
>  	size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num,
> -	uint32_t pg_shift, __rte_unused unsigned int flags)
> +	uint32_t pg_shift, unsigned int flags)
>  {
>  	uint32_t elt_cnt = 0;
>  	phys_addr_t start, end;
>  	uint32_t paddr_idx;
>  	size_t pg_sz = (size_t)1 << pg_shift;
>  
> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
> +		/* alignment need one additional object */
> +		elt_num += 1;
> +

Same here