[dpdk-dev] [PATCH 3/8] trace: simplify trace point headers

[David Marchand <david.marchand@redhat.com>]

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>
---
 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 7feacfbabc..4891493687 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 <rte_trace_point_register.h>
 
 #include "test_trace.h"
 
diff --git a/doc/guides/prog_guide/trace_lib.rst b/doc/guides/prog_guide/trace_lib.rst
index 9cad4ff4ac..9bbfd165d8 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,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::
 
diff --git a/lib/librte_cryptodev/cryptodev_trace_points.c b/lib/librte_cryptodev/cryptodev_trace_points.c
index aa31103404..5d58951fd5 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 <rte_trace_point_register.h>
 
 #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 d1d8d1875c..292ec91bed 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 <rte_trace_point_register.h>
 
 #include <rte_eal_trace.h>
 
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 dbd648c054..942f458e7a 100644
--- a/lib/librte_eal/include/rte_trace_point.h
+++ b/lib/librte_eal/include/rte_trace_point.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
 }
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 <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_ */
diff --git a/lib/librte_eal/include/rte_trace_point_register.h b/lib/librte_eal/include/rte_trace_point_register.h
index 26e383a8bb..548fe4dab3 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 <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);
 
diff --git a/lib/librte_ethdev/ethdev_trace_points.c b/lib/librte_ethdev/ethdev_trace_points.c
index 5be377521c..2919409a15 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 <rte_trace_point_register.h>
 
 #include <rte_ethdev_trace.h>
 
diff --git a/lib/librte_eventdev/eventdev_trace_points.c b/lib/librte_eventdev/eventdev_trace_points.c
index 221a62b71c..1a0ccc4481 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 <rte_trace_point_register.h>
 
 #include "rte_eventdev_trace.h"
 
diff --git a/lib/librte_mempool/mempool_trace_points.c b/lib/librte_mempool/mempool_trace_points.c
index 3dac0bc536..4ad76deb34 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 <rte_trace_point_register.h>
 
 #include "rte_mempool_trace.h"
 
-- 
2.23.0