Hi Adrien
> -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > Sent: Wednesday, December 6, 2017 6:23 PM > To: Matan Azrad <ma...@mellanox.com> > Cc: dev@dpdk.org > Subject: Re: [PATCH v2 7/8] net/mlx4: align Tx descriptors number > > On Wed, Dec 06, 2017 at 02:48:12PM +0000, Matan Azrad wrote: > > Using power of 2 descriptors number makes the ring management easier > > and allows to use mask operation instead of wraparound conditions. > > > > Adjust Tx descriptor number to be power of 2 and change calculation to > > use mask accordingly. > > > > Signed-off-by: Matan Azrad <ma...@mellanox.com> > > --- > > drivers/net/mlx4/mlx4_rxtx.c | 28 +++++++++++++--------------- > > drivers/net/mlx4/mlx4_txq.c | 13 +++++++++---- > > 2 files changed, 22 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/net/mlx4/mlx4_rxtx.c > > b/drivers/net/mlx4/mlx4_rxtx.c index 8b8d95e..14192fe 100644 > > --- a/drivers/net/mlx4/mlx4_rxtx.c > > +++ b/drivers/net/mlx4/mlx4_rxtx.c > > @@ -312,10 +312,14 @@ struct pv { > > * > > * @param txq > > * Pointer to Tx queue structure. > > + * @param sq > > + * Pointer to the SQ structure. > > + * @param elts_m > > + * Tx elements number mask. > > It's minor however these parameters should be described in the same order > as they appear in the function prototype, please swap them if you send an > updated series. > > > */ > > static void > > -mlx4_txq_complete(struct txq *txq, const unsigned int elts_n, > > - struct mlx4_sq *sq) > > +mlx4_txq_complete(struct txq *txq, const unsigned int elts_m, > > + struct mlx4_sq *sq) > > { > <snip> > > diff --git a/drivers/net/mlx4/mlx4_txq.c b/drivers/net/mlx4/mlx4_txq.c > > index 4c7b62a..7eb4b04 100644 > > --- a/drivers/net/mlx4/mlx4_txq.c > > +++ b/drivers/net/mlx4/mlx4_txq.c > > @@ -76,17 +76,16 @@ > > unsigned int elts_head = txq->elts_head; > > unsigned int elts_tail = txq->elts_tail; > > struct txq_elt (*elts)[txq->elts_n] = txq->elts; > > + unsigned int elts_m = txq->elts_n - 1; > > > > DEBUG("%p: freeing WRs", (void *)txq); > > while (elts_tail != elts_head) { > > - struct txq_elt *elt = &(*elts)[elts_tail]; > > + struct txq_elt *elt = &(*elts)[elts_tail++ & elts_m]; > > > > assert(elt->buf != NULL); > > rte_pktmbuf_free(elt->buf); > > elt->buf = NULL; > > elt->wqe = NULL; > > - if (++elts_tail == RTE_DIM(*elts)) > > - elts_tail = 0; > > } > > txq->elts_tail = txq->elts_head; > > } > > @@ -208,7 +207,7 @@ struct txq_mp2mr_mbuf_check_data { > > struct mlx4dv_obj mlxdv; > > struct mlx4dv_qp dv_qp; > > struct mlx4dv_cq dv_cq; > > - struct txq_elt (*elts)[desc]; > > + struct txq_elt (*elts)[rte_align32pow2(desc)]; > > OK, I'm curious about what happened to the magic 0x1000 though? Was it a > limitation or some leftover debugging code? > Wrong limitation to the max number of descriptors. Thanks again for the second good review. Will adjust all your comments for v3. > -- > Adrien Mazarguil > 6WIND
