> -----Original Message-----
> From: David Marchand <david.march...@redhat.com>
> Sent: Tuesday, October 4, 2022 3:14 PM
> To: dev@dpdk.org
> Cc: sk...@mavell.com; Jerin Jacob Kollanukkaran <jer...@marvell.com>;
> sta...@dpdk.org; Sunil Kumar Kori <sk...@marvell.com>
> Subject: [EXT] [PATCH v2 5/9] trace: fix dynamically enabling trace points
>
> External Email
>
> ----------------------------------------------------------------------
> Enabling trace points at runtime was not working if no trace point had been
> enabled first at rte_eal_init() time. The reason was that trace.args reflected
> the arguments passed to --trace= EAL option.
>
> To fix this:
> - the trace subsystem initialisation is updated: trace directory
> creation is deferred to when traces are dumped (to avoid creating
> directories that may not be used),
> - per lcore memory allocation still relies on rte_trace_is_enabled() but
> this helper now tracks if any trace point is enabled. The
> documentation is updated accordingly,
> - cleanup helpers must always be called in rte_eal_cleanup() since some
> trace points might have been enabled and disabled in the lifetime of
> the DPDK application,
>
> With this fix, we can update the unit test and check that a trace point
> callback
> is invoked when expected.
>
> Note:
> - the 'trace' global variable might be shadowed with the argument
> passed to the functions dealing with trace point handles.
> 'tp' has been used for referring to trace_point object.
> Prefer 't' for referring to handles,
>
> Fixes: 84c4fae4628f ("trace: implement operation APIs")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> Changes since v1:
> - restored level to INFO for trace directory log message,
> - moved trace_mkdir() to rte_trace_save,
>
> ---
> app/test/test_trace.c | 20 ++++++++++
> app/test/test_trace.h | 2 +
> doc/guides/prog_guide/trace_lib.rst | 14 +++++--
> lib/eal/common/eal_common_trace.c | 53 ++++++++++---------------
> lib/eal/common/eal_common_trace_utils.c | 11 ++++-
> lib/eal/common/eal_trace.h | 3 +-
> 6 files changed, 65 insertions(+), 38 deletions(-)
>
[snip]
> diff --git a/lib/eal/common/eal_common_trace_utils.c
> b/lib/eal/common/eal_common_trace_utils.c
> index 2b55dbec65..7bf1c05e12 100644
> --- a/lib/eal/common/eal_common_trace_utils.c
> +++ b/lib/eal/common/eal_common_trace_utils.c
> @@ -314,14 +314,18 @@ trace_dir_default_path_get(char *dir_path)
> return 0;
> }
>
> -int
> +static int
> trace_mkdir(void)
> {
> struct trace *trace = trace_obj_get();
> char session[TRACE_DIR_STR_LEN];
> + static bool already_done;
> char *dir_path;
> int rc;
>
> + if (already_done)
> + return 0;
> +
As trace_mkdir() call is being moved to rte_trace_save() so there won't be
another context which will be invoking trace_mkdir().
So is this logic still needed here ?
> if (!trace->dir_offset) {
> dir_path = calloc(1, sizeof(trace->dir));
> if (dir_path == NULL) {
> @@ -365,6 +369,7 @@ trace_mkdir(void)
> }
>
> RTE_LOG(INFO, EAL, "Trace dir: %s\n", trace->dir);
> + already_done = true;
> return 0;
> }
>
> @@ -434,6 +439,10 @@ rte_trace_save(void)
> if (trace->nb_trace_mem_list == 0)
> return rc;
>
> + rc = trace_mkdir();
> + if (rc < 0)
> + return rc;
> +
> rc = trace_meta_save(trace);
> if (rc)
> return rc;
[snip]
> 2.37.3
