From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id yGICKOpvpmAqWwAAWB0awg (envelope-from ) for ; Thu, 20 May 2021 10:19:22 -0400 Received: by simark.ca (Postfix, from userid 112) id A1AC81F11C; Thu, 20 May 2021 10:19:22 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.lttng.org (lists.lttng.org [167.114.26.123]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id E1B5C1E54D for ; Thu, 20 May 2021 10:19:21 -0400 (EDT) Received: from lists-lttng01.efficios.com (localhost [IPv6:::1]) by lists.lttng.org (Postfix) with ESMTP id 4FmBgs49WLz1rRp; Thu, 20 May 2021 10:19:21 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=lists.lttng.org; s=default; t=1621520361; bh=YmzTdDSjM2u74IcgqurombXTO0OtOGL567j1XrpGOiw=; h=Date:To:Cc:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=XxwAXlRZ0ECHQBY6pJnvd/Igobnx2VuCWwM+TjiB9c3DGmwtKfAvkQ0ueyypDJYwE vmMEKfcK1hX4gcFia6WuYM2NnDKl7g8KoZcd9r+N5TrpkmmRbig60twQYdE+PXgk+c JL6VfLTxbA58mXzRmC7egcXK4N0060bUhHMyxAb5KADYMyx0LfVrkWNqA9HK5ti8p1 76xepjMfGVoc4Et0t+y8qFGHbKwnNpCQ+9gGThFFSZsg6tkhDmWDw6HCl6z51GobD9 zPb8qEbEH1OPLCTTvJ93ATEaCiyk8WwiqiabOXS0NDUWwF8wb2A4ZbobaP/3un+yWw bk2Aio0aLOFeQ== Received: from mail.efficios.com (mail.efficios.com [167.114.26.124]) by lists.lttng.org (Postfix) with ESMTPS id 4FmBgq6mc2z1rrR for ; Thu, 20 May 2021 10:19:19 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id A12AD335D57 for ; Thu, 20 May 2021 10:19:13 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 11Vj7u7w9Uhw; Thu, 20 May 2021 10:19:13 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 12256335D55; Thu, 20 May 2021 10:19:13 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 12256335D55 X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 9elMuluS1Jww; Thu, 20 May 2021 10:19:13 -0400 (EDT) Received: from mail03.efficios.com (mail03.efficios.com [167.114.26.124]) by mail.efficios.com (Postfix) with ESMTP id 06638336053; Thu, 20 May 2021 10:19:13 -0400 (EDT) Date: Thu, 20 May 2021 10:19:12 -0400 (EDT) To: Norbert Lange Cc: lttng-dev Message-ID: <1600729511.52338.1621520352970.JavaMail.zimbra@efficios.com> In-Reply-To: <20210520121807.55428-1-nolange79@gmail.com> References: <20210520121807.55428-1-nolange79@gmail.com> MIME-Version: 1.0 X-Originating-IP: [167.114.26.124] X-Mailer: Zimbra 8.8.15_GA_4018 (ZimbraWebClient - FF88 (Linux)/8.8.15_GA_4007) Thread-Topic: Improve tracelog handling, reduce exported functions Thread-Index: 95s96LKVRCKfehbKM7OcjWxVc88aTQ== Subject: Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions X-BeenThere: lttng-dev@lists.lttng.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: LTTng development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mathieu Desnoyers via lttng-dev Reply-To: Mathieu Desnoyers Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: lttng-dev-bounces@lists.lttng.org Sender: "lttng-dev" ----- 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 > --- > 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