[PATCH v2] eal: add tracepoints to track lcores and services

[PATCH v2] eal: add tracepoints to track lcores and services

[Arnaud Fiorini]

The tracepoints added are used to track lcore role and status,
as well as service mapping and service runstates. These
tracepoints are then used in analyses in Trace Compass.

Signed-off-by: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
---
 .mailmap                                 |  1 +
 doc/guides/prog_guide/service_cores.rst  | 18 +++++++
 lib/eal/common/eal_common_thread.c       |  4 ++
 lib/eal/common/eal_common_trace_points.c | 21 +++++++++
 lib/eal/common/rte_service.c             | 17 ++++++-
 lib/eal/include/eal_trace_internal.h     | 60 ++++++++++++++++++++++++
 6 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 0859104404..6453cb2aa5 100644
--- a/.mailmap
+++ b/.mailmap
@@ -120,6 +120,7 @@ Archana Muniganti <marchana@marvell.com> <muniganti.archana@caviumnetworks.com>
 Archit Pandey <architpandeynitk@gmail.com>
 Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
 Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
+Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
 Arnon Warshavsky <arnon@qwilt.com>
 Arshdeep Kaur <arshdeep.kaur@intel.com>
 Artem V. Andreev <artem.andreev@oktetlabs.ru>
diff --git a/doc/guides/prog_guide/service_cores.rst b/doc/guides/prog_guide/service_cores.rst
index 270b875783..9ef6580128 100644
--- a/doc/guides/prog_guide/service_cores.rst
+++ b/doc/guides/prog_guide/service_cores.rst
@@ -52,3 +52,21 @@ The service core library is capable of collecting runtime statistics like number
 of calls to a specific service, and number of cycles used by the service. The
 cycle count collection is dynamically configurable, allowing any application to
 profile the services running on the system at any time.
+
+Service Core Tracing
+~~~~~~~~~~~~~~~~~~~~
+
+The service core library is instrumented with tracepoints using the DPDK Trace
+Library. These tracepoints allow you to track the service and logical cores
+state. The tracepoints that are on the fast path are compiled out by default.
+To activate the fast path tracepoints, it is necessary to add the
+``enable_trace_fp`` option when building DPDK using meson.
+
+To activate tracing when launching a DPDK program it is necessary to use the
+``--trace`` option to specify a regular expression to select which tracepoints
+to enable. Here is an example if you want to only specify service core tracing::
+
+    ./dpdk/examples/service_cores/build/service_cores --trace="lib.eal.thread*" --trace="lib.eal.service*"
+
+See the `DPDK Trace Library <https://doc.dpdk.org/guides/prog_guide/trace_lib.html>`_
+for details.
diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
index 079a385630..25dbdd68e3 100644
--- a/lib/eal/common/eal_common_thread.c
+++ b/lib/eal/common/eal_common_thread.c
@@ -205,6 +205,8 @@ eal_thread_loop(void *arg)
 				__ATOMIC_ACQUIRE)) == NULL)
 			rte_pause();
 
+		rte_eal_trace_thread_lcore_running(lcore_id, f);
+
 		/* call the function and store the return value */
 		fct_arg = lcore_config[lcore_id].arg;
 		ret = f(fct_arg);
@@ -219,6 +221,8 @@ eal_thread_loop(void *arg)
 		 */
 		__atomic_store_n(&lcore_config[lcore_id].state, WAIT,
 			__ATOMIC_RELEASE);
+
+		rte_eal_trace_thread_lcore_stopped(lcore_id);
 	}
 
 	/* never reached */
diff --git a/lib/eal/common/eal_common_trace_points.c b/lib/eal/common/eal_common_trace_points.c
index 3f5bf5c55c..0f1240ea3a 100644
--- a/lib/eal/common/eal_common_trace_points.c
+++ b/lib/eal/common/eal_common_trace_points.c
@@ -70,6 +70,27 @@ RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_remote_launch,
 	lib.eal.thread.remote.launch)
 RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_ready,
 	lib.eal.thread.lcore.ready)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_running,
+	lib.eal.thread.lcore.running)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_stopped,
+	lib.eal.thread.lcore.stopped)
+
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_map_lcore,
+	lib.eal.service.map.lcore)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_state_change,
+	lib.eal.service.lcore.state.change)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_start,
+	lib.eal.service.lcore.start)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_stop,
+	lib.eal.service.lcore.stop)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_run_begin,
+	lib.eal.service.run.begin)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_runstate_set,
+	lib.eal.service.run.state.set)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_run_end,
+	lib.eal.service.run.end)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_component_register,
+	lib.eal.service.component.register)
 
 RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_register,
 	lib.eal.intr.register)
