[v2,3/4] net/mlx5: add free on completion queue

Message ID 1578567380-26994-4-git-send-email-viacheslavo@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: remove Tx descriptor reserved field usage |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Slava Ovsiienko Jan. 9, 2020, 10:56 a.m. UTC
  The new software manged entity is introduced in Tx datapath
- free on completion queue. This queue keeps the information
how many buffers stored in elts array must freed on send
comletion. Each element of the queue contains transmitting
descriptor index to be in synch with completion entries (in
debug build only) and the index in elts array to free buffers.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxtx.h |  5 +++++
 drivers/net/mlx5/mlx5_txq.c  | 15 +++++++++++++++
 2 files changed, 20 insertions(+)
  

Comments

Ferruh Yigit Jan. 9, 2020, 3:12 p.m. UTC | #1
On 1/9/2020 10:56 AM, Viacheslav Ovsiienko wrote:
> The new software manged entity is introduced in Tx datapath
> - free on completion queue. This queue keeps the information
> how many buffers stored in elts array must freed on send
> comletion. Each element of the queue contains transmitting
> descriptor index to be in synch with completion entries (in
> debug build only) and the index in elts array to free buffers.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>

<...>

> @@ -297,6 +297,11 @@ struct mlx5_txq_data {
>  	struct mlx5_mr_ctrl mr_ctrl; /* MR control descriptor. */
>  	struct mlx5_wqe *wqes; /* Work queue. */
>  	struct mlx5_wqe *wqes_end; /* Work queue array limit. */
> +#ifdef NDEBUG
> +	uint32_t *fcqs; /* Free completion queue. */
> +#else
> +	uint32_t *fcqs; /* Free completion queue (debug extended). */
> +#endif

Why is the #ifdef required?
  
Slava Ovsiienko Jan. 9, 2020, 3:22 p.m. UTC | #2
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, January 9, 2020 17:12
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v2 3/4] net/mlx5: add free on completion
> queue
> 
> On 1/9/2020 10:56 AM, Viacheslav Ovsiienko wrote:
> > The new software manged entity is introduced in Tx datapath
> > - free on completion queue. This queue keeps the information how many
> > buffers stored in elts array must freed on send comletion. Each
> > element of the queue contains transmitting descriptor index to be in
> > synch with completion entries (in debug build only) and the index in
> > elts array to free buffers.
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > Acked-by: Matan Azrad <matan@mellanox.com>
> 
> <...>
> 
> > @@ -297,6 +297,11 @@ struct mlx5_txq_data {
> >  	struct mlx5_mr_ctrl mr_ctrl; /* MR control descriptor. */
> >  	struct mlx5_wqe *wqes; /* Work queue. */
> >  	struct mlx5_wqe *wqes_end; /* Work queue array limit. */
> > +#ifdef NDEBUG
> > +	uint32_t *fcqs; /* Free completion queue. */ #else
> > +	uint32_t *fcqs; /* Free completion queue (debug extended). */ #endif
> 
> Why is the #ifdef required?

It is a misprint, in non-debug version it should be "uint16_t*" to save some memory.
Thanks for the pointing out, will fix.

With best regards, Slava
  

Patch

diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 8a2185a..ee1895b 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -297,6 +297,11 @@  struct mlx5_txq_data {
 	struct mlx5_mr_ctrl mr_ctrl; /* MR control descriptor. */
 	struct mlx5_wqe *wqes; /* Work queue. */
 	struct mlx5_wqe *wqes_end; /* Work queue array limit. */
+#ifdef NDEBUG
+	uint32_t *fcqs; /* Free completion queue. */
+#else
+	uint32_t *fcqs; /* Free completion queue (debug extended). */
+#endif
 	volatile struct mlx5_cqe *cqes; /* Completion queue. */
 	volatile uint32_t *qp_db; /* Work queue doorbell. */
 	volatile uint32_t *cq_db; /* Completion queue doorbell. */
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index abe0947..aee0970 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -724,6 +724,17 @@  struct mlx5_txq_obj *
 	txq_data->wqe_pi = 0;
 	txq_data->wqe_comp = 0;
 	txq_data->wqe_thres = txq_data->wqe_s / MLX5_TX_COMP_THRESH_INLINE_DIV;
+	txq_data->fcqs = rte_calloc_socket(__func__,
+					   txq_data->cqe_s,
+					   sizeof(*txq_data->fcqs),
+					   RTE_CACHE_LINE_SIZE,
+					   txq_ctrl->socket);
+	if (!txq_data->fcqs) {
+		DRV_LOG(ERR, "port %u Tx queue %u cannot allocate memory (FCQ)",
+			dev->data->port_id, idx);
+		rte_errno = ENOMEM;
+		goto error;
+	}
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 	/*
 	 * If using DevX need to query and store TIS transport domain value.
@@ -772,6 +783,8 @@  struct mlx5_txq_obj *
 		claim_zero(mlx5_glue->destroy_cq(tmpl.cq));
 	if (tmpl.qp)
 		claim_zero(mlx5_glue->destroy_qp(tmpl.qp));
+	if (txq_data && txq_data->fcqs)
+		rte_free(txq_data->fcqs);
 	if (txq_obj)
 		rte_free(txq_obj);
 	priv->verbs_alloc_ctx.type = MLX5_VERBS_ALLOC_TYPE_NONE;
@@ -826,6 +839,8 @@  struct mlx5_txq_obj *
 		} else {
 			claim_zero(mlx5_glue->destroy_qp(txq_obj->qp));
 			claim_zero(mlx5_glue->destroy_cq(txq_obj->cq));
+				if (txq_obj->txq_ctrl->txq.fcqs)
+					rte_free(txq_obj->txq_ctrl->txq.fcqs);
 		}
 		LIST_REMOVE(txq_obj, next);
 		rte_free(txq_obj);