[dpdk-dev] [PATCH v2 7/8] net/mlx4: align Tx descriptors number
Matan Azrad
matan at mellanox.com
Wed Dec 6 18:24:22 CET 2017
Hi Adrien
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Wednesday, December 6, 2017 6:23 PM
> To: Matan Azrad <matan at mellanox.com>
> Cc: dev at 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 <matan at 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
More information about the dev
mailing list