diff --git a/lib/eal/common/rte_service.c b/lib/eal/common/rte_service.c
index 7ab48f2be8..94e872a08a 100644
--- a/lib/eal/common/rte_service.c
+++ b/lib/eal/common/rte_service.c
@@ -9,6 +9,7 @@
 #include <rte_service.h>
 #include <rte_service_component.h>
 
+#include <eal_trace_internal.h>
 #include <rte_lcore.h>
 #include <rte_branch_prediction.h>
 #include <rte_common.h>
@@ -16,6 +17,7 @@
 #include <rte_atomic.h>
 #include <rte_malloc.h>
 #include <rte_spinlock.h>
+#include <rte_trace_point.h>
 
 #include "eal_private.h"
 
@@ -276,6 +278,8 @@ rte_service_component_register(const struct rte_service_spec *spec,
 	if (id_ptr)
 		*id_ptr = free_slot;
 
+	rte_eal_trace_service_component_register(free_slot, spec->name);
+
 	return 0;
 }
 
@@ -336,6 +340,7 @@ rte_service_runstate_set(uint32_t id, uint32_t runstate)
 		__atomic_store_n(&s->app_runstate, RUNSTATE_STOPPED,
 			__ATOMIC_RELEASE);
 
+	rte_eal_trace_service_runstate_set(id, runstate);
 	return 0;
 }
 
@@ -368,6 +373,7 @@ static inline void
 service_runner_do_callback(struct rte_service_spec_impl *s,
 			   struct core_state *cs, uint32_t service_idx)
 {
+	rte_eal_trace_service_run_begin(service_idx, rte_lcore_id());
 	void *userdata = s->spec.callback_userdata;
 
 	if (service_stats_enabled(s)) {
@@ -395,8 +401,10 @@ service_runner_do_callback(struct rte_service_spec_impl *s,
 
 		__atomic_store_n(&service_stats->calls,
 			service_stats->calls + 1, __ATOMIC_RELAXED);
-	} else
+	} else {
 		s->spec.callback(userdata);
+	}
+	rte_eal_trace_service_run_end(service_idx, rte_lcore_id());
 }
 
 
@@ -658,6 +666,7 @@ int32_t
 rte_service_map_lcore_set(uint32_t id, uint32_t lcore, uint32_t enabled)
 {
 	uint32_t on = enabled > 0;
+	rte_eal_trace_service_map_lcore(id, lcore, enabled);
 	return service_update(id, lcore, &on, 0);
 }
 
@@ -683,6 +692,8 @@ set_lcore_state(uint32_t lcore, int32_t state)
 
 	/* update per-lcore optimized state tracking */
 	lcore_states[lcore].is_service_core = (state == ROLE_SERVICE);
+
+	rte_eal_trace_service_lcore_state_change(lcore, state);
 }
 
 int32_t
@@ -780,6 +791,8 @@ rte_service_lcore_start(uint32_t lcore)
 	 */
 	__atomic_store_n(&cs->runstate, RUNSTATE_RUNNING, __ATOMIC_RELEASE);
 
+	rte_eal_trace_service_lcore_start(lcore);
+
 	int ret = rte_eal_remote_launch(service_runner_func, 0, lcore);
 	/* returns -EBUSY if the core is already launched, 0 on success */
 	return ret;
@@ -824,6 +837,8 @@ rte_service_lcore_stop(uint32_t lcore)
 	__atomic_store_n(&lcore_states[lcore].runstate, RUNSTATE_STOPPED,
 		__ATOMIC_RELEASE);
 
+	rte_eal_trace_service_lcore_stop(lcore);
+
 	return 0;
 }
 
diff --git a/lib/eal/include/eal_trace_internal.h b/lib/eal/include/eal_trace_internal.h
index 57d6de2535..9a37a08567 100644
--- a/lib/eal/include/eal_trace_internal.h
+++ b/lib/eal/include/eal_trace_internal.h
@@ -174,6 +174,66 @@ RTE_TRACE_POINT(
 	rte_trace_point_emit_u32(lcore_id);
 	rte_trace_point_emit_string(cpuset);
 )
+RTE_TRACE_POINT_FP(
+	rte_eal_trace_thread_lcore_running,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id, void *f),
+	rte_trace_point_emit_u32(lcore_id);
+	rte_trace_point_emit_ptr(f);
+)
+RTE_TRACE_POINT_FP(
+	rte_eal_trace_thread_lcore_stopped,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
+	rte_trace_point_emit_u32(lcore_id);
+)
+
+/* Service */
+RTE_TRACE_POINT(
+	rte_eal_trace_service_map_lcore,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id, unsigned int enabled),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(lcore_id);
+	rte_trace_point_emit_u32(enabled);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_lcore_state_change,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id, int lcore_state),
+	rte_trace_point_emit_u32(lcore_id);
+	rte_trace_point_emit_i32(lcore_state);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_lcore_start,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_lcore_stop,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT_FP(
+	rte_eal_trace_service_run_begin,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_runstate_set,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int run_state),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(run_state);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_run_end,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_component_register,
+	RTE_TRACE_POINT_ARGS(int id, const char *service_name),
+	rte_trace_point_emit_i32(id);
+	rte_trace_point_emit_string(service_name);
+)
 
 #ifdef __cplusplus
 }
