From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f193.google.com (mail-wr0-f193.google.com [209.85.128.193]) by dpdk.org (Postfix) with ESMTP id E9FC4A48C for ; Tue, 16 Jan 2018 09:10:17 +0100 (CET) Received: by mail-wr0-f193.google.com with SMTP id 36so14282172wrh.1 for ; Tue, 16 Jan 2018 00:10:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=s8v/F7z/2icTKb5YiHCowo0gTCT9WL4S1/wGWSOSufo=; b=EUNWAJ3N+sPeUnIWGuxLqOsfn45RnU62GBIW+POd9szLZ0fCflT8j2iTr2HSuxMhym TZx2PM+NzrcUjRKj+t8IgOgyHXimaKiJN06QQaOoDz3OHsDq1ToeHpXWK3QNeIDwFcON RgfGOEqsh7f6iyfYc8KqckvD/nwFFV/eYSCq2ClfJKiwcL6Sr+SLar1nSlEIYcHHtpo+ UrVIj59l2Cl/n5+W5BOHL7yH8mk99NYg2tZ1Ze/CT1P1ks51ypNnHDZ1aOh2X4ylxjLv EvIA7+veVPb1l7wggJKCEErp/D7qxCHxIoO8f+2CF7PD+BqexcK2KbL2GDEK5zvPQR2I bBkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=s8v/F7z/2icTKb5YiHCowo0gTCT9WL4S1/wGWSOSufo=; b=YtUPEK0opFR3bcmbbWZAU/Hf6fMmmtJUpzpkg/CJhopF4lEDBxCYzGaKHEYl+TTKL5 zLSg6YNH7+mYE+m5vCPqnDb5UjJpSSIxZDYu/L0HLhJ6DT8O1ZarXVHhSpxwkfyrlrns 7j1VFV4NrRNgns/pa0r6Cm25JlgRLd1N1JjKQwBufqvALBPpeeyx200SQF0OYHme6r9I XJuyGLDuiarur1zdptVmLVnY4O1y8fao/7bk8oREYbPNb+7Vzlitk6sbbhZmmJDQUWzJ 2dkM68LC/YojYHBlw0bL/4Z8HLoRrtafRjGhvK1iq4xKAbhRVI8NHx6xlMkDr7xfhfmW q9Qw== X-Gm-Message-State: AKwxyte/Ch2h/xorc1xJgfT3jZHpL/P6tgrPK+OfDowKyP0p3zjOqBmZ nNJPxrvQF9KGYJB1WpVW6dZ3 X-Google-Smtp-Source: ACJfBote30YAv6T+LNjocvuNw/9Sy5SalLWG/YPTIbgvmQ1CE37X9yN5ShW7BSExyKFQZhXwzAmOOA== X-Received: by 10.223.131.7 with SMTP id 7mr11810301wrd.272.1516090217473; Tue, 16 Jan 2018 00:10:17 -0800 (PST) Received: from laranjeiro-vm.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p128sm2823471wmb.23.2018.01.16.00.10.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Jan 2018 00:10:16 -0800 (PST) Date: Tue, 16 Jan 2018 09:09:59 +0100 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro To: Xueming Li Cc: Shahaf Shuler , dev@dpdk.org Message-ID: <20180116080959.ksz5abrwryuxflth@laranjeiro-vm.dev.6wind.com> References: <20180115065420.44065-1-xuemingl@mellanox.com> <20180115065420.44065-4-xuemingl@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180115065420.44065-4-xuemingl@mellanox.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH 4/4] net/mlx5: remove MR refcnt X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jan 2018 08:10:18 -0000 Hi Xueming, On Mon, Jan 15, 2018 at 02:54:20PM +0800, Xueming Li wrote: > MRs are registered to priv at device start or on the fly, destroyied > when device stop. > No mR registration to perticular TXQ, MR references cache in TXQ just > part of device MR list, no reason to keep MR refcnt. > > Signed-off-by: Xueming Li > --- > drivers/net/mlx5/mlx5.h | 1 - > drivers/net/mlx5/mlx5_mr.c | 51 +++++--------------------------------------- > drivers/net/mlx5/mlx5_rxq.c | 1 - > drivers/net/mlx5/mlx5_rxtx.h | 2 -- > drivers/net/mlx5/mlx5_txq.c | 18 ---------------- > 5 files changed, 5 insertions(+), 68 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index a6cd1474c..df6a70d96 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -318,7 +318,6 @@ int priv_socket_connect(struct priv *priv); > > struct mlx5_mr *priv_mr_new(struct priv *priv, struct rte_mempool *mp, > uintptr_t addr); > -struct mlx5_mr *priv_mr_get(struct priv *, struct rte_mempool *); > int priv_mr_release(struct priv *, struct mlx5_mr *); > int priv_mr_verify(struct priv *); > struct mlx5_mr *priv_mr_lookup(struct priv *priv, uintptr_t addr); > diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c > index 8d8fabea1..bd1be5606 100644 > --- a/drivers/net/mlx5/mlx5_mr.c > +++ b/drivers/net/mlx5/mlx5_mr.c > @@ -88,15 +88,11 @@ mlx5_mempool_register_cb(struct rte_mempool *mp, void *opaque, > cb->error = -1; > return; > } > - mr->mp = mp; > DEBUG("mempool %p:%u area start=%p size=%zu lkey=0x%08x", > (void *)mp, mem_idx, memhdr->addr, memhdr->len, mr->mr->lkey); > mr->lkey = rte_cpu_to_be_32(mr->mr->lkey); > mr->start = (uintptr_t)memhdr->addr; > mr->end = (uintptr_t)mr->mr->addr + mr->mr->length; > - rte_atomic32_inc(&mr->refcnt); > - DEBUG("%p: new Memory Region %p refcnt: %d", (void *)priv, > - (void *)mr, rte_atomic32_read(&mr->refcnt)); Please keep the debug message. > LIST_INSERT_HEAD(&priv->mr, mr, next); > cb->mr = mr; > } > @@ -121,11 +117,8 @@ mlx5_mp2mr(struct priv *priv, struct rte_mempool *mp, uintptr_t addr) > > priv_lock(priv); > mr = priv_mr_lookup(priv, addr); > - if (!mr) { > + if (!mr) > mr = priv_mr_new(priv, mp, addr); > - if (mr) > - rte_atomic32_inc(&mr->refcnt); > - } > priv_unlock(priv); > return mr; > } > @@ -217,35 +210,6 @@ priv_mr_new(struct priv *priv, struct rte_mempool *mp, uintptr_t addr) > } > > /** > - * Search the memory region object in the memory region list. > - * > - * @param priv > - * Pointer to private structure. > - * @param mp > - * Pointer to the memory pool to register. > - * @return > - * The memory region on success. > - */ > -struct mlx5_mr* > -priv_mr_get(struct priv *priv, struct rte_mempool *mp) > -{ > - struct mlx5_mr *mr; > - > - assert(mp); > - if (LIST_EMPTY(&priv->mr)) > - return NULL; > - LIST_FOREACH(mr, &priv->mr, next) { > - if (mr->mp == mp) { > - rte_atomic32_inc(&mr->refcnt); > - DEBUG("Memory Region %p refcnt: %d", > - (void *)mr, rte_atomic32_read(&mr->refcnt)); > - return mr; > - } > - } > - return NULL; > -} > - > -/** > * Release the memory region object. > * > * @param mr > @@ -259,15 +223,10 @@ priv_mr_release(struct priv *priv, struct mlx5_mr *mr) > { > (void)priv; > assert(mr); > - DEBUG("Memory Region %p refcnt: %d", > - (void *)mr, rte_atomic32_read(&mr->refcnt)); > - if (rte_atomic32_dec_and_test(&mr->refcnt)) { > - claim_zero(ibv_dereg_mr(mr->mr)); > - LIST_REMOVE(mr, next); > - rte_free(mr); > - return 0; > - } > - return EBUSY; > + claim_zero(ibv_dereg_mr(mr->mr)); > + LIST_REMOVE(mr, next); > + rte_free(mr); > + return 0; > } > > /** > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c > index a581a2d61..461b4ff91 100644 > --- a/drivers/net/mlx5/mlx5_rxq.c > +++ b/drivers/net/mlx5/mlx5_rxq.c > @@ -852,7 +852,6 @@ mlx5_priv_rxq_ibv_get(struct priv *priv, uint16_t idx) > return NULL; > rxq_ctrl = container_of(rxq_data, struct mlx5_rxq_ctrl, rxq); > if (rxq_ctrl->ibv) { > - priv_mr_get(priv, rxq_data->mp); > rte_atomic32_inc(&rxq_ctrl->ibv->refcnt); > DEBUG("%p: Verbs Rx queue %p: refcnt %d", (void *)priv, > (void *)rxq_ctrl->ibv, > diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h > index 6589a662e..0b8332cc0 100644 > --- a/drivers/net/mlx5/mlx5_rxtx.h > +++ b/drivers/net/mlx5/mlx5_rxtx.h > @@ -85,12 +85,10 @@ struct priv; > /* Memory region queue object. */ > struct mlx5_mr { > LIST_ENTRY(mlx5_mr) next; /**< Pointer to the next element. */ > - rte_atomic32_t refcnt; /*<< Reference counter. */ > uint32_t lkey; /*<< rte_cpu_to_be_32(mr->lkey) */ > uintptr_t start; /* Start address of MR */ > uintptr_t end; /* End address of MR */ > struct ibv_mr *mr; /*<< Memory Region. */ > - struct rte_mempool *mp; /*<< Memory Pool. */ > }; > > /* Compressed CQE context. */ > diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c > index 26db15a4f..3c337a4ac 100644 > --- a/drivers/net/mlx5/mlx5_txq.c > +++ b/drivers/net/mlx5/mlx5_txq.c > @@ -801,18 +801,7 @@ mlx5_priv_txq_get(struct priv *priv, uint16_t idx) > if ((*priv->txqs)[idx]) { > ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl, > txq); > - unsigned int i; > - > mlx5_priv_txq_ibv_get(priv, idx); > - for (i = 0; i != MLX5_PMD_TX_MP_CACHE; ++i) { > - struct mlx5_mr *mr = NULL; > - > - (void)mr; > - if (ctrl->txq.mp2mr[i]) { > - mr = priv_mr_get(priv, ctrl->txq.mp2mr[i]->mp); > - assert(mr); > - } > - } > rte_atomic32_inc(&ctrl->refcnt); > DEBUG("%p: Tx queue %p: refcnt %d", (void *)priv, > (void *)ctrl, rte_atomic32_read(&ctrl->refcnt)); > @@ -834,7 +823,6 @@ mlx5_priv_txq_get(struct priv *priv, uint16_t idx) > int > mlx5_priv_txq_release(struct priv *priv, uint16_t idx) > { > - unsigned int i; > struct mlx5_txq_ctrl *txq; > > if (!(*priv->txqs)[idx]) > @@ -849,12 +837,6 @@ mlx5_priv_txq_release(struct priv *priv, uint16_t idx) > if (!ret) > txq->ibv = NULL; > } > - for (i = 0; i != MLX5_PMD_TX_MP_CACHE; ++i) { > - if (txq->txq.mp2mr[i]) { > - priv_mr_release(priv, txq->txq.mp2mr[i]); > - txq->txq.mp2mr[i] = NULL; > - } > - } Nullifying the case when the queue is release should remain. It is better to detect an access to a null pointer than to a freed memory space. > if (rte_atomic32_dec_and_test(&txq->refcnt)) { > txq_free_elts(txq); > LIST_REMOVE(txq, next); > -- > 2.13.3 > Thanks, -- Nélio Laranjeiro 6WIND