Re: [dpdk-dev] [PATCH v2 7/8] net/mlx4: align Tx descriptors number

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

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@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?

-- 
Adrien Mazarguil
6WIND