-- 
2.25.1

Re: [PATCH v2] eal: add tracepoints to track lcores and services

[Stephen Hemminger]

On Thu, 11 May 2023 20:16:28 +0000
Arnaud Fiorini <arnaud.fiorini@polymtl.ca> wrote:

> +The service core library is instrumented with tracepoints using the DPDK Trace
> +Library. These tracepoints allow you to track the service and logical cores
> +state. The tracepoints that are on the fast path are compiled out by default.
> +To activate the fast path tracepoints, it is necessary to add the
> +``enable_trace_fp`` option when building DPDK using meson.
> +
> +To activate tracing when launching a DPDK program it is necessary to use the
> +``--trace`` option to specify a regular expression to select which tracepoints
> +to enable. Here is an example if you want to only specify service core tracing::
> +
> +    ./dpdk/examples/service_cores/build/service_cores --trace="lib.eal.thread*" --trace="lib.eal.service*"
> +

These state changes are not in the hot path. So they should always be available?

Re: [PATCH v2] eal: add tracepoints to track lcores and services

[Arnaud Fiorini]

On 2023-05-11 4:50 p.m., Stephen Hemminger wrote:
> On Thu, 11 May 2023 20:16:28 +0000
> Arnaud Fiorini <arnaud.fiorini@polymtl.ca> wrote:
>
>> +The service core library is instrumented with tracepoints using the DPDK Trace
>> +Library. These tracepoints allow you to track the service and logical cores
>> +state. The tracepoints that are on the fast path are compiled out by default.
>> +To activate the fast path tracepoints, it is necessary to add the
>> +``enable_trace_fp`` option when building DPDK using meson.
>> +
>> +To activate tracing when launching a DPDK program it is necessary to use the
>> +``--trace`` option to specify a regular expression to select which tracepoints
>> +to enable. Here is an example if you want to only specify service core tracing::
>> +
>> +    ./dpdk/examples/service_cores/build/service_cores --trace="lib.eal.thread*" --trace="lib.eal.service*"
>> +
> These state changes are not in the hot path. So they should always be available?

Some of the tracepoints (rte_eal_trace_thread_lcore_running, 
rte_eal_trace_thread_lcore_stopped, rte_eal_trace_service_run_begin) are 
defined as fast path tracepoints. If these changes do not need to be in 
the fast path, I can change their definition.

Re: [PATCH v2] eal: add tracepoints to track lcores and services

[Stephen Hemminger]

On Thu, 11 May 2023 17:01:48 -0400
Arnaud Fiorini <arnaud.fiorini@polymtl.ca> wrote:

> On 2023-05-11 4:50 p.m., Stephen Hemminger wrote:
> > On Thu, 11 May 2023 20:16:28 +0000
> > Arnaud Fiorini <arnaud.fiorini@polymtl.ca> wrote:
> >  
> >> +The service core library is instrumented with tracepoints using the DPDK Trace
> >> +Library. These tracepoints allow you to track the service and logical cores
> >> +state. The tracepoints that are on the fast path are compiled out by default.
> >> +To activate the fast path tracepoints, it is necessary to add the
> >> +``enable_trace_fp`` option when building DPDK using meson.
> >> +
> >> +To activate tracing when launching a DPDK program it is necessary to use the
> >> +``--trace`` option to specify a regular expression to select which tracepoints
> >> +to enable. Here is an example if you want to only specify service core tracing::
> >> +
> >> +    ./dpdk/examples/service_cores/build/service_cores --trace="lib.eal.thread*" --trace="lib.eal.service*"
> >> +  
> > These state changes are not in the hot path. So they should always be available?  
> 
> Some of the tracepoints (rte_eal_trace_thread_lcore_running, 
> rte_eal_trace_thread_lcore_stopped, rte_eal_trace_service_run_begin) are 
> defined as fast path tracepoints. If these changes do not need to be in 
> the fast path, I can change their definition.
> 

Only inside the loop itself, ie polling is really fast path.
State transitions are not.

[PATCH v3] eal: add tracepoints to track lcores and services

[Arnaud Fiorini]

