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 E36FE7F70
 for <dev@dpdk.org>; Thu, 19 Apr 2018 18:41:35 +0200 (CEST)
Received: from alille-654-1-86-209.w90-58.abo.wanadoo.fr ([90.58.125.209]
 helo=droids-corp.org)
 by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.89) (envelope-from <olivier.matz@6wind.com>)
 id 1f9Ccq-0001Kh-Bv; Thu, 19 Apr 2018 18:41:40 +0200
Received: by droids-corp.org (sSMTP sendmail emulation);
 Thu, 19 Apr 2018 18:41:30 +0200
Date: Thu, 19 Apr 2018 18:41:30 +0200
From: Olivier Matz <olivier.matz@6wind.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: dev@dpdk.org, "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
Message-ID: <20180419164130.ghqbhz6qvkwhnnua@platinum>
References: <1516713372-10572-1-git-send-email-arybchenko@solarflare.com>
 <1522080779-25472-1-git-send-email-arybchenko@solarflare.com>
 <1522080779-25472-4-git-send-email-arybchenko@solarflare.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1522080779-25472-4-git-send-email-arybchenko@solarflare.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH v1 3/6] mempool: support block dequeue
	operation
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 19 Apr 2018 16:41:36 -0000

On Mon, Mar 26, 2018 at 05:12:56PM +0100, Andrew Rybchenko wrote:
> From: "Artem V. Andreev" <Artem.Andreev@oktetlabs.ru>
> 
> If mempool manager supports object blocks (physically and virtual
> contiguous set of objects), it is sufficient to get the first
> object only and the function allows to avoid filling in of
> information about each block member.
> 
> Signed-off-by: Artem V. Andreev <Artem.Andreev@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Minor things here, please see below.

[...]

> @@ -1531,6 +1615,71 @@ rte_mempool_get(struct rte_mempool *mp, void **obj_p)
>  }
>  
>  /**
> + * @internal Get contiguous blocks of objects from the pool. Used internally.
> + * @param mp
> + *   A pointer to the mempool structure.
> + * @param first_obj_table
> + *   A pointer to a pointer to the first object in each block.
> + * @param n
> + *   A number of blocks to get.
> + * @return
> + *   - >0: Success
> + *   - <0: Error

I guess it is 0 here, not >0.


> + */
> +static __rte_always_inline int
> +__mempool_generic_get_contig_blocks(struct rte_mempool *mp,
> +				    void **first_obj_table, unsigned int n)
> +{
> +	int ret;
> +
> +	ret = rte_mempool_ops_dequeue_contig_blocks(mp, first_obj_table, n);
> +	if (ret < 0)
> +		__MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_fail, n);
> +	else
> +		__MEMPOOL_CONTIG_BLOCKS_STAT_ADD(mp, get_success, n);
> +
> +	return ret;
> +}
> +

Is it worth having this function?
I think it would be simple to include the code in
rte_mempool_get_contig_blocks() below... or am I missing something?


> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Get a contiguous blocks of objects from the mempool.
> + *
> + * If cache is enabled, consider to flush it first, to reuse objects
> + * as soon as possible.
> + *
> + * The application should check that the driver supports the operation
> + * by calling rte_mempool_ops_get_info() and checking that `contig_block_size`
> + * is not zero.
> + *
> + * @param mp
> + *   A pointer to the mempool structure.
> + * @param first_obj_table
> + *   A pointer to a pointer to the first object in each block.
> + * @param n
> + *   The number of blocks to get from mempool.
> + * @return
> + *   - 0: Success; blocks taken.
> + *   - -ENOBUFS: Not enough entries in the mempool; no object is retrieved.
> + *   - -EOPNOTSUPP: The mempool driver does not support block dequeue
> + */
> +static __rte_always_inline int
> +__rte_experimental
> +rte_mempool_get_contig_blocks(struct rte_mempool *mp,
> +			      void **first_obj_table, unsigned int n)
> +{
> +	int ret;
> +
> +	ret = __mempool_generic_get_contig_blocks(mp, first_obj_table, n);
> +	if (ret == 0)
> +		__mempool_contig_blocks_check_cookies(mp, first_obj_table, n,
> +						      1);
> +	return ret;
> +}