* [PATCH 0/1] eal: add tracepoints to track lcores and services @ 2023-04-24 15:24 Arnaud Fiorini 2023-04-24 15:24 ` [PATCH 1/1] " Arnaud Fiorini 0 siblings, 1 reply; 4+ messages in thread From: Arnaud Fiorini @ 2023-04-24 15:24 UTC (permalink / raw) Cc: dev, Arnaud Fiorini The events generated by these tracepoints are then used in Trace Compass[1] to show the lcore state and service state throughout the execution of a program. A trace has been generated using the service cores application and can be used to test the analysis. [1] https://git.eclipse.org/r/c/tracecompass.incubator/org.eclipse.tracecompass.incubator/+/200457 Arnaud Fiorini (1): eal: add tracepoints to track lcores and services .mailmap | 1 + lib/eal/common/eal_common_thread.c | 4 ++ lib/eal/common/eal_common_trace_points.c | 21 +++++++++ lib/eal/common/rte_service.c | 18 ++++++- lib/eal/include/eal_trace_internal.h | 60 ++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 1 deletion(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] eal: add tracepoints to track lcores and services 2023-04-24 15:24 [PATCH 0/1] eal: add tracepoints to track lcores and services Arnaud Fiorini @ 2023-04-24 15:24 ` Arnaud Fiorini 2023-05-03 10:14 ` Van Haaren, Harry 0 siblings, 1 reply; 4+ messages in thread From: Arnaud Fiorini @ 2023-04-24 15:24 UTC (permalink / raw) To: Thomas Monjalon, Jerin Jacob, Sunil Kumar Kori, Harry van Haaren Cc: dev, 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 + lib/eal/common/eal_common_thread.c | 4 ++ lib/eal/common/eal_common_trace_points.c | 21 +++++++++ lib/eal/common/rte_service.c | 18 ++++++- lib/eal/include/eal_trace_internal.h | 60 ++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index dc30369117..2a0b132572 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/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 42ca1d001d..5daec007aa 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; } @@ -427,11 +432,15 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, if (!rte_spinlock_trylock(&s->execute_lock)) return -EBUSY; + rte_eal_trace_service_run_begin(i, rte_lcore_id()); service_runner_do_callback(s, cs, i); rte_spinlock_unlock(&s->execute_lock); - } else + } else { + rte_eal_trace_service_run_begin(i, rte_lcore_id()); service_runner_do_callback(s, cs, i); + } + rte_eal_trace_service_run_end(i, rte_lcore_id()); return 0; } @@ -658,6 +667,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 +693,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 +792,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 +838,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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 1/1] eal: add tracepoints to track lcores and services 2023-04-24 15:24 ` [PATCH 1/1] " Arnaud Fiorini @ 2023-05-03 10:14 ` Van Haaren, Harry 2023-05-03 15:17 ` Mattias Rönnblom 0 siblings, 1 reply; 4+ messages in thread From: Van Haaren, Harry @ 2023-05-03 10:14 UTC (permalink / raw) To: Arnaud Fiorini, Thomas Monjalon, Jerin Jacob, Sunil Kumar Kori Cc: dev, mattias.ronnblom, Honnappa Nagarahalli > -----Original Message----- > From: Arnaud Fiorini <arnaud.fiorini@polymtl.ca> > Sent: Monday, April 24, 2023 4:24 PM > To: Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinj@marvell.com>; > Sunil Kumar Kori <skori@marvell.com>; Van Haaren, Harry > <harry.van.haaren@intel.com> > Cc: dev@dpdk.org; Arnaud Fiorini <arnaud.fiorini@polymtl.ca> > Subject: [PATCH 1/1] eal: add tracepoints to track lcores and services > > 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> I've had a look at these changes, and don't have any big objections; When disabled, the tracepoints add no measurable overhead here, but When enabled they cause a ~+50% cycle-cost (eg from ~100 to 150 cycles). Running the "service_perf_autotest" prints cycle-costs, so this is easy to check. Please add documentation on enabling the traces; today it is hard to identify/find an example of enabling tracing on service-cores. Suggest to add it to: 1) the commit message, how to enable service cores traces only 2) add a section in the service-cores documentation, to enable service-cores only, and link to tracing documentation page for more info. +CC Mattias and Honnappa who have expressed specific interest in service-cores Performance in the past, any concerns/input Mattias/Honnappa? @Arnaud, assuming no concerns from wider DPDK community, I'm happy to Ack a v2 with docs added. > --- > .mailmap | 1 + > lib/eal/common/eal_common_thread.c | 4 ++ > lib/eal/common/eal_common_trace_points.c | 21 +++++++++ > lib/eal/common/rte_service.c | 18 ++++++- > lib/eal/include/eal_trace_internal.h | 60 ++++++++++++++++++++++++ > 5 files changed, 103 insertions(+), 1 deletion(-) > > diff --git a/.mailmap b/.mailmap > index dc30369117..2a0b132572 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/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 42ca1d001d..5daec007aa 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; > } > > @@ -427,11 +432,15 @@ service_run(uint32_t i, struct core_state *cs, uint64_t > service_mask, > if (!rte_spinlock_trylock(&s->execute_lock)) > return -EBUSY; > > + rte_eal_trace_service_run_begin(i, rte_lcore_id()); > service_runner_do_callback(s, cs, i); > rte_spinlock_unlock(&s->execute_lock); > - } else > + } else { > + rte_eal_trace_service_run_begin(i, rte_lcore_id()); > service_runner_do_callback(s, cs, i); > + } > > + rte_eal_trace_service_run_end(i, rte_lcore_id()); > return 0; > } > > @@ -658,6 +667,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 +693,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 +792,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 +838,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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] eal: add tracepoints to track lcores and services 2023-05-03 10:14 ` Van Haaren, Harry @ 2023-05-03 15:17 ` Mattias Rönnblom 0 siblings, 0 replies; 4+ messages in thread From: Mattias Rönnblom @ 2023-05-03 15:17 UTC (permalink / raw) To: Van Haaren, Harry, Arnaud Fiorini, Thomas Monjalon, Jerin Jacob, Sunil Kumar Kori Cc: dev, Honnappa Nagarahalli On 2023-05-03 12:14, Van Haaren, Harry wrote: >> -----Original Message----- >> From: Arnaud Fiorini <arnaud.fiorini@polymtl.ca> >> Sent: Monday, April 24, 2023 4:24 PM >> To: Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinj@marvell.com>; >> Sunil Kumar Kori <skori@marvell.com>; Van Haaren, Harry >> <harry.van.haaren@intel.com> >> Cc: dev@dpdk.org; Arnaud Fiorini <arnaud.fiorini@polymtl.ca> >> Subject: [PATCH 1/1] eal: add tracepoints to track lcores and services >> >> 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> > > I've had a look at these changes, and don't have any big objections; > When disabled, the tracepoints add no measurable overhead here, but > When enabled they cause a ~+50% cycle-cost (eg from ~100 to 150 cycles). > Running the "service_perf_autotest" prints cycle-costs, so this is easy to check. > > Please add documentation on enabling the traces; today it is hard to identify/find > an example of enabling tracing on service-cores. Suggest to add it to: > 1) the commit message, how to enable service cores traces only > 2) add a section in the service-cores documentation, to enable service-cores only, and link to tracing documentation page for more info. > > +CC Mattias and Honnappa who have expressed specific interest in service-cores > Performance in the past, any concerns/input Mattias/Honnappa? > Adding tracepoints to DPDK services seems like a great idea to me. > @Arnaud, assuming no concerns from wider DPDK community, I'm happy to Ack a v2 with docs added. > > >> --- >> .mailmap | 1 + >> lib/eal/common/eal_common_thread.c | 4 ++ >> lib/eal/common/eal_common_trace_points.c | 21 +++++++++ >> lib/eal/common/rte_service.c | 18 ++++++- >> lib/eal/include/eal_trace_internal.h | 60 ++++++++++++++++++++++++ >> 5 files changed, 103 insertions(+), 1 deletion(-) >> >> diff --git a/.mailmap b/.mailmap >> index dc30369117..2a0b132572 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/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 42ca1d001d..5daec007aa 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; >> } >> >> @@ -427,11 +432,15 @@ service_run(uint32_t i, struct core_state *cs, uint64_t >> service_mask, >> if (!rte_spinlock_trylock(&s->execute_lock)) >> return -EBUSY; >> >> + rte_eal_trace_service_run_begin(i, rte_lcore_id()); Should you move the trace point to the runner function, to avoid duplication? >> service_runner_do_callback(s, cs, i); >> rte_spinlock_unlock(&s->execute_lock); >> - } else >> + } else { >> + rte_eal_trace_service_run_begin(i, rte_lcore_id()); >> service_runner_do_callback(s, cs, i); >> + } >> >> + rte_eal_trace_service_run_end(i, rte_lcore_id()); >> return 0; >> } >> >> @@ -658,6 +667,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 +693,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 +792,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 +838,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); There's a certain asymmetry here, with lcore_id being called both a uint32_t and unsigned int. I see now there is no rte_tracepoint_emit_uint(), so maybe this is the best you can do. Those two types are the same on anything DPDK supports anyway, I think, so it's a cosmetic issue only. >> + 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-03 15:17 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-24 15:24 [PATCH 0/1] eal: add tracepoints to track lcores and services Arnaud Fiorini 2023-04-24 15:24 ` [PATCH 1/1] " Arnaud Fiorini 2023-05-03 10:14 ` Van Haaren, Harry 2023-05-03 15:17 ` Mattias Rönnblom
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).