The tracepoints added are used to track lcore role and status,
as well as service mapping and service runstates. These
tracepoints are then used in analyses in Trace Compass.

Signed-off-by: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
---
 .mailmap                                 |  1 +
 doc/guides/prog_guide/service_cores.rst  | 14 ++++++
 lib/eal/common/eal_common_thread.c       |  4 ++
 lib/eal/common/eal_common_trace_points.c | 21 +++++++++
 lib/eal/common/rte_service.c             | 17 ++++++-
 lib/eal/include/eal_trace_internal.h     | 60 ++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 0859104404..6453cb2aa5 100644
--- a/.mailmap
+++ b/.mailmap
@@ -120,6 +120,7 @@ Archana Muniganti <marchana@marvell.com> <muniganti.archana@caviumnetworks.com>
 Archit Pandey <architpandeynitk@gmail.com>
 Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
 Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
+Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
 Arnon Warshavsky <arnon@qwilt.com>
 Arshdeep Kaur <arshdeep.kaur@intel.com>
 Artem V. Andreev <artem.andreev@oktetlabs.ru>
diff --git a/doc/guides/prog_guide/service_cores.rst b/doc/guides/prog_guide/service_cores.rst
index 270b875783..3b9a947e4c 100644
--- a/doc/guides/prog_guide/service_cores.rst
+++ b/doc/guides/prog_guide/service_cores.rst
@@ -52,3 +52,17 @@ The service core library is capable of collecting runtime statistics like number
 of calls to a specific service, and number of cycles used by the service. The
 cycle count collection is dynamically configurable, allowing any application to
 profile the services running on the system at any time.
+
+Service Core Tracing
+~~~~~~~~~~~~~~~~~~~~
+
+The service core library is instrumented with tracepoints using the DPDK Trace
+Library. These tracepoints allow you to track the service and logical cores
+state. To activate tracing when launching a DPDK program it is necessary to use the
+``--trace`` option to specify a regular expression to select which tracepoints
+to enable. Here is an example if you want to only specify service core tracing::
+
+    ./dpdk/examples/service_cores/build/service_cores --trace="lib.eal.thread*" --trace="lib.eal.service*"
+
+See the `DPDK Trace Library <https://doc.dpdk.org/guides/prog_guide/trace_lib.html>`_
+for details.
diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
index 079a385630..25dbdd68e3 100644
--- a/lib/eal/common/eal_common_thread.c
+++ b/lib/eal/common/eal_common_thread.c
@@ -205,6 +205,8 @@ eal_thread_loop(void *arg)
 				__ATOMIC_ACQUIRE)) == NULL)
 			rte_pause();
 
+		rte_eal_trace_thread_lcore_running(lcore_id, f);
+
 		/* call the function and store the return value */
 		fct_arg = lcore_config[lcore_id].arg;
 		ret = f(fct_arg);
@@ -219,6 +221,8 @@ eal_thread_loop(void *arg)
 		 */
 		__atomic_store_n(&lcore_config[lcore_id].state, WAIT,
 			__ATOMIC_RELEASE);
+
+		rte_eal_trace_thread_lcore_stopped(lcore_id);
 	}
 
 	/* never reached */
diff --git a/lib/eal/common/eal_common_trace_points.c b/lib/eal/common/eal_common_trace_points.c
index 3f5bf5c55c..0f1240ea3a 100644
--- a/lib/eal/common/eal_common_trace_points.c
+++ b/lib/eal/common/eal_common_trace_points.c
@@ -70,6 +70,27 @@ RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_remote_launch,
 	lib.eal.thread.remote.launch)
 RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_ready,
 	lib.eal.thread.lcore.ready)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_running,
+	lib.eal.thread.lcore.running)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_stopped,
+	lib.eal.thread.lcore.stopped)
+
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_map_lcore,
+	lib.eal.service.map.lcore)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_state_change,
+	lib.eal.service.lcore.state.change)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_start,
+	lib.eal.service.lcore.start)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_stop,
+	lib.eal.service.lcore.stop)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_run_begin,
+	lib.eal.service.run.begin)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_runstate_set,
+	lib.eal.service.run.state.set)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_run_end,
+	lib.eal.service.run.end)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_component_register,
+	lib.eal.service.component.register)
 
 RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_register,
 	lib.eal.intr.register)
diff --git a/lib/eal/common/rte_service.c b/lib/eal/common/rte_service.c
index 7ab48f2be8..94e872a08a 100644
--- a/lib/eal/common/rte_service.c
+++ b/lib/eal/common/rte_service.c
@@ -9,6 +9,7 @@
 #include <rte_service.h>
 #include <rte_service_component.h>
 
