Re: [dpdk-dev] [PATCH v2 09/30] net/mlx5: add reference counter on memory region

[Yongseok Koh <yskoh@mellanox.com>]

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