From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v4 2/8] net/mlx4: associate MR to MP in a short function Date: Thu, 2 Nov 2017 14:42:28 +0100 Message-ID: <20171102134228.GA24849@6wind.com> References: <1509358049-18854-1-git-send-email-matan@mellanox.com> <1509474093-31388-1-git-send-email-matan@mellanox.com> <1509474093-31388-3-git-send-email-matan@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, Ophir Munk To: Matan Azrad Return-path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id 145511B3DC for ; Thu, 2 Nov 2017 14:42:41 +0100 (CET) Received: by mail-wm0-f68.google.com with SMTP id r196so11515416wmf.2 for ; Thu, 02 Nov 2017 06:42:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1509474093-31388-3-git-send-email-matan@mellanox.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Matan/Ophir, On Tue, Oct 31, 2017 at 06:21:27PM +0000, Matan Azrad wrote: > From: Ophir Munk > > Associate memory region to mempool (on data path) in a short function. > Handle the less common case of adding a new memory region to mempool > in a separate function. > > Signed-off-by: Ophir Munk Thanks, a few minor comments below to address compilation issues, besides that: Acked-by: Adrien Mazarguil There's an issue about a missing inclusion of assert.h. This one should be addressed before applying this series by the patch I submitted separately: "net/mlx4: fix missing include" Can you include it in front of v5 to make things easier for Ferruh? > --- > drivers/net/mlx4/mlx4.h | 2 ++ > drivers/net/mlx4/mlx4_mr.c | 47 ++++++++++++++++++++++++++++++++++++ > drivers/net/mlx4/mlx4_rxtx.c | 57 -------------------------------------------- > drivers/net/mlx4/mlx4_rxtx.h | 31 +++++++++++++++++++++++- > 4 files changed, 79 insertions(+), 58 deletions(-) > > diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h > index e0a9853..70cf453 100644 > --- a/drivers/net/mlx4/mlx4.h > +++ b/drivers/net/mlx4/mlx4.h > @@ -160,5 +160,7 @@ int mlx4_flow_ctrl_set(struct rte_eth_dev *dev, > /* mlx4_mr.c */ > > struct ibv_mr *mlx4_mp2mr(struct ibv_pd *pd, struct rte_mempool *mp); > +uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp, > + uint32_t i); > > #endif /* RTE_PMD_MLX4_H_ */ > diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c > index 9700884..75ee32f 100644 > --- a/drivers/net/mlx4/mlx4_mr.c > +++ b/drivers/net/mlx4/mlx4_mr.c > @@ -55,6 +55,7 @@ You need to include inttypes.h for PRIu32 definition. Even though it doesn't cause a compilation failure, rte_branch_prediction.h must also be included here for unlikely(). > #include > #include > > +#include "mlx4_rxtx.h" > #include "mlx4_utils.h" > > struct mlx4_check_mempool_data { > @@ -181,3 +182,49 @@ struct ibv_mr * > rte_errno = errno ? errno : EINVAL; > return mr; > } > + > +/** > + * Add memory region (MR) <-> memory pool (MP) association to txq->mp2mr[]. > + * If mp2mr[] is full, remove an entry first. > + * > + * @param txq > + * Pointer to Tx queue structure. > + * @param[in] mp > + * Memory pool for which a memory region lkey must be added. > + * @param[in] i > + * Index in memory pool (MP) where to add memory region (MR). > + * > + * @return > + * Added mr->lkey on success, (uint32_t)-1 on failure. > + */ > +uint32_t > +mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp, uint32_t i) > +{ > + struct ibv_mr *mr; > + > + /* Add a new entry, register MR first. */ > + DEBUG("%p: discovered new memory pool \"%s\" (%p)", > + (void *)txq, mp->name, (void *)mp); > + mr = mlx4_mp2mr(txq->priv->pd, mp); > + if (unlikely(mr == NULL)) { > + DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.", > + (void *)txq); > + return (uint32_t)-1; > + } > + if (unlikely(i == RTE_DIM(txq->mp2mr))) { > + /* Table is full, remove oldest entry. */ > + DEBUG("%p: MR <-> MP table full, dropping oldest entry.", > + (void *)txq); > + --i; > + claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr)); > + memmove(&txq->mp2mr[0], &txq->mp2mr[1], > + (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0]))); > + } > + /* Store the new entry. */ > + txq->mp2mr[i].mp = mp; > + txq->mp2mr[i].mr = mr; > + txq->mp2mr[i].lkey = mr->lkey; > + DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIU32, > + (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey); PRIU32 -> PRIu32 -- Adrien Mazarguil 6WIND