From: Mathieu Desnoyers via lttng-dev <lttng-dev@lists.lttng.org>
To: Norbert Lange <nolange79@gmail.com>
Cc: lttng-dev <lttng-dev@lists.lttng.org>
Subject: Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions
Date: Thu, 20 May 2021 10:19:12 -0400 (EDT) [thread overview]
Message-ID: <1600729511.52338.1621520352970.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20210520121807.55428-1-nolange79@gmail.com>
----- On May 20, 2021, at 8:18 AM, lttng-dev lttng-dev@lists.lttng.org wrote:
> Instead of creating functions for each loglevel, simply pass the
> callback as argument.
>
> Further pack all preprocessor information into a struct that
> the compiler already can prepare.
This introduces an ABI break too late in the cycle.
Also, I'm not so keen on adding an indirect call on the fast-path
when it's not absolutely needed.
What is wrong with having one symbol per loglevel ?
Thanks,
Mathieu
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> include/lttng/tracelog.h | 49 +++++--------
> src/lib/lttng-ust/tracelog.c | 130 +++++++++++++++--------------------
> 2 files changed, 75 insertions(+), 104 deletions(-)
>
> diff --git a/include/lttng/tracelog.h b/include/lttng/tracelog.h
> index e97c8275..cd5032e3 100644
> --- a/include/lttng/tracelog.h
> +++ b/include/lttng/tracelog.h
> @@ -14,51 +14,40 @@
> extern "C" {
> #endif
>
> -#define LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(level) \
> - extern void lttng_ust__tracelog_##level(const char *file, \
> - int line, const char *func, const char *fmt, ...) \
> - __attribute__ ((format(printf, 4, 5))); \
> - \
> - extern void lttng_ust__vtracelog_##level(const char *file, \
> - int line, const char *func, const char *fmt, \
> - va_list ap) \
> - __attribute__ ((format(printf, 4, 0)));
> -
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_EMERG);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_ALERT);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_CRIT);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_ERR);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_WARNING);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_NOTICE);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_INFO);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_SYSTEM);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_PROGRAM);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_PROCESS);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_MODULE);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_UNIT);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_FUNCTION);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_LINE);
> -LTTNG_UST_TP_TRACELOG_CB_TEMPLATE(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG);
> -
> -#undef LTTNG_UST_TP_TRACELOG_CB_TEMPLATE
> +struct lttng_ust__tracelog_sourceinfo {
> + const char *file;
> + const char *func;
> + int line;
> +};
> +
> +extern void lttng_ust__tracelog_printf(
> +
> __typeof__(lttng_ust_tracepoint_cb_lttng_ust_tracelog___LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG)
> *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source, const char *fmt, ...)
> + __attribute__ ((format(printf, 3, 4)));
> +
> +extern void lttng_ust__tracelog_vprintf(
> +
> __typeof__(lttng_ust_tracepoint_cb_lttng_ust_tracelog___LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG)
> *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source, const char *fmt, va_list
> ap)
> + __attribute__ ((format(printf, 3, 0)));
>
> #define lttng_ust_tracelog(level, fmt, ...) \
> do { \
> + static const struct lttng_ust__tracelog_sourceinfo src = { __FILE__,
> __func__, __LINE__ }; \
> LTTNG_UST_STAP_PROBEV(tracepoint_lttng_ust_tracelog, level, ## __VA_ARGS__); \
> if (caa_unlikely(lttng_ust_tracepoint_lttng_ust_tracelog___##level.state)) \
> - lttng_ust__tracelog_##level(__FILE__, __LINE__, __func__, \
> +
> lttng_ust__tracelog_printf(<tng_ust_tracepoint_cb_lttng_ust_tracelog___##level,
> &src, \
> fmt, ## __VA_ARGS__); \
> } while (0)
>
> #define lttng_ust_vtracelog(level, fmt, ap) \
> do { \
> + static const struct lttng_ust__tracelog_sourceinfo src = { __FILE__,
> __func__, __LINE__ }; \
> if (caa_unlikely(lttng_ust_tracepoint_lttng_ust_tracelog___##level.state)) \
> - lttng_ust__vtracelog_##level(__FILE__, __LINE__, __func__, \
> +
> lttng_ust__tracelog_vprintf(<tng_ust_tracepoint_cb_lttng_ust_tracelog___##level,
> &src, \
> fmt, ap); \
> } while (0)
>
> #if LTTNG_UST_COMPAT_API(0)
> -#define TP_TRACELOG_CB_TEMPLATE LTTNG_UST_TP_TRACELOG_CB_TEMPLATE
> #define tracelog lttng_ust_tracelog
> #define vtracelog lttng_ust_vtracelog
> #endif
> diff --git a/src/lib/lttng-ust/tracelog.c b/src/lib/lttng-ust/tracelog.c
> index 8147d7a3..b28c6c78 100644
> --- a/src/lib/lttng-ust/tracelog.c
> +++ b/src/lib/lttng-ust/tracelog.c
> @@ -15,78 +15,60 @@
> #define LTTNG_UST_TRACEPOINT_DEFINE
> #include "lttng-ust-tracelog-provider.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) \
> - __attribute__ ((format(printf, 4, 0))); \
> - \
> - void lttng_ust__vtracelog_##level(const char *file, \
> - int line, const char *func, \
> - const char *fmt, va_list ap); \
> - void lttng_ust__vtracelog_##level(const char *file, \
> - int line, const char *func, \
> - const char *fmt, va_list ap) \
> - { \
> - lttng_ust___vtracelog_##level(file, line, func, fmt, ap); \
> - } \
> - \
> - void lttng_ust__tracelog_##level(const char *file, \
> - int line, const char *func, \
> - const char *fmt, ...) \
> - __attribute__ ((format(printf, 4, 5))); \
> - \
> - void lttng_ust__tracelog_##level(const char *file, \
> - int line, const char *func, \
> - const char *fmt, ...); \
> - void lttng_ust__tracelog_##level(const char *file, \
> - 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); \
> - }
> +struct lttng_ust__tracelog_sourceinfo {
> + const char *file;
> + const char *func;
> + int line;
> +};
>
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_EMERG)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_ALERT)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_CRIT)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_ERR)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_WARNING)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_NOTICE)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_INFO)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_SYSTEM)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_PROGRAM)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_PROCESS)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_MODULE)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_UNIT)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_FUNCTION)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG_LINE)
> -LTTNG_UST_TRACELOG_CB(LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG)
> +typedef
> __typeof__(lttng_ust_tracepoint_cb_lttng_ust_tracelog___LTTNG_UST_TRACEPOINT_LOGLEVEL_DEBUG)
> tpcallback_t;
> +
> +extern void lttng_ust__tracelog_printf(tpcallback_t *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source, const char *fmt, ...)
> + __attribute__ ((format(printf, 3, 4)));
> +
> +extern void lttng_ust__tracelog_vprintf(tpcallback_t *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source, const char *fmt, va_list
> ap)
> + __attribute__ ((format(printf, 3, 0)));
> +
> +static inline
> +void lttng_ust___tracelog_vprintf(tpcallback_t *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source,
> + const char *fmt, va_list ap)
> + __attribute__((always_inline, format(printf, 3, 0)));
> +
> +
> +static inline
> +void lttng_ust___tracelog_vprintf(tpcallback_t *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source,
> + 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;
> + (*callback)(source->file, source->line, source->func, msg, len,
> + LTTNG_UST_CALLER_IP());
> + free(msg);
> +end:
> + return;
> +}
> +
> +
> +void lttng_ust__tracelog_printf(tpcallback_t *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source, const char *fmt, ...)
> +{
> + va_list ap;
> +
> + va_start(ap, fmt);
> + lttng_ust___tracelog_vprintf(callback, source, fmt, ap);
> + va_end(ap);
> +}
> +
> +void lttng_ust__tracelog_vprintf(tpcallback_t *callback,
> + const struct lttng_ust__tracelog_sourceinfo *source, const char *fmt, va_list
> ap)
> +{
> + lttng_ust___tracelog_vprintf(callback, source, fmt, ap);
> +}
> --
> 2.30.2
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
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-20 14:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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
2021-05-20 14:19 ` Mathieu Desnoyers via lttng-dev [this message]
2021-05-20 14:57 ` [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions Norbert Lange via lttng-dev
2021-05-20 15:21 ` Mathieu Desnoyers via lttng-dev
2021-05-20 15:54 ` Norbert Lange via lttng-dev
2021-05-20 16:25 ` Mathieu Desnoyers via lttng-dev
2021-05-20 16:51 ` Norbert Lange via lttng-dev
2021-05-20 17:18 ` Mathieu Desnoyers via lttng-dev
2021-05-20 17:43 ` Norbert Lange via lttng-dev
2021-05-21 14:55 ` Mathieu Desnoyers via lttng-dev
2021-05-25 13:32 ` 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=1600729511.52338.1621520352970.JavaMail.zimbra@efficios.com \
--to=lttng-dev@lists.lttng.org \
--cc=mathieu.desnoyers@efficios.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).