From ebaee64097027be82f2bdfbbf148a34f94f2b280 Mon Sep 17 00:00:00 2001 From: David Marchand Date: Wed, 29 Apr 2020 15:46:49 +0200 Subject: [PATCH] trace: simplify trace point headers Invert the current trace point headers logic by making rte_trace_point_register.h include rte_trace_point.h. There is no more need for a RTE_TRACE_POINT_REGISTER_SELECT special macro since including rte_trace_point_register.h itself means we want to register trace points. The unexplained "provider" notion is removed from the documentation and rte_trace_point_provider.h is merged into rte_trace_point.h. Signed-off-by: David Marchand Acked-by: Jerin Jacob --- app/test/test_trace_register.c | 3 +- doc/guides/prog_guide/trace_lib.rst | 19 ++- lib/librte_cryptodev/cryptodev_trace_points.c | 2 +- .../common/eal_common_trace_points.c | 2 +- lib/librte_eal/include/meson.build | 1 - lib/librte_eal/include/rte_trace_point.h | 140 +++++++++++++++--- .../include/rte_trace_point_provider.h | 131 ---------------- .../include/rte_trace_point_register.h | 9 +- lib/librte_ethdev/ethdev_trace_points.c | 2 +- lib/librte_eventdev/eventdev_trace_points.c | 2 +- lib/librte_mempool/mempool_trace_points.c | 2 +- 11 files changed, 142 insertions(+), 171 deletions(-) delete mode 100644 lib/librte_eal/include/rte_trace_point_provider.h diff --git a/app/test/test_trace_register.c b/app/test/test_trace_register.c index 8f40822cad..b9372d3f9b 100644 --- a/app/test/test_trace_register.c +++ b/app/test/test_trace_register.c @@ -1,7 +1,8 @@ /* SPDX-License-Identifier: BSD-3-Clause * Copyright(C) 2020 Marvell International Ltd. */ -#define RTE_TRACE_POINT_REGISTER_SELECT + +#include #include "test_trace.h" diff --git a/doc/guides/prog_guide/trace_lib.rst b/doc/guides/prog_guide/trace_lib.rst index 6a2016c7dc..b6c6285779 100644 --- a/doc/guides/prog_guide/trace_lib.rst +++ b/doc/guides/prog_guide/trace_lib.rst @@ -52,10 +52,10 @@ How to add a tracepoint? This section steps you through the details of adding a simple tracepoint. -.. _create_provider_header_file: +.. _create_tracepoint_header_file: -Create the tracepoint provider header file -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Create the tracepoint header file +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ .. code-block:: c @@ -96,10 +96,9 @@ Register the tracepoint .. code-block:: c - /* Select tracepoint register macros */ - #define RTE_TRACE_POINT_REGISTER_SELECT + #include - #include + #include RTE_TRACE_POINT_DEFINE(app_trace_string); @@ -109,8 +108,8 @@ Register the tracepoint } The above code snippet registers the ``app_trace_string`` tracepoint to -trace library. Here, the ``my_tracepoint_provider.h`` is the header file -that the user created in the first step :ref:`create_provider_header_file`. +trace library. Here, the ``my_tracepoint.h`` is the header file +that the user created in the first step :ref:`create_tracepoint_header_file`. The second argument for the ``RTE_TRACE_POINT_REGISTER`` is the name for the tracepoint. This string will be used for tracepoint lookup or regular @@ -124,8 +123,8 @@ The user can use the ``RTE_INIT`` construction scheme to achieve this. .. note:: - The ``RTE_TRACE_POINT_REGISTER_SELECT`` must be defined before including the - header for the tracepoint registration to work properly. + The ``rte_trace_point_register.h`` header must be included before any + inclusion of the ``rte_trace_point.h`` header. .. note:: diff --git a/lib/librte_cryptodev/cryptodev_trace_points.c b/lib/librte_cryptodev/cryptodev_trace_points.c index 7d03c93882..7672c7b99b 100644 --- a/lib/librte_cryptodev/cryptodev_trace_points.c +++ b/lib/librte_cryptodev/cryptodev_trace_points.c @@ -2,7 +2,7 @@ * Copyright(C) 2020 Marvell International Ltd. */ -#define RTE_TRACE_POINT_REGISTER_SELECT +#include #include "rte_cryptodev_trace.h" diff --git a/lib/librte_eal/common/eal_common_trace_points.c b/lib/librte_eal/common/eal_common_trace_points.c index 7611977a15..4a8ce90888 100644 --- a/lib/librte_eal/common/eal_common_trace_points.c +++ b/lib/librte_eal/common/eal_common_trace_points.c @@ -2,7 +2,7 @@ * Copyright(C) 2020 Marvell International Ltd. */ -#define RTE_TRACE_POINT_REGISTER_SELECT +#include #include diff --git a/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build index e9537c91f9..13315bfcec 100644 --- a/lib/librte_eal/include/meson.build +++ b/lib/librte_eal/include/meson.build @@ -43,7 +43,6 @@ headers += files( 'rte_time.h', 'rte_trace.h', 'rte_trace_point.h', - 'rte_trace_point_provider.h', 'rte_trace_point_register.h', 'rte_uuid.h', 'rte_version.h', diff --git a/lib/librte_eal/include/rte_trace_point.h b/lib/librte_eal/include/rte_trace_point.h index 4d956ec164..b45171275f 100644 --- a/lib/librte_eal/include/rte_trace_point.h +++ b/lib/librte_eal/include/rte_trace_point.h @@ -23,8 +23,13 @@ extern "C" { #include #include +#include #include #include +#include +#include +#include +#include /** The tracepoint object. */ typedef uint64_t rte_trace_point_t; @@ -100,17 +105,6 @@ _tp _args \ #ifdef __DOXYGEN__ -/** - * Macro to select rte_trace_point_emit_* definition for trace register function - * - * rte_trace_point_emit_* emits different definitions for trace function. - * Application must define RTE_TRACE_POINT_REGISTER_SELECT before including - * rte_trace_point.h in the C file where RTE_TRACE_POINT_REGISTER used. - * - * @see RTE_TRACE_POINT_REGISTER - */ -#define RTE_TRACE_POINT_REGISTER_SELECT - /** * Register a tracepoint. * @@ -121,8 +115,6 @@ _tp _args \ * @return * - 0: Successfully registered the tracepoint. * - <0: Failure to register the tracepoint. - * - * @see RTE_TRACE_POINT_REGISTER_SELECT */ #define RTE_TRACE_POINT_REGISTER(trace, name) @@ -275,13 +267,123 @@ __rte_experimental int __rte_trace_point_register(rte_trace_point_t *trace, const char *name, void (*register_fn)(void)); -#ifdef RTE_TRACE_POINT_REGISTER_SELECT -#include +#ifndef __DOXYGEN__ + +#ifndef _RTE_TRACE_POINT_REGISTER_H_ +#ifdef ALLOW_EXPERIMENTAL_API + +#define __RTE_TRACE_EVENT_HEADER_ID_SHIFT (48) + +#define __RTE_TRACE_FIELD_SIZE_SHIFT 0 +#define __RTE_TRACE_FIELD_SIZE_MASK (0xffffULL << __RTE_TRACE_FIELD_SIZE_SHIFT) +#define __RTE_TRACE_FIELD_ID_SHIFT (16) +#define __RTE_TRACE_FIELD_ID_MASK (0xffffULL << __RTE_TRACE_FIELD_ID_SHIFT) +#define __RTE_TRACE_FIELD_ENABLE_MASK (1ULL << 63) +#define __RTE_TRACE_FIELD_ENABLE_DISCARD (1ULL << 62) + +struct __rte_trace_stream_header { + uint32_t magic; + rte_uuid_t uuid; + uint32_t lcore_id; + char thread_name[__RTE_TRACE_EMIT_STRING_LEN_MAX]; +} __rte_packed; + +struct __rte_trace_header { + uint32_t offset; + uint32_t len; + struct __rte_trace_stream_header stream_header; + uint8_t mem[]; +}; + +RTE_DECLARE_PER_LCORE(void *, trace_mem); + +static __rte_always_inline void * +__rte_trace_mem_get(uint64_t in) +{ + struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem); + const uint16_t sz = in & __RTE_TRACE_FIELD_SIZE_MASK; + + /* Trace memory is not initialized for this thread */ + if (unlikely(trace == NULL)) { + __rte_trace_mem_per_thread_alloc(); + trace = RTE_PER_LCORE(trace_mem); + if (unlikely(trace == NULL)) + return NULL; + } + /* Check the wrap around case */ + uint32_t offset = trace->offset; + if (unlikely((offset + sz) >= trace->len)) { + /* Disable the trace event if it in DISCARD mode */ + if (unlikely(in & __RTE_TRACE_FIELD_ENABLE_DISCARD)) + return NULL; + + offset = 0; + } + /* Align to event header size */ + offset = RTE_ALIGN_CEIL(offset, __RTE_TRACE_EVENT_HEADER_SZ); + void *mem = RTE_PTR_ADD(&trace->mem[0], offset); + offset += sz; + trace->offset = offset; + + return mem; +} + +static __rte_always_inline void * +__rte_trace_point_emit_ev_header(void *mem, uint64_t in) +{ + uint64_t val; + + /* Event header [63:0] = id [63:48] | timestamp [47:0] */ + val = rte_get_tsc_cycles() & + ~(0xffffULL << __RTE_TRACE_EVENT_HEADER_ID_SHIFT); + val |= ((in & __RTE_TRACE_FIELD_ID_MASK) << + (__RTE_TRACE_EVENT_HEADER_ID_SHIFT - + __RTE_TRACE_FIELD_ID_SHIFT)); + + *(uint64_t *)mem = val; + return RTE_PTR_ADD(mem, __RTE_TRACE_EVENT_HEADER_SZ); +} + +#define __rte_trace_point_emit_header_generic(t) \ +void *mem; \ +do { \ + const uint64_t val = __atomic_load_n(t, __ATOMIC_ACQUIRE); \ + if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK))) \ + return; \ + mem = __rte_trace_mem_get(val); \ + if (unlikely(mem == NULL)) \ + return; \ + mem = __rte_trace_point_emit_ev_header(mem, val); \ +} while (0) + +#define __rte_trace_point_emit_header_fp(t) \ + if (!__rte_trace_point_fp_is_enabled()) \ + return; \ + __rte_trace_point_emit_header_generic(t) + +#define __rte_trace_point_emit(in, type) \ +do { \ + memcpy(mem, &(in), sizeof(in)); \ + mem = RTE_PTR_ADD(mem, sizeof(in)); \ +} while (0) + +#define rte_trace_point_emit_string(in) \ +do { \ + if (unlikely(in == NULL)) \ + return; \ + rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \ + mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \ +} while (0) + #else -#include -#endif -#ifndef __DOXYGEN__ +#define __rte_trace_point_emit_header_generic(t) RTE_SET_USED(t) +#define __rte_trace_point_emit_header_fp(t) RTE_SET_USED(t) +#define __rte_trace_point_emit(in, type) RTE_SET_USED(in) +#define rte_trace_point_emit_string(in) RTE_SET_USED(in) + +#endif /* ALLOW_EXPERIMENTAL_API */ +#endif /* _RTE_TRACE_POINT_REGISTER_H_ */ #define rte_trace_point_emit_u64(in) __rte_trace_point_emit(in, uint64_t) #define rte_trace_point_emit_i64(in) __rte_trace_point_emit(in, int64_t) @@ -297,7 +399,7 @@ int __rte_trace_point_register(rte_trace_point_t *trace, const char *name, #define rte_trace_point_emit_double(in) __rte_trace_point_emit(in, double) #define rte_trace_point_emit_ptr(in) __rte_trace_point_emit(in, uintptr_t) -#endif +#endif /* __DOXYGEN__ */ #ifdef __cplusplus } diff --git a/lib/librte_eal/include/rte_trace_point_provider.h b/lib/librte_eal/include/rte_trace_point_provider.h deleted file mode 100644 index bf53282f38..0000000000 --- a/lib/librte_eal/include/rte_trace_point_provider.h +++ /dev/null @@ -1,131 +0,0 @@ -/* SPDX-License-Identifier: BSD-3-Clause - * Copyright(C) 2020 Marvell International Ltd. - */ - -#ifndef _RTE_TRACE_POINT_H_ -#error do not include this file directly, use instead -#endif - -#ifndef _RTE_TRACE_POINT_PROVIDER_H_ -#define _RTE_TRACE_POINT_PROVIDER_H_ - -#ifdef ALLOW_EXPERIMENTAL_API - -#include -#include -#include -#include -#include - -#define __RTE_TRACE_EVENT_HEADER_ID_SHIFT (48) - -#define __RTE_TRACE_FIELD_SIZE_SHIFT 0 -#define __RTE_TRACE_FIELD_SIZE_MASK (0xffffULL << __RTE_TRACE_FIELD_SIZE_SHIFT) -#define __RTE_TRACE_FIELD_ID_SHIFT (16) -#define __RTE_TRACE_FIELD_ID_MASK (0xffffULL << __RTE_TRACE_FIELD_ID_SHIFT) -#define __RTE_TRACE_FIELD_ENABLE_MASK (1ULL << 63) -#define __RTE_TRACE_FIELD_ENABLE_DISCARD (1ULL << 62) - -struct __rte_trace_stream_header { - uint32_t magic; - rte_uuid_t uuid; - uint32_t lcore_id; - char thread_name[__RTE_TRACE_EMIT_STRING_LEN_MAX]; -} __rte_packed; - -struct __rte_trace_header { - uint32_t offset; - uint32_t len; - struct __rte_trace_stream_header stream_header; - uint8_t mem[]; -}; - -RTE_DECLARE_PER_LCORE(void *, trace_mem); - -static __rte_always_inline void * -__rte_trace_mem_get(uint64_t in) -{ - struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem); - const uint16_t sz = in & __RTE_TRACE_FIELD_SIZE_MASK; - - /* Trace memory is not initialized for this thread */ - if (unlikely(trace == NULL)) { - __rte_trace_mem_per_thread_alloc(); - trace = RTE_PER_LCORE(trace_mem); - if (unlikely(trace == NULL)) - return NULL; - } - /* Check the wrap around case */ - uint32_t offset = trace->offset; - if (unlikely((offset + sz) >= trace->len)) { - /* Disable the trace event if it in DISCARD mode */ - if (unlikely(in & __RTE_TRACE_FIELD_ENABLE_DISCARD)) - return NULL; - - offset = 0; - } - /* Align to event header size */ - offset = RTE_ALIGN_CEIL(offset, __RTE_TRACE_EVENT_HEADER_SZ); - void *mem = RTE_PTR_ADD(&trace->mem[0], offset); - offset += sz; - trace->offset = offset; - - return mem; -} - -static __rte_always_inline void * -__rte_trace_point_emit_ev_header(void *mem, uint64_t in) -{ - uint64_t val; - - /* Event header [63:0] = id [63:48] | timestamp [47:0] */ - val = rte_get_tsc_cycles() & - ~(0xffffULL << __RTE_TRACE_EVENT_HEADER_ID_SHIFT); - val |= ((in & __RTE_TRACE_FIELD_ID_MASK) << - (__RTE_TRACE_EVENT_HEADER_ID_SHIFT - __RTE_TRACE_FIELD_ID_SHIFT)); - - *(uint64_t *)mem = val; - return RTE_PTR_ADD(mem, __RTE_TRACE_EVENT_HEADER_SZ); -} - -#define __rte_trace_point_emit_header_generic(t) \ -void *mem; \ -do { \ - const uint64_t val = __atomic_load_n(t, __ATOMIC_ACQUIRE); \ - if (likely(!(val & __RTE_TRACE_FIELD_ENABLE_MASK))) \ - return; \ - mem = __rte_trace_mem_get(val); \ - if (unlikely(mem == NULL)) \ - return; \ - mem = __rte_trace_point_emit_ev_header(mem, val); \ -} while (0) - -#define __rte_trace_point_emit_header_fp(t) \ - if (!__rte_trace_point_fp_is_enabled()) \ - return; \ - __rte_trace_point_emit_header_generic(t) - -#define __rte_trace_point_emit(in, type) \ -do { \ - memcpy(mem, &(in), sizeof(in)); \ - mem = RTE_PTR_ADD(mem, sizeof(in)); \ -} while (0) - -#define rte_trace_point_emit_string(in) \ -do { \ - if (unlikely(in == NULL)) \ - return; \ - rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX); \ - mem = RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX); \ -} while (0) - -#else - -#define __rte_trace_point_emit_header_generic(t) RTE_SET_USED(t) -#define __rte_trace_point_emit_header_fp(t) RTE_SET_USED(t) -#define __rte_trace_point_emit(in, type) RTE_SET_USED(in) -#define rte_trace_point_emit_string(in) RTE_SET_USED(in) - -#endif - -#endif /* _RTE_TRACE_POINT_PROVIDER_H_ */ diff --git a/lib/librte_eal/include/rte_trace_point_register.h b/lib/librte_eal/include/rte_trace_point_register.h index 4e2306f1af..4e7c25ba10 100644 --- a/lib/librte_eal/include/rte_trace_point_register.h +++ b/lib/librte_eal/include/rte_trace_point_register.h @@ -2,14 +2,15 @@ * Copyright(C) 2020 Marvell International Ltd. */ -#ifndef _RTE_TRACE_POINT_H_ -#error do not include this file directly, use instead -#endif - #ifndef _RTE_TRACE_POINT_REGISTER_H_ #define _RTE_TRACE_POINT_REGISTER_H_ +#ifdef _RTE_TRACE_POINT_H_ +#error for registration, include this file first before +#endif + #include +#include RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz); diff --git a/lib/librte_ethdev/ethdev_trace_points.c b/lib/librte_ethdev/ethdev_trace_points.c index 05de34f3ca..722578c1c4 100644 --- a/lib/librte_ethdev/ethdev_trace_points.c +++ b/lib/librte_ethdev/ethdev_trace_points.c @@ -2,7 +2,7 @@ * Copyright(C) 2020 Marvell International Ltd. */ -#define RTE_TRACE_POINT_REGISTER_SELECT +#include #include diff --git a/lib/librte_eventdev/eventdev_trace_points.c b/lib/librte_eventdev/eventdev_trace_points.c index 2aa6e6bcf5..3c06087143 100644 --- a/lib/librte_eventdev/eventdev_trace_points.c +++ b/lib/librte_eventdev/eventdev_trace_points.c @@ -2,7 +2,7 @@ * Copyright(C) 2020 Marvell International Ltd. */ -#define RTE_TRACE_POINT_REGISTER_SELECT +#include #include "rte_eventdev_trace.h" diff --git a/lib/librte_mempool/mempool_trace_points.c b/lib/librte_mempool/mempool_trace_points.c index afab8dff68..df4368b173 100644 --- a/lib/librte_mempool/mempool_trace_points.c +++ b/lib/librte_mempool/mempool_trace_points.c @@ -2,7 +2,7 @@ * Copyright(C) 2020 Marvell International Ltd. */ -#define RTE_TRACE_POINT_REGISTER_SELECT +#include #include "rte_mempool_trace.h" -- 2.20.1