[lttng-dev] [PATCH v2] Improve tracef/tracelog to use the stack for small strings

Norbert Lange nolange79 at gmail.com
Mon Aug 1 10:44:02 EDT 2022


Am Mo., 1. Aug. 2022 um 16:35 Uhr schrieb Norbert Lange <nolange79 at gmail.com>:
>
> From: Norbert Lange <nolange79 at 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 at gmail.com>
> ---
> v2 -> v3:
>
> *   Wrap macro in do/while
> *   drop loop
> *   other changes in response to feedback
>
> v2:
> -   move define into src/common/tracer.h
>     see https://lists.lttng.org/pipermail/lttng-dev/2021-May/029977.html
> -   moved macro magic into common tracelog-internal.h header
> -   rebased onto master
> ---
>  src/common/tracer.h                   |  2 +
>  src/lib/lttng-ust/tracef.c            | 32 +++------
>  src/lib/lttng-ust/tracelog-internal.h | 95 +++++++++++++++++++++++++++
>  src/lib/lttng-ust/tracelog.c          | 40 +++--------
>  4 files changed, 114 insertions(+), 55 deletions(-)
>  create mode 100644 src/lib/lttng-ust/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/tracef.c b/src/lib/lttng-ust/tracef.c
> index c05c7811..92911e1d 100644
> --- a/src/lib/lttng-ust/tracef.c
> +++ b/src/lib/lttng-ust/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/tracelog-internal.h b/src/lib/lttng-ust/tracelog-internal.h
> new file mode 100644
> index 00000000..34537023
> --- /dev/null
> +++ b/src/lib/lttng-ust/tracelog-internal.h
> @@ -0,0 +1,95 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright (C) 2013-2014 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> + * Copyright (C) 2021 Norbert Lange <nolange79 at gmail.com>
> + *
> + * Shared helper macro for tracelog and tracef.
> + */
> +
> +#define LTTNG_UST_TRACELOG_VARARG(fmt, callback, ...) \
> +       do { \
> +               va_list ap; \
> +               char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE]; \
> +               size_t len = 0; \
> +               char *msg = 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 { \
> +                       int ret; \
> +                       size_t buflen = sizeof(local_buf); \
> +\
> +                       /* On-stack buffer attempt */ \
> +                       va_start(ap, fmt); \
> +                       ret = vsnprintf(msg, buflen, fmt, ap); \
> +                       va_end(ap); \
> +                       if (caa_unlikely(ret < 0)) \
> +                               break; \
> +                       len = (size_t)ret; \
> +\
> +                       if (caa_unlikely(len >= sizeof(local_buf))) { \
> +                               buflen = len + 1; \
> +                               alloc_buff = (char *)malloc(buflen); \
> +                               if (!alloc_buff) \
> +                                       goto end; \
> +                               msg = alloc_buff; \
> +                               va_start(ap, fmt); \
> +                               ret = vsnprintf(msg, buflen, fmt, ap); \
> +                               va_end(ap); \
> +                               lttng_ust_runtime_bug_on(ret < 0 || (size_t)ret != buflen - 1); \
> +                               len = (size_t)ret; \
> +                       } \
> +               } \
> +\
> +               callback(__VA_ARGS__); \
> +end: \
> +               /* Don't call a potentially instrumented forbidden free needlessly. */ \
> +               if (caa_unlikely(alloc_buff)) \
> +                       free(alloc_buff); \
> +       } while(0)
> +
> +#define LTTNG_UST_TRACELOG_VALIST(fmt, ap, callback, ...) \
> +       do { \
> +               char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE]; \
> +               size_t len = 0; \
> +               char *msg = 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 { \
> +                       va_list ap2; \
> +                       int ret; \
> +                       size_t buflen = sizeof(local_buf); \
> +\
> +                       va_copy(ap2, ap); \
> +                       ret = vsnprintf(msg, buflen, fmt, ap2); \
> +                       va_end(ap2); \
> +                       if (caa_unlikely(ret < 0)) \
> +                               break; \
> +                       len = (size_t)ret; \
> +\
> +                       if (caa_unlikely(len >= sizeof(local_buf))) { \
> +                               buflen = len + 1; \
> +                               alloc_buff = (char *)malloc(buflen); \
> +                               if (!alloc_buff) \
> +                                       goto end; \
> +                               msg = alloc_buff; \
> +                               ret = vsnprintf(msg, buflen, fmt, ap); \
> +                               lttng_ust_runtime_bug_on(ret < 0 || (size_t)ret != buflen - 1); \
> +                               len = (size_t)ret; \
> +                       } \
> +               } \
> +\
> +               callback(__VA_ARGS__); \
> +end: \
> +               /* Don't call a potentially instrumented forbidden free needlessly. */ \
> +               if (caa_unlikely(alloc_buff)) \
> +                       free(alloc_buff); \
> +       } while (0)
> diff --git a/src/lib/lttng-ust/tracelog.c b/src/lib/lttng-ust/tracelog.c
> index 8147d7a3..bd38032c 100644
> --- a/src/lib/lttng-ust/tracelog.c
> +++ b/src/lib/lttng-ust/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.35.1
>

Any chance of not having to link liblttng-ust for the tracelog/tracef functions,
like all other tracepoints (only hard dependency on liblttng-ust-tracepoint)?
My involvement is limited to "complain-only" for that one, but think of
how technically great that would be!

Norbert


More information about the lttng-dev mailing list