On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
>
> 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 <david.marchand@redhat.com>
>
> @@ -271,13 +258,128 @@ __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 <rte_trace_point_register.h>
> +#ifndef __DOXYGEN__
> +
> +#ifndef _RTE_TRACE_POINT_REGISTER_H_
> +#ifdef ALLOW_EXPERIMENTAL_API
> +
> +#include <rte_branch_prediction.h>
> +#include <rte_cycles.h>
> +#include <rte_per_lcore.h>
> +#include <rte_string_fns.h>
> +#include <rte_uuid.h>
I think, we can move this header file to the beginning of the file.
>
> -#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
Please add / *_RTE_TRACE_POINT_REGISTER_H_ */ for this endif
Please fix the below checkpatch warnings.
1) WARNING:LONG_LINE: line over 80 characters
#449: FILE: lib/librte_eal/include/rte_trace_point_register.h:9:
+#error for tracepoint registration, include this file first before
<rte_trace_point.h>
2) WARNING:LONG_LINE: line over 80 characters
#237: FILE: lib/librte_eal/include/rte_trace_point.h:337:
+ (__RTE_TRACE_EVENT_HEADER_ID_SHIFT -
__RTE_TRACE_FIELD_ID_SHIFT));
With the above change,
Acked-by: Jerin Jacob <jerinj@marvell.com>
@@ -1,7 +1,8 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright(C) 2020 Marvell International Ltd.
*/
-#define RTE_TRACE_POINT_REGISTER_SELECT
+
+#include <rte_trace_point_register.h>
#include "test_trace.h"
@@ -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,16 +96,15 @@ Register the tracepoint
.. code-block:: c
- /* Select tracepoint register macros */
- #define RTE_TRACE_POINT_REGISTER_SELECT
+ #include <rte_trace_point_register.h>
- #include <my_tracepoint_provider.h>
+ #include <my_tracepoint.h>
RTE_TRACE_POINT_REGISTER(app_trace_string, app.trace.string)
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
@@ -116,8 +115,8 @@ convention.
.. 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::
@@ -2,7 +2,7 @@
* Copyright(C) 2020 Marvell International Ltd.
*/
-#define RTE_TRACE_POINT_REGISTER_SELECT
+#include <rte_trace_point_register.h>
#include "rte_cryptodev_trace.h"
@@ -2,7 +2,7 @@
* Copyright(C) 2020 Marvell International Ltd.
*/
-#define RTE_TRACE_POINT_REGISTER_SELECT
+#include <rte_trace_point_register.h>
#include <rte_eal_trace.h>
@@ -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',
@@ -96,17 +96,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.
*
@@ -117,8 +106,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)
@@ -271,13 +258,128 @@ __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 <rte_trace_point_register.h>
+#ifndef __DOXYGEN__
+
+#ifndef _RTE_TRACE_POINT_REGISTER_H_
+#ifdef ALLOW_EXPERIMENTAL_API
+
+#include <rte_branch_prediction.h>
+#include <rte_cycles.h>
+#include <rte_per_lcore.h>
+#include <rte_string_fns.h>
+#include <rte_uuid.h>
+
+#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 <rte_trace_point_provider.h>
-#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
#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)
@@ -293,7 +395,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
}
deleted file mode 100644
@@ -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 <rte_trace_point.h> instead
-#endif
-
-#ifndef _RTE_TRACE_POINT_PROVIDER_H_
-#define _RTE_TRACE_POINT_PROVIDER_H_
-
-#ifdef ALLOW_EXPERIMENTAL_API
-
-#include <rte_branch_prediction.h>
-#include <rte_cycles.h>
-#include <rte_per_lcore.h>
-#include <rte_string_fns.h>
-#include <rte_uuid.h>
-
-#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_ */
@@ -2,14 +2,15 @@
* Copyright(C) 2020 Marvell International Ltd.
*/
-#ifndef _RTE_TRACE_POINT_H_
-#error do not include this file directly, use <rte_trace_point.h> instead
-#endif
-
#ifndef _RTE_TRACE_POINT_REGISTER_H_
#define _RTE_TRACE_POINT_REGISTER_H_
+#ifdef _RTE_TRACE_POINT_H_
+#error for tracepoint registration, include this file first before <rte_trace_point.h>
+#endif
+
#include <rte_per_lcore.h>
+#include <rte_trace_point.h>
RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
@@ -2,7 +2,7 @@
* Copyright(C) 2020 Marvell International Ltd.
*/
-#define RTE_TRACE_POINT_REGISTER_SELECT
+#include <rte_trace_point_register.h>
#include <rte_ethdev_trace.h>
@@ -2,7 +2,7 @@
* Copyright(C) 2020 Marvell International Ltd.
*/
-#define RTE_TRACE_POINT_REGISTER_SELECT
+#include <rte_trace_point_register.h>
#include "rte_eventdev_trace.h"
@@ -2,7 +2,7 @@
* Copyright(C) 2020 Marvell International Ltd.
*/
-#define RTE_TRACE_POINT_REGISTER_SELECT
+#include <rte_trace_point_register.h>
#include "rte_mempool_trace.h"