+#include <eal_trace_internal.h>
 #include <rte_lcore.h>
 #include <rte_branch_prediction.h>
 #include <rte_common.h>
@@ -16,6 +17,7 @@
 #include <rte_atomic.h>
 #include <rte_malloc.h>
 #include <rte_spinlock.h>
+#include <rte_trace_point.h>
 
 #include "eal_private.h"
 
@@ -276,6 +278,8 @@ rte_service_component_register(const struct rte_service_spec *spec,
 	if (id_ptr)
 		*id_ptr = free_slot;
 
+	rte_eal_trace_service_component_register(free_slot, spec->name);
+
 	return 0;
 }
 
@@ -336,6 +340,7 @@ rte_service_runstate_set(uint32_t id, uint32_t runstate)
 		__atomic_store_n(&s->app_runstate, RUNSTATE_STOPPED,
 			__ATOMIC_RELEASE);
 
+	rte_eal_trace_service_runstate_set(id, runstate);
 	return 0;
 }
 
@@ -368,6 +373,7 @@ static inline void
 service_runner_do_callback(struct rte_service_spec_impl *s,
 			   struct core_state *cs, uint32_t service_idx)
 {
+	rte_eal_trace_service_run_begin(service_idx, rte_lcore_id());
 	void *userdata = s->spec.callback_userdata;
 
 	if (service_stats_enabled(s)) {
@@ -395,8 +401,10 @@ service_runner_do_callback(struct rte_service_spec_impl *s,
 
 		__atomic_store_n(&service_stats->calls,
 			service_stats->calls + 1, __ATOMIC_RELAXED);
-	} else
+	} else {
 		s->spec.callback(userdata);
+	}
+	rte_eal_trace_service_run_end(service_idx, rte_lcore_id());
 }
 
 
@@ -658,6 +666,7 @@ int32_t
 rte_service_map_lcore_set(uint32_t id, uint32_t lcore, uint32_t enabled)
 {
 	uint32_t on = enabled > 0;
+	rte_eal_trace_service_map_lcore(id, lcore, enabled);
 	return service_update(id, lcore, &on, 0);
 }
 
@@ -683,6 +692,8 @@ set_lcore_state(uint32_t lcore, int32_t state)
 
 	/* update per-lcore optimized state tracking */
 	lcore_states[lcore].is_service_core = (state == ROLE_SERVICE);
+
+	rte_eal_trace_service_lcore_state_change(lcore, state);
 }
 
 int32_t
@@ -780,6 +791,8 @@ rte_service_lcore_start(uint32_t lcore)
 	 */
 	__atomic_store_n(&cs->runstate, RUNSTATE_RUNNING, __ATOMIC_RELEASE);
 
+	rte_eal_trace_service_lcore_start(lcore);
+
 	int ret = rte_eal_remote_launch(service_runner_func, 0, lcore);
 	/* returns -EBUSY if the core is already launched, 0 on success */
 	return ret;
@@ -824,6 +837,8 @@ rte_service_lcore_stop(uint32_t lcore)
 	__atomic_store_n(&lcore_states[lcore].runstate, RUNSTATE_STOPPED,
 		__ATOMIC_RELEASE);
 
+	rte_eal_trace_service_lcore_stop(lcore);
+
 	return 0;
 }
 
diff --git a/lib/eal/include/eal_trace_internal.h b/lib/eal/include/eal_trace_internal.h
index 57d6de2535..09c354717f 100644
--- a/lib/eal/include/eal_trace_internal.h
+++ b/lib/eal/include/eal_trace_internal.h
@@ -174,6 +174,66 @@ RTE_TRACE_POINT(
 	rte_trace_point_emit_u32(lcore_id);
 	rte_trace_point_emit_string(cpuset);
 )
+RTE_TRACE_POINT(
+	rte_eal_trace_thread_lcore_running,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id, void *f),
+	rte_trace_point_emit_u32(lcore_id);
+	rte_trace_point_emit_ptr(f);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_thread_lcore_stopped,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
+	rte_trace_point_emit_u32(lcore_id);
+)
+
+/* Service */
+RTE_TRACE_POINT(
+	rte_eal_trace_service_map_lcore,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id, unsigned int enabled),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(lcore_id);
+	rte_trace_point_emit_u32(enabled);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_lcore_state_change,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id, int lcore_state),
+	rte_trace_point_emit_u32(lcore_id);
+	rte_trace_point_emit_i32(lcore_state);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_lcore_start,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_lcore_stop,
+	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_run_begin,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_runstate_set,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int run_state),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(run_state);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_run_end,
+	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id),
+	rte_trace_point_emit_u32(id);
+	rte_trace_point_emit_u32(lcore_id);
+)
+RTE_TRACE_POINT(
+	rte_eal_trace_service_component_register,
+	RTE_TRACE_POINT_ARGS(int id, const char *service_name),
+	rte_trace_point_emit_i32(id);
+	rte_trace_point_emit_string(service_name);
+)
 
 #ifdef __cplusplus
 }
