On Thu, Oct 05, 2017 at 02:49:42PM +0200, Nelio Laranjeiro wrote:
[...]
> +struct mlx5_rxq_ibv*
> +mlx5_priv_rxq_ibv_get(struct priv *priv, uint16_t idx)
> +{
> +     struct mlx5_rxq_data *rxq_data = (*priv->rxqs)[idx];
> +     struct mlx5_rxq_ctrl *rxq_ctrl;
> +
> +     if (idx >= priv->rxqs_n)
> +             return NULL;
> +     if (!rxq_data)
> +             return NULL;
> +     rxq_ctrl = container_of(rxq_data, struct mlx5_rxq_ctrl, rxq);
> +     if (rxq_ctrl->ibv) {
> +             priv_mr_get(priv, rxq_data->mp);

One rxq_ibv has one mr as one rxq has one mp. As long as rxq_ibv exist, the mr
can't be released. So, it looks unnecessary to increase refcnt of the mr here.

> +             rte_atomic32_inc(&rxq_ctrl->ibv->refcnt);
> +             DEBUG("%p: Verbs Rx queue %p: refcnt %d", (void *)priv,
> +                   (void *)rxq_ctrl->ibv,
> +                   rte_atomic32_read(&rxq_ctrl->ibv->refcnt));
> +     }
> +     return rxq_ctrl->ibv;
> +}
> +
> +/**
> + * Release an Rx verbs queue object.
> + *
> + * @param priv
> + *   Pointer to private structure.
> + * @param rxq_ibv
> + *   Verbs Rx queue object.
> + *
> + * @return
> + *   0 on success, errno value on failure.
> + */
> +int
> +mlx5_priv_rxq_ibv_release(struct priv *priv, struct mlx5_rxq_ibv *rxq_ibv)
> +{
> +     int ret;
> +
> +     assert(rxq_ibv);
> +     assert(rxq_ibv->wq);
> +     assert(rxq_ibv->cq);
> +     assert(rxq_ibv->mr);
> +     ret = priv_mr_release(priv, rxq_ibv->mr);

Like I mentioned above, this can be moved inside the following destruction part.
 
As current code is logically flawless,
Acked-by: Yongseok Koh <ys...@mellanox.com>
 
Thanks

Reply via email to