diff mbox series

[v1,03/13] graph: add macro to walk on graph circular buffer

Message ID 20221117050926.136974-4-zhirun.yan@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series graph enhancement for multi-core dispatch |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Yan, Zhirun Nov. 17, 2022, 5:09 a.m. UTC 
  It is common to walk on graph circular buffer and use macro to make
it reusable for other worker models.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
---
 lib/graph/rte_graph_model_rtc.h     | 23 ++---------------------
 lib/graph/rte_graph_worker_common.h | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 21 deletions(-)

Comments

Jerin Jacob Feb. 20, 2023, 1:45 p.m. UTC | #1 
On Thu, Nov 17, 2022 at 10:40 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> It is common to walk on graph circular buffer and use macro to make
> it reusable for other worker models.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> ---
>  lib/graph/rte_graph_model_rtc.h     | 23 ++---------------------
>  lib/graph/rte_graph_worker_common.h | 23 +++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 21 deletions(-)

> +/**
> + * Macro to walk on the source node(s) ((cir_start - head) -> cir_start)
> + * and then on the pending streams
> + * (cir_start -> (cir_start + mask) -> cir_start)
> + * in a circular buffer fashion.
> + *
> + *     +-----+ <= cir_start - head [number of source nodes]
> + *     |     |
> + *     | ... | <= source nodes
> + *     |     |
> + *     +-----+ <= cir_start [head = 0] [tail = 0]
> + *     |     |
> + *     | ... | <= pending streams
> + *     |     |
> + *     +-----+ <= cir_start + mask
> + */
> +#define rte_graph_walk_node(graph, head, node)                                         \
> +       for ((node) = RTE_PTR_ADD((graph), (graph)->cir_start[(int32_t)(head)]);        \
> +            likely((head) != (graph)->tail);                                           \
> +            (head)++,                                                                  \
> +            (node) = RTE_PTR_ADD((graph), (graph)->cir_start[(int32_t)(head)]),        \

This is an additional assignment compare to original while() based
version. Right?
No need to generalize with performance impact.


> +            (head) = likely((int32_t)(head) > 0) ? (head) & (graph)->cir_mask : (head))
> +
>  /**
>   * @internal
>   *
> --
> 2.25.1
>
Yan, Zhirun Feb. 24, 2023, 6:30 a.m. UTC | #2 
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, February 20, 2023 9:45 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> ndabilpuram@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> Haiyue <haiyue.wang@intel.com>
> Subject: Re: [PATCH v1 03/13] graph: add macro to walk on graph circular buffer
> 
> On Thu, Nov 17, 2022 at 10:40 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
> >
> > It is common to walk on graph circular buffer and use macro to make it
> > reusable for other worker models.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> > ---
> >  lib/graph/rte_graph_model_rtc.h     | 23 ++---------------------
> >  lib/graph/rte_graph_worker_common.h | 23 +++++++++++++++++++++++
> >  2 files changed, 25 insertions(+), 21 deletions(-)
> 
> > +/**
> > + * Macro to walk on the source node(s) ((cir_start - head) ->
> > +cir_start)
> > + * and then on the pending streams
> > + * (cir_start -> (cir_start + mask) -> cir_start)
> > + * in a circular buffer fashion.
> > + *
> > + *     +-----+ <= cir_start - head [number of source nodes]
> > + *     |     |
> > + *     | ... | <= source nodes
> > + *     |     |
> > + *     +-----+ <= cir_start [head = 0] [tail = 0]
> > + *     |     |
> > + *     | ... | <= pending streams
> > + *     |     |
> > + *     +-----+ <= cir_start + mask
> > + */
> > +#define rte_graph_walk_node(graph, head, node)                                         \
> > +       for ((node) = RTE_PTR_ADD((graph), (graph)->cir_start[(int32_t)(head)]);
> \
> > +            likely((head) != (graph)->tail);                                           \
> > +            (head)++,                                                                  \
> > +            (node) = RTE_PTR_ADD((graph), (graph)->cir_start[(int32_t)(head)]),
> \
> 
> This is an additional assignment compare to original while() based version. Right?
> No need to generalize with performance impact.
Yes, you are right. I will change the macro to use the original while loop.

> 
> 
> > +            (head) = likely((int32_t)(head) > 0) ? (head) &
> > + (graph)->cir_mask : (head))
> > +
> >  /**
> >   * @internal
> >   *
> > --
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/lib/graph/rte_graph_model_rtc.h b/lib/graph/rte_graph_model_rtc.h
index c80b0ce962..5474b06063 100644
--- a/lib/graph/rte_graph_model_rtc.h
+++ b/lib/graph/rte_graph_model_rtc.h
@@ -12,30 +12,11 @@ 
 static inline void
 rte_graph_walk_rtc(struct rte_graph *graph)
 {
-	const rte_graph_off_t *cir_start = graph->cir_start;
-	const rte_node_t mask = graph->cir_mask;
 	uint32_t head = graph->head;
 	struct rte_node *node;
 
-	/*
-	 * Walk on the source node(s) ((cir_start - head) -> cir_start) and then
-	 * on the pending streams (cir_start -> (cir_start + mask) -> cir_start)
-	 * in a circular buffer fashion.
-	 *
-	 *	+-----+ <= cir_start - head [number of source nodes]
-	 *	|     |
-	 *	| ... | <= source nodes
-	 *	|     |
-	 *	+-----+ <= cir_start [head = 0] [tail = 0]
-	 *	|     |
-	 *	| ... | <= pending streams
-	 *	|     |
-	 *	+-----+ <= cir_start + mask
-	 */
-	while (likely(head != graph->tail)) {
-		node = (struct rte_node *)RTE_PTR_ADD(graph, cir_start[(int32_t)head++]);
+	rte_graph_walk_node(graph, head, node)
 		__rte_node_process(graph, node);
-		head = likely((int32_t)head > 0) ? head & mask : head;
-	}
+
 	graph->tail = 0;
 }
diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_worker_common.h
index b7b2bb958c..df33204336 100644
--- a/lib/graph/rte_graph_worker_common.h
+++ b/lib/graph/rte_graph_worker_common.h
@@ -121,6 +121,29 @@  void __rte_node_stream_alloc_size(struct rte_graph *graph,
 
 /* Fast path helper functions */
 
+/**
+ * Macro to walk on the source node(s) ((cir_start - head) -> cir_start)
+ * and then on the pending streams
+ * (cir_start -> (cir_start + mask) -> cir_start)
+ * in a circular buffer fashion.
+ *
+ *	+-----+ <= cir_start - head [number of source nodes]
+ *	|     |
+ *	| ... | <= source nodes
+ *	|     |
+ *	+-----+ <= cir_start [head = 0] [tail = 0]
+ *	|     |
+ *	| ... | <= pending streams
+ *	|     |
+ *	+-----+ <= cir_start + mask
+ */
+#define rte_graph_walk_node(graph, head, node)                                         \
+	for ((node) = RTE_PTR_ADD((graph), (graph)->cir_start[(int32_t)(head)]);        \
+	     likely((head) != (graph)->tail);                                           \
+	     (head)++,                                                                  \
+	     (node) = RTE_PTR_ADD((graph), (graph)->cir_start[(int32_t)(head)]),        \
+	     (head) = likely((int32_t)(head) > 0) ? (head) & (graph)->cir_mask : (head))
+
 /**
  * @internal
  *