-- 
2.25.1

Re: [PATCH v3] eal: add tracepoints to track lcores and services

[Arnaud Fiorini]

On 2023-05-12 10:30 a.m., Arnaud Fiorini wrote:
> The tracepoints added are used to track lcore role and status,
> as well as service mapping and service runstates. These
> tracepoints are then used in analyses in Trace Compass.
>
> Signed-off-by: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
> ---
>   .mailmap                                 |  1 +
>   doc/guides/prog_guide/service_cores.rst  | 14 ++++++
>   lib/eal/common/eal_common_thread.c       |  4 ++
>   lib/eal/common/eal_common_trace_points.c | 21 +++++++++
>   lib/eal/common/rte_service.c             | 17 ++++++-
>   lib/eal/include/eal_trace_internal.h     | 60 ++++++++++++++++++++++++
>   6 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/.mailmap b/.mailmap
> index 0859104404..6453cb2aa5 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -120,6 +120,7 @@ Archana Muniganti <marchana@marvell.com> <muniganti.archana@caviumnetworks.com>
>   Archit Pandey <architpandeynitk@gmail.com>
>   Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>   Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> +Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
>   Arnon Warshavsky <arnon@qwilt.com>
>   Arshdeep Kaur <arshdeep.kaur@intel.com>
>   Artem V. Andreev <artem.andreev@oktetlabs.ru>
> diff --git a/doc/guides/prog_guide/service_cores.rst b/doc/guides/prog_guide/service_cores.rst
> index 270b875783..3b9a947e4c 100644
> --- a/doc/guides/prog_guide/service_cores.rst
> +++ b/doc/guides/prog_guide/service_cores.rst
> @@ -52,3 +52,17 @@ The service core library is capable of collecting runtime statistics like number
>   of calls to a specific service, and number of cycles used by the service. The
>   cycle count collection is dynamically configurable, allowing any application to
>   profile the services running on the system at any time.
> +
> +Service Core Tracing
> +~~~~~~~~~~~~~~~~~~~~
> +
> +The service core library is instrumented with tracepoints using the DPDK Trace
> +Library. These tracepoints allow you to track the service and logical cores
> +state. To activate tracing when launching a DPDK program it is necessary to use the
> +``--trace`` option to specify a regular expression to select which tracepoints
> +to enable. Here is an example if you want to only specify service core tracing::
> +
> +    ./dpdk/examples/service_cores/build/service_cores --trace="lib.eal.thread*" --trace="lib.eal.service*"
> +
> +See the `DPDK Trace Library <https://doc.dpdk.org/guides/prog_guide/trace_lib.html>`_
> +for details.
> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index 079a385630..25dbdd68e3 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -205,6 +205,8 @@ eal_thread_loop(void *arg)
>   				__ATOMIC_ACQUIRE)) == NULL)
>   			rte_pause();
>   
> +		rte_eal_trace_thread_lcore_running(lcore_id, f);
> +
>   		/* call the function and store the return value */
>   		fct_arg = lcore_config[lcore_id].arg;
>   		ret = f(fct_arg);
> @@ -219,6 +221,8 @@ eal_thread_loop(void *arg)
>   		 */
>   		__atomic_store_n(&lcore_config[lcore_id].state, WAIT,
>   			__ATOMIC_RELEASE);
> +
> +		rte_eal_trace_thread_lcore_stopped(lcore_id);
>   	}
>   
>   	/* never reached */
> diff --git a/lib/eal/common/eal_common_trace_points.c b/lib/eal/common/eal_common_trace_points.c
> index 3f5bf5c55c..0f1240ea3a 100644
> --- a/lib/eal/common/eal_common_trace_points.c
> +++ b/lib/eal/common/eal_common_trace_points.c
> @@ -70,6 +70,27 @@ RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_remote_launch,
>   	lib.eal.thread.remote.launch)
>   RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_ready,
>   	lib.eal.thread.lcore.ready)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_running,
> +	lib.eal.thread.lcore.running)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_stopped,
> +	lib.eal.thread.lcore.stopped)
> +
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_map_lcore,
> +	lib.eal.service.map.lcore)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_state_change,
> +	lib.eal.service.lcore.state.change)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_start,
> +	lib.eal.service.lcore.start)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_lcore_stop,
> +	lib.eal.service.lcore.stop)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_run_begin,
> +	lib.eal.service.run.begin)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_runstate_set,
> +	lib.eal.service.run.state.set)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_run_end,
> +	lib.eal.service.run.end)
> +RTE_TRACE_POINT_REGISTER(rte_eal_trace_service_component_register,
> +	lib.eal.service.component.register)
>   
>   RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_register,
>   	lib.eal.intr.register)
> diff --git a/lib/eal/common/rte_service.c b/lib/eal/common/rte_service.c
> index 7ab48f2be8..94e872a08a 100644
> --- a/lib/eal/common/rte_service.c
> +++ b/lib/eal/common/rte_service.c
> @@ -9,6 +9,7 @@
>   #include <rte_service.h>
>   #include <rte_service_component.h>
>   
> +#include <eal_trace_internal.h>
>   #include <rte_lcore.h>
>   #include <rte_branch_prediction.h>
>   #include <rte_common.h>
> @@ -16,6 +17,7 @@
>   #include <rte_atomic.h>
>   #include <rte_malloc.h>
>   #include <rte_spinlock.h>
> +#include <rte_trace_point.h>
>   
>   #include "eal_private.h"
>   
> @@ -276,6 +278,8 @@ rte_service_component_register(const struct rte_service_spec *spec,
>   	if (id_ptr)
>   		*id_ptr = free_slot;
>   
> +	rte_eal_trace_service_component_register(free_slot, spec->name);
> +
>   	return 0;
>   }
>   
> @@ -336,6 +340,7 @@ rte_service_runstate_set(uint32_t id, uint32_t runstate)
>   		__atomic_store_n(&s->app_runstate, RUNSTATE_STOPPED,
>   			__ATOMIC_RELEASE);
>   
> +	rte_eal_trace_service_runstate_set(id, runstate);
>   	return 0;
>   }
>   
> @@ -368,6 +373,7 @@ static inline void
>   service_runner_do_callback(struct rte_service_spec_impl *s,
>   			   struct core_state *cs, uint32_t service_idx)
>   {
> +	rte_eal_trace_service_run_begin(service_idx, rte_lcore_id());
>   	void *userdata = s->spec.callback_userdata;
>   
>   	if (service_stats_enabled(s)) {
> @@ -395,8 +401,10 @@ service_runner_do_callback(struct rte_service_spec_impl *s,
>   
>   		__atomic_store_n(&service_stats->calls,
>   			service_stats->calls + 1, __ATOMIC_RELAXED);
> -	} else
> +	} else {
>   		s->spec.callback(userdata);
> +	}
> +	rte_eal_trace_service_run_end(service_idx, rte_lcore_id());
>   }
>   
>   
> @@ -658,6 +666,7 @@ int32_t
>   rte_service_map_lcore_set(uint32_t id, uint32_t lcore, uint32_t enabled)
>   {
>   	uint32_t on = enabled > 0;
> +	rte_eal_trace_service_map_lcore(id, lcore, enabled);
>   	return service_update(id, lcore, &on, 0);
>   }
>   
> @@ -683,6 +692,8 @@ set_lcore_state(uint32_t lcore, int32_t state)
>   
>   	/* update per-lcore optimized state tracking */
>   	lcore_states[lcore].is_service_core = (state == ROLE_SERVICE);
> +
> +	rte_eal_trace_service_lcore_state_change(lcore, state);
>   }
>   
>   int32_t
> @@ -780,6 +791,8 @@ rte_service_lcore_start(uint32_t lcore)
>   	 */
>   	__atomic_store_n(&cs->runstate, RUNSTATE_RUNNING, __ATOMIC_RELEASE);
>   
> +	rte_eal_trace_service_lcore_start(lcore);
> +
>   	int ret = rte_eal_remote_launch(service_runner_func, 0, lcore);
>   	/* returns -EBUSY if the core is already launched, 0 on success */
>   	return ret;
> @@ -824,6 +837,8 @@ rte_service_lcore_stop(uint32_t lcore)
>   	__atomic_store_n(&lcore_states[lcore].runstate, RUNSTATE_STOPPED,
>   		__ATOMIC_RELEASE);
>   
> +	rte_eal_trace_service_lcore_stop(lcore);
> +
>   	return 0;
>   }
>   
> diff --git a/lib/eal/include/eal_trace_internal.h b/lib/eal/include/eal_trace_internal.h
> index 57d6de2535..09c354717f 100644
> --- a/lib/eal/include/eal_trace_internal.h
> +++ b/lib/eal/include/eal_trace_internal.h
> @@ -174,6 +174,66 @@ RTE_TRACE_POINT(
>   	rte_trace_point_emit_u32(lcore_id);
>   	rte_trace_point_emit_string(cpuset);
>   )
> +RTE_TRACE_POINT(
> +	rte_eal_trace_thread_lcore_running,
> +	RTE_TRACE_POINT_ARGS(unsigned int lcore_id, void *f),
> +	rte_trace_point_emit_u32(lcore_id);
> +	rte_trace_point_emit_ptr(f);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_thread_lcore_stopped,
> +	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
> +	rte_trace_point_emit_u32(lcore_id);
> +)
> +
> +/* Service */
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_map_lcore,
> +	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id, unsigned int enabled),
> +	rte_trace_point_emit_u32(id);
> +	rte_trace_point_emit_u32(lcore_id);
> +	rte_trace_point_emit_u32(enabled);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_lcore_state_change,
> +	RTE_TRACE_POINT_ARGS(unsigned int lcore_id, int lcore_state),
> +	rte_trace_point_emit_u32(lcore_id);
> +	rte_trace_point_emit_i32(lcore_state);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_lcore_start,
> +	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
> +	rte_trace_point_emit_u32(lcore_id);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_lcore_stop,
> +	RTE_TRACE_POINT_ARGS(unsigned int lcore_id),
> +	rte_trace_point_emit_u32(lcore_id);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_run_begin,
> +	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id),
> +	rte_trace_point_emit_u32(id);
> +	rte_trace_point_emit_u32(lcore_id);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_runstate_set,
> +	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int run_state),
> +	rte_trace_point_emit_u32(id);
> +	rte_trace_point_emit_u32(run_state);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_run_end,
> +	RTE_TRACE_POINT_ARGS(unsigned int id, unsigned int lcore_id),
> +	rte_trace_point_emit_u32(id);
> +	rte_trace_point_emit_u32(lcore_id);
> +)
> +RTE_TRACE_POINT(
> +	rte_eal_trace_service_component_register,
> +	RTE_TRACE_POINT_ARGS(int id, const char *service_name),
> +	rte_trace_point_emit_i32(id);
> +	rte_trace_point_emit_string(service_name);
> +)
>   
>   #ifdef __cplusplus
>   }

Hi,

I added documentation, removed tracepoint duplication and remove the 
unnecessary use of RTE_TRACE_POINT_FP.

Are there any other changes needed for this patch ?

Re: [PATCH v3] eal: add tracepoints to track lcores and services

[David Marchand]

Hello,

On Fri, May 12, 2023 at 4:30 PM Arnaud Fiorini
<arnaud.fiorini@polymtl.ca> wrote:
>
> The tracepoints added are used to track lcore role and status,
> as well as service mapping and service runstates. These
> tracepoints are then used in analyses in Trace Compass.
>
> Signed-off-by: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>

Harry, the majority of trace points added by this patch are in the service code.
Could you review please?

Thanks.


-- 
David Marchand

RE: [PATCH v3] eal: add tracepoints to track lcores and services

[Van Haaren, Harry]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, June 13, 2023 9:39 AM
> To: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>; Van Haaren, Harry
> <harry.van.haaren@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob
> <jerinj@marvell.com>; Sunil Kumar Kori <skori@marvell.com>; dev@dpdk.org
> Subject: Re: [PATCH v3] eal: add tracepoints to track lcores and services
> 
> Hello,

Hi David,

> On Fri, May 12, 2023 at 4:30 PM Arnaud Fiorini
> <arnaud.fiorini@polymtl.ca> wrote:
> >
> > The tracepoints added are used to track lcore role and status,
> > as well as service mapping and service runstates. These
> > tracepoints are then used in analyses in Trace Compass.
> >
> > Signed-off-by: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
> 
> Harry, the majority of trace points added by this patch are in the service code.
> Could you review please?

Performance wise, this patch is the same as V1 comments, no concerns here just noting that enabling tracing impacts perf.
http://patches.dpdk.org/project/dpdk/patch/20230424152412.3784016-2-arnaud.fiorini@polymtl.ca/#159839

Docs look good, thanks for adding the --trace=  example, it will help users in future!

Acked-by: Harry van Haaren <harry.van.haaren@intel.com> 

Re: [PATCH v3] eal: add tracepoints to track lcores and services

[David Marchand]

On Fri, May 12, 2023 at 4:30 PM Arnaud Fiorini
<arnaud.fiorini@polymtl.ca> wrote:
>
> The tracepoints added are used to track lcore role and status,
> as well as service mapping and service runstates. These
> tracepoints are then used in analyses in Trace Compass.
>
> Signed-off-by: Arnaud Fiorini <arnaud.fiorini@polymtl.ca>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

I adjusted the reference to the trace library in the documentation
(see https://patchwork.dpdk.org/project/dpdk/patch/20230621080920.861541-1-david.marchand@redhat.com/
for an explanation).
And applied, thanks!


-- 
David Marchand