From patchwork Sun May 3 20:31:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 69680 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1DCBEA04AF; Sun, 3 May 2020 22:32:29 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D4F691D51F; Sun, 3 May 2020 22:32:22 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by dpdk.org (Postfix) with ESMTP id 269DD1D443 for ; Sun, 3 May 2020 22:32:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588537940; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3P05o+Cfa/MQE66lVH7a6KjhUYKn4Dpag2qhZuyHucM=; b=gWnP/Z4Lat7UoWByjYYeilR2nOXpaw4XhmnpKVW+p19quCQbcTu+TCwmprhLOxVlYUn29v zK1fMgvIq+OA1QKW8X9RahDSnd0pc5fBrztFGv3BfkknYx5qjG6srGXB2ER9jNlOjq8CkQ Nb2D9wRq3OmOKlvH1f60LpisM4EA0B4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-330-aN9ZVWLONWGnCwS7Lz1nlg-1; Sun, 03 May 2020 16:32:18 -0400 X-MC-Unique: aN9ZVWLONWGnCwS7Lz1nlg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 072E91895943; Sun, 3 May 2020 20:32:17 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.192.236]) by smtp.corp.redhat.com (Postfix) with ESMTP id 02AFE6FDB2; Sun, 3 May 2020 20:32:12 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: thomas@monjalon.net, Jerin Jacob , Sunil Kumar Kori , John McNamara , Marko Kovacevic , Declan Doherty , Ferruh Yigit , Andrew Rybchenko , Olivier Matz Date: Sun, 3 May 2020 22:31:30 +0200 Message-Id: <20200503203135.6493-4-david.marchand@redhat.com> In-Reply-To: <20200503203135.6493-1-david.marchand@redhat.com> References: <20200503203135.6493-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [dpdk-dev] [PATCH 3/8] trace: simplify trace point headers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 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 #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 - #include + #include 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 #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 #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 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 +#ifndef __DOXYGEN__ + +#ifndef _RTE_TRACE_POINT_REGISTER_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 -#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 #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 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 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 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 +#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 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 #include 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 #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 #include "rte_mempool_trace.h"