From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yongseok Koh <yskoh@mellanox.com>
Subject: Re: [PATCH v2 09/30] net/mlx5: add reference counter on
	memory region
Date: Thu, 5 Oct 2017 18:11:26 -0700
Message-ID: <20171006011125.GA19124@yongseok-MBP.local>
References: <cover.1501681927.git.nelio.laranjeiro@6wind.com>
 <cover.1507207731.git.nelio.laranjeiro@6wind.com>
 <d01b3dd40ac3aa449dfdeb1121180c70339535cc.1507207731.git.nelio.laranjeiro@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, adrien.mazarguil@6wind.com, ferruh.yigit@intel.com
To: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from EUR02-AM5-obe.outbound.protection.outlook.com
 (mail-eopbgr00072.outbound.protection.outlook.com [40.107.0.72])
 by dpdk.org (Postfix) with ESMTP id 61EC51B210
 for <dev@dpdk.org>; Fri,  6 Oct 2017 03:11:40 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <d01b3dd40ac3aa449dfdeb1121180c70339535cc.1507207731.git.nelio.laranjeiro@6wind.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Oct 05, 2017 at 02:49:41PM +0200, Nelio Laranjeiro wrote:
[...]
> @@ -180,12 +133,14 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
>  {
>  	struct mlx5_txq_ctrl *txq_ctrl =
>  		container_of(txq, struct mlx5_txq_ctrl, txq);
> -	struct ibv_mr *mr;
> +	struct mlx5_mr *mr;
>  
>  	/* Add a new entry, register MR first. */
>  	DEBUG("%p: discovered new memory pool \"%s\" (%p)",
>  	      (void *)txq_ctrl, mp->name, (void *)mp);
> -	mr = mlx5_mp2mr(txq_ctrl->priv->pd, mp);
> +	mr = priv_mr_get(txq_ctrl->priv, mp);
> +	if (mr == NULL)
> +		mr = priv_mr_new(txq_ctrl->priv, mp);
>  	if (unlikely(mr == NULL)) {
>  		DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
>  		      (void *)txq_ctrl);
> @@ -196,20 +151,17 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
>  		DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
>  		      (void *)txq_ctrl);
>  		--idx;
> -		claim_zero(ibv_dereg_mr(txq_ctrl->txq.mp2mr[0].mr));
> +		priv_mr_release(txq_ctrl->priv, txq_ctrl->txq.mp2mr[0]);
In this function, txq_ctrl->txq can be replaced with txq.

[...]
> @@ -564,26 +572,34 @@ mlx5_tx_mb2mr(struct mlx5_txq_data *txq, struct rte_mbuf *mb)
>  {
>  	uint16_t i = txq->mr_cache_idx;
>  	uintptr_t addr = rte_pktmbuf_mtod(mb, uintptr_t);
> +	uint32_t lkey;
>  
>  	assert(i < RTE_DIM(txq->mp2mr));
> -	if (likely(txq->mp2mr[i].start <= addr && txq->mp2mr[i].end >= addr))
> -		return txq->mp2mr[i].lkey;
> +	if (likely(txq->mp2mr[i]->start <= addr && txq->mp2mr[i]->end >= addr))
> +		return txq->mp2mr[i]->lkey;
>  	for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
> -		if (unlikely(txq->mp2mr[i].mr == NULL)) {
> +		if (unlikely(txq->mp2mr[i]->mr == NULL)) {
>  			/* Unknown MP, add a new MR for it. */
>  			break;
>  		}
> -		if (txq->mp2mr[i].start <= addr &&
> -		    txq->mp2mr[i].end >= addr) {
> -			assert(txq->mp2mr[i].lkey != (uint32_t)-1);
> -			assert(rte_cpu_to_be_32(txq->mp2mr[i].mr->lkey) ==
> -			       txq->mp2mr[i].lkey);
> +		if (txq->mp2mr[i]->start <= addr &&
> +		    txq->mp2mr[i]->end >= addr) {
> +			assert(txq->mp2mr[i]->lkey != (uint32_t)-1);
> +			assert(rte_cpu_to_be_32(txq->mp2mr[i]->mr->lkey) ==
> +			       txq->mp2mr[i]->lkey);
>  			txq->mr_cache_idx = i;
> -			return txq->mp2mr[i].lkey;
> +			return txq->mp2mr[i]->lkey;
>  		}
>  	}
>  	txq->mr_cache_idx = 0;
> -	return mlx5_txq_mp2mr_reg(txq, mlx5_tx_mb2mp(mb), i);
> +	lkey = mlx5_txq_mp2mr_reg(txq, mlx5_tx_mb2mp(mb), i);
> +	/*
> +	 * Request the reference to use in this queue, the original one is
> +	 * kept by the control plane.
> +	 */
> +	if (lkey != (uint32_t)-1)
> +		rte_atomic32_inc(&txq->mp2mr[i]->refcnt);
If mp2mr is overflowed (i == RTE_DIM(txq->mp2mr)), then mp2mr[0] will be removed
with shifting other slots and the new entry will be added at the end. But
referencing txq->mp2mr[i] would be illegal - out of range.

Thanks,
Yongseok