From: Norbert Lange via lttng-dev <lttng-dev@lists.lttng.org>
To: lttng-dev <lttng-dev@lists.lttng.org>
Subject: Re: [lttng-dev] [PATCH lttng-ust] Improve tracef/tracelog to use the stack for small strings
Date: Tue, 25 May 2021 15:47:03 +0200 [thread overview]
Message-ID: <CADYdroN-MQo2SsAL8msYddNnUXr1A0uR9eEasp8W-FV4bMM7Lw@mail.gmail.com> (raw)
In-Reply-To: <20210525134422.620425-1-nolange79@gmail.com>
Am Di., 25. Mai 2021 um 15:44 Uhr schrieb Norbert Lange <nolange79@gmail.com>:
>
> Support two common cases, one being that the resulting message is
> small enough to fit into a on-stack buffer.
> The seconds being the common 'printf("%s", "Message")' scheme.
>
> Unfortunately, iterating a va_list is destructive,
> so it has to be copied before calling vprintf.
>
> The implementation was moved to a separate file,
> used by both tracef.c and tracelog.c.
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> src/common/tracer.h | 2 +
> src/lib/lttng-ust-tracepoint/tracef.c | 32 ++-----
> .../lttng-ust-tracepoint/tracelog-internal.h | 83 +++++++++++++++++++
> src/lib/lttng-ust-tracepoint/tracelog.c | 40 ++-------
> 4 files changed, 102 insertions(+), 55 deletions(-)
> create mode 100644 src/lib/lttng-ust-tracepoint/tracelog-internal.h
>
> diff --git a/src/common/tracer.h b/src/common/tracer.h
> index 2affd6ab..8e18c9b5 100644
> --- a/src/common/tracer.h
> +++ b/src/common/tracer.h
> @@ -26,6 +26,8 @@
> #define LTTNG_RFLAG_EXTENDED RING_BUFFER_RFLAG_END
> #define LTTNG_RFLAG_END (LTTNG_RFLAG_EXTENDED << 1)
>
> +#define LTTNG_TRACE_PRINTF_BUFSIZE 512
> +
> /*
> * LTTng client type enumeration. Used by the consumer to map the
> * callbacks from its own address space.
> diff --git a/src/lib/lttng-ust-tracepoint/tracef.c b/src/lib/lttng-ust-tracepoint/tracef.c
> index c05c7811..92911e1d 100644
> --- a/src/lib/lttng-ust-tracepoint/tracef.c
> +++ b/src/lib/lttng-ust-tracepoint/tracef.c
> @@ -7,6 +7,7 @@
> #define _LGPL_SOURCE
> #include <stdio.h>
> #include "common/macros.h"
> +#include "common/tracer.h"
>
> /* The tracepoint definition is public, but the provider definition is hidden. */
> #define LTTNG_UST_TRACEPOINT_PROVIDER_HIDDEN_DEFINITION
> @@ -15,39 +16,22 @@
> #define LTTNG_UST_TRACEPOINT_DEFINE
> #include "lttng-ust-tracef-provider.h"
>
> -static inline
> -void lttng_ust___vtracef(const char *fmt, va_list ap)
> - __attribute__((always_inline, format(printf, 1, 0)));
> -static inline
> -void lttng_ust___vtracef(const char *fmt, va_list ap)
> -{
> - char *msg;
> - const int len = vasprintf(&msg, fmt, ap);
> -
> - /* len does not include the final \0 */
> - if (len < 0)
> - goto end;
> - lttng_ust_tracepoint_cb_lttng_ust_tracef___event(msg, len,
> - LTTNG_UST_CALLER_IP());
> - free(msg);
> -end:
> - return;
> -}
> +#include "tracelog-internal.h"
>
> void lttng_ust__vtracef(const char *fmt, va_list ap)
> __attribute__((format(printf, 1, 0)));
> void lttng_ust__vtracef(const char *fmt, va_list ap)
> {
> - lttng_ust___vtracef(fmt, ap);
> + LTTNG_UST_TRACELOG_VALIST(fmt, ap,
> + lttng_ust_tracepoint_cb_lttng_ust_tracef___event,
> + msg, len, LTTNG_UST_CALLER_IP());
> }
>
> void lttng_ust__tracef(const char *fmt, ...)
> __attribute__((format(printf, 1, 2)));
> void lttng_ust__tracef(const char *fmt, ...)
> {
> - va_list ap;
> -
> - va_start(ap, fmt);
> - lttng_ust___vtracef(fmt, ap);
> - va_end(ap);
> + LTTNG_UST_TRACELOG_VARARG(fmt,
> + lttng_ust_tracepoint_cb_lttng_ust_tracef___event,
> + msg, len, LTTNG_UST_CALLER_IP());
> }
> diff --git a/src/lib/lttng-ust-tracepoint/tracelog-internal.h b/src/lib/lttng-ust-tracepoint/tracelog-internal.h
> new file mode 100644
> index 00000000..291ff27c
> --- /dev/null
> +++ b/src/lib/lttng-ust-tracepoint/tracelog-internal.h
> @@ -0,0 +1,83 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright (C) 2013-2014 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> + * Copyright (C) 2021 Norbert Lange <nolange79@gmail.com>
> + *
> + * Shared helper macro for tracelog and tracef
> + */
> +
> +#define LTTNG_UST_TRACELOG_VARARG(fmt, callback, ...) \
> + va_list ap; \
> + char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE]; \
> + int len; \
> + char *msg = local_buf; \
> + size_t buflen = sizeof(local_buf); \
> + char *alloc_buff = NULL; \
> +\
> + if (caa_unlikely(fmt[0] == '%' && fmt[1] == 's' && fmt[2] == '\0')) { \
> + va_start(ap, fmt); \
> + msg = va_arg(ap, char *); \
> + va_end(ap); \
> + len = strlen(msg); \
> + } else \
> + for(;;) { \
> + va_start(ap, fmt); \
> + len = vsnprintf(msg, buflen, fmt, ap); \
> + va_end(ap); \
> +\
> + if (caa_unlikely(len >= (int)sizeof(local_buf) && !alloc_buff)) { \
> + buflen = (size_t)len + 1U; \
> + len = -1; \
> + alloc_buff = (char *)malloc(buflen); \
> + msg = alloc_buff; \
> + if (!alloc_buff) \
> + break; \
> + } else \
> + break; \
> + } \
> +\
> + /* len does not include the final \0 */ \
> + if (caa_likely(len >= 0)) { \
> + callback(__VA_ARGS__); \
> + } \
> + /* Dont call a potentially instrumented forbidden free needlessly. */ \
> + if (caa_unlikely(alloc_buff)) \
> + free(alloc_buff);
> +
> +#define LTTNG_UST_TRACELOG_VALIST(fmt, ap, callback, ...) \
> + char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE]; \
> + int len; \
> + char *msg = local_buf; \
> + size_t buflen = sizeof(local_buf); \
> + char *alloc_buff = NULL; \
> +\
> + if (caa_unlikely(fmt[0] == '%' && fmt[1] == 's' && fmt[2] == '\0')) { \
> + msg = va_arg(ap, char *); \
> + len = strlen(msg); \
> + } else \
> + for(;;) { \
> + va_list ap2; \
> +\
> + va_copy(ap2, ap); \
> + len = vsnprintf(msg, buflen, fmt, ap2); \
> + va_end(ap2); \
> +\
> + if (caa_unlikely(len >= (int)sizeof(local_buf) && !alloc_buff)) { \
> + buflen = (size_t)len + 1U; \
> + len = -1; \
> + alloc_buff = (char *)malloc(buflen); \
> + msg = alloc_buff; \
> + if (!alloc_buff) \
> + break; \
> + } else \
> + break; \
> + } \
> +\
> + /* len does not include the final \0 */ \
> + if (caa_likely(len >= 0)) { \
> + callback(__VA_ARGS__); \
> + } \
> + /* Dont call a potentially instrumented forbidden free needlessly. */ \
> + if (caa_unlikely(alloc_buff)) \
> + free(alloc_buff);
> diff --git a/src/lib/lttng-ust-tracepoint/tracelog.c b/src/lib/lttng-ust-tracepoint/tracelog.c
> index 8147d7a3..bd38032c 100644
> --- a/src/lib/lttng-ust-tracepoint/tracelog.c
> +++ b/src/lib/lttng-ust-tracepoint/tracelog.c
> @@ -7,6 +7,7 @@
> #define _LGPL_SOURCE
> #include <stdio.h>
> #include "common/macros.h"
> +#include "common/tracer.h"
>
> /* The tracepoint definition is public, but the provider definition is hidden. */
> #define LTTNG_UST_TRACEPOINT_PROVIDER_HIDDEN_DEFINITION
> @@ -15,32 +16,9 @@
> #define LTTNG_UST_TRACEPOINT_DEFINE
> #include "lttng-ust-tracelog-provider.h"
>
> +#include "tracelog-internal.h"
> +
> #define LTTNG_UST_TRACELOG_CB(level) \
> - static inline \
> - void lttng_ust___vtracelog_##level(const char *file, \
> - int line, const char *func, \
> - const char *fmt, va_list ap) \
> - __attribute__((always_inline, format(printf, 4, 0))); \
> - \
> - static inline \
> - void lttng_ust___vtracelog_##level(const char *file, \
> - int line, const char *func, \
> - const char *fmt, va_list ap) \
> - { \
> - char *msg; \
> - const int len = vasprintf(&msg, fmt, ap); \
> - \
> - /* len does not include the final \0 */ \
> - if (len < 0) \
> - goto end; \
> - lttng_ust_tracepoint_cb_lttng_ust_tracelog___##level(file, \
> - line, func, msg, len, \
> - LTTNG_UST_CALLER_IP()); \
> - free(msg); \
> - end: \
> - return; \
> - } \
> - \
> void lttng_ust__vtracelog_##level(const char *file, \
> int line, const char *func, \
> const char *fmt, va_list ap) \
> @@ -53,7 +31,9 @@
> int line, const char *func, \
> const char *fmt, va_list ap) \
> { \
> - lttng_ust___vtracelog_##level(file, line, func, fmt, ap); \
> + LTTNG_UST_TRACELOG_VALIST(fmt, ap, \
> + lttng_ust_tracepoint_cb_lttng_ust_tracelog___##level, \
> + file, line, func, msg, len, LTTNG_UST_CALLER_IP()); \
> } \
> \
> void lttng_ust__tracelog_##level(const char *file, \
> @@ -68,11 +48,9 @@
> int line, const char *func, \
> const char *fmt, ...) \
> { \
> - va_list ap; \
> - \
> - va_start(ap, fmt); \
> - lttng_ust___vtracelog_##level(file, line, func, fmt, ap); \
> - va_end(ap); \
> + LTTNG_UST_TRACELOG_VARARG(fmt, \
> + lttng_ust_tracepoint_cb_lttng_ust_tracelog___##level, \
> + file, line, func, msg, len, LTTNG_UST_CALLER_IP()); \
> }
>
> LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_EMERG)
> --
> 2.30.2
>
This is to be applied on top of:
https://review.lttng.org/c/lttng-ust/+/5927 Move tracef/tracelog
symbols to liblttng-ust-tracepoint.so
So consider this a WIP for feedback.
Norbert
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
next prev parent reply other threads:[~2021-05-25 13:47 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-25 13:44 [lttng-dev] [PATCH lttng-ust] Improve tracef/tracelog to use the stack for small strings Norbert Lange via lttng-dev
2021-05-25 13:47 ` Norbert Lange via lttng-dev [this message]
-- strict thread matches above, loose matches on Subject: below --
2021-05-20 12:18 [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions Norbert Lange via lttng-dev
2021-05-20 12:18 ` [lttng-dev] [PATCH lttng-ust] Improve tracef/tracelog to use the stack for small strings Norbert Lange via lttng-dev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADYdroN-MQo2SsAL8msYddNnUXr1A0uR9eEasp8W-FV4bMM7Lw@mail.gmail.com \
--to=lttng-dev@lists.lttng.org \
--cc=nolange79@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.