[lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Norbert Lange via lttng-dev]

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.

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(&lttng_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(&lttng_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

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

[Norbert Lange via lttng-dev]

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.
This will result in the function never becoming inlined,
thus the helper function was manually "inlined".

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
 src/common/tracer.h          |  2 +
 src/lib/lttng-ust/tracef.c   | 83 ++++++++++++++++++++++++---------
 src/lib/lttng-ust/tracelog.c | 90 ++++++++++++++++++++++++------------
 3 files changed, 122 insertions(+), 53 deletions(-)

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..21af5b9e 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,30 +16,40 @@
 #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;
-}
-
 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);
+	char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE];
+	char *alloc_buff = NULL;
+	char *msg = local_buf;
+	size_t buflen = sizeof(local_buf);
+	int len = -1;
+
+	if (caa_unlikely(fmt[0] == '%' && fmt[1] == 's' && fmt[2] == '\0')) {
+		msg = va_arg(ap, char *);
+		len = strlen(msg);
+	} else
+		do {
+			va_list ap2;
+
+			if (caa_unlikely(len >= sizeof(local_buf))) {
+				buflen = (size_t)(len) + 1U;
+				alloc_buff = (char *)malloc(buflen);
+				msg = alloc_buff;
+				if (!alloc_buff)
+					return;
+			}
+			va_copy(ap2, ap);
+			len = vsnprintf(msg, buflen, fmt, ap2);
+			va_end(ap2);
+		} while (caa_unlikely(len >= sizeof(local_buf) && !alloc_buff));
+
+	/* len does not include the final \0 */
+	if (caa_likely(len >= 0))
+		lttng_ust_tracepoint_cb_lttng_ust_tracef___event(msg, len,
+			LTTNG_UST_CALLER_IP());
+	free(alloc_buff);
 }
 
 void lttng_ust__tracef(const char *fmt, ...)
@@ -46,8 +57,34 @@ void lttng_ust__tracef(const char *fmt, ...)
 void lttng_ust__tracef(const char *fmt, ...)
 {
 	va_list ap;
+	char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE];
+	char *alloc_buff = NULL;
+	char *msg = local_buf;
+	size_t buflen = sizeof(local_buf);
+	int len = -1;
 
-	va_start(ap, fmt);
-	lttng_ust___vtracef(fmt, ap);
-	va_end(ap);
+	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
+		do {
+			if (caa_unlikely(len >= sizeof(local_buf))) {
+				buflen = (size_t)(len) + 1U;
+				alloc_buff = (char *)malloc(buflen);
+				msg = alloc_buff;
+				if (!alloc_buff)
+					return;
+			}
+			va_start(ap, fmt);
+			len = vsnprintf(msg, buflen, fmt, ap);
+			va_end(ap);
+		} while (caa_unlikely(len >= sizeof(local_buf) && !alloc_buff));
+
+	/* len does not include the final \0 */
+	if (caa_likely(len >= 0))
+		lttng_ust_tracepoint_cb_lttng_ust_tracef___event(msg, len,
+			LTTNG_UST_CALLER_IP());
+	free(alloc_buff);
 }
diff --git a/src/lib/lttng-ust/tracelog.c b/src/lib/lttng-ust/tracelog.c
index b28c6c78..6889869c 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
@@ -31,44 +32,73 @@ 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;
+	char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE];
+	char *alloc_buff = NULL;
+	char *msg = local_buf;
+	size_t buflen = sizeof(local_buf);
+	int len = -1;
+
+	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
+		do {
+			if (caa_unlikely(len >= sizeof(local_buf))) {
+				buflen = (size_t)(len) + 1U;
+				alloc_buff = (char *)malloc(buflen);
+				msg = alloc_buff;
+				if (!alloc_buff)
+					return;
+			}
+			va_start(ap, fmt);
+			len = vsnprintf(msg, buflen, fmt, ap);
+			va_end(ap);
+		} while (caa_unlikely(len >= sizeof(local_buf) && !alloc_buff));
 
-	va_start(ap, fmt);
-	lttng_ust___tracelog_vprintf(callback, source, fmt, ap);
-	va_end(ap);
+	/* len does not include the final \0 */
+	if (caa_likely(len >= 0))
+		(*callback)(source->file, source->line, source->func, msg, len,
+			LTTNG_UST_CALLER_IP());
+	free(alloc_buff);
 }
 
 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);
+	char local_buf[LTTNG_TRACE_PRINTF_BUFSIZE];
+	char *alloc_buff = NULL;
+	char *msg = local_buf;
+	size_t buflen = sizeof(local_buf);
+	int len = -1;
+
+	if (caa_unlikely(fmt[0] == '%' && fmt[1] == 's' && fmt[2] == '\0')) {
+		msg = va_arg(ap, char *);
+		len = strlen(msg);
+	} else
+		do {
+			va_list ap2;
+
+			if (caa_unlikely(len >= sizeof(local_buf))) {
+				buflen = (size_t)(len) + 1U;
+				alloc_buff = (char *)malloc(buflen);
+				msg = alloc_buff;
+				if (!alloc_buff)
+					return;
+			}
+			va_copy(ap2, ap);
+			len = vsnprintf(msg, buflen, fmt, ap2);
+			va_end(ap2);
+		} while (caa_unlikely(len >= sizeof(local_buf) && !alloc_buff));
+
+	/* len does not include the final \0 */
+	if (caa_likely(len >= 0))
+		(*callback)(source->file, source->line, source->func, msg, len,
+			LTTNG_UST_CALLER_IP());
+	free(alloc_buff);
 }
-- 
2.30.2

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Mathieu Desnoyers via 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 <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(&lttng_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(&lttng_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

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Norbert Lange via lttng-dev]

Am Do., 20. Mai 2021 um 16:19 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.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.

So 2.14 would be the next chance I guess

> Also, I'm not so keen on adding an indirect call on the fast-path
> when it's not absolutely needed.

Code seems pretty similar: https://godbolt.org/z/oK1WhWqGT

> What is wrong with having one symbol per loglevel ?

Macro-magic is cumbersome to edit, more code, more relocations.

Easier to adapt aswell, could roll my own tracelog functions while
using lttng_ust__tracelog_printf (started soind that as I don't want
to link to lttng-ust.so)

Norbert
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Mathieu Desnoyers via lttng-dev]

----- On May 20, 2021, at 10:57 AM, Norbert Lange nolange79@gmail.com wrote:

> Am Do., 20. Mai 2021 um 16:19 Uhr schrieb Mathieu Desnoyers
> <mathieu.desnoyers@efficios.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.
> 
> So 2.14 would be the next chance I guess

No. The original ABI was introduced about 10 years ago with lttng-ust 2.0,
and lttng-ust 2.13 introduces the first ABI break since. I don't
plan on doing any ABI break in lttng-ust in the foreseeable future.

ABI breaks require that our users recompile all their instrumented
applications, which is really cumbersome for large software deployments.
We don't break ABI lightly.

We should rather introduce new features as extensions (new symbols).

> 
>> Also, I'm not so keen on adding an indirect call on the fast-path
>> when it's not absolutely needed.
> 
> Code seems pretty similar: https://godbolt.org/z/oK1WhWqGT

By fast-path, I also mean:

+        (*callback)(source->file, source->line, source->func, msg, len,
+                LTTNG_UST_CALLER_IP());

Which introduces an indirect call which needs to be taken when tracing
is active.

> 
>> What is wrong with having one symbol per loglevel ?
> 
> Macro-magic is cumbersome to edit, more code, more relocations.

If it was still time for ABI breaks, I would be tempted to consider it
especially given that tracelog and tracef are not expected to be "high-speed",
but now is too late for breaking ABI.

> 
> Easier to adapt aswell, could roll my own tracelog functions while
> using lttng_ust__tracelog_printf (started soind that as I don't want
> to link to lttng-ust.so)

What prevents you from linking against lttng-ust.so again ?

Thanks,

Mathieu

-- 
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

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Norbert Lange via lttng-dev]

Am Do., 20. Mai 2021 um 17:21 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
> ----- On May 20, 2021, at 10:57 AM, Norbert Lange nolange79@gmail.com wrote:
>
> > Am Do., 20. Mai 2021 um 16:19 Uhr schrieb Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.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.
> >
> > So 2.14 would be the next chance I guess
>
> No. The original ABI was introduced about 10 years ago with lttng-ust 2.0,
> and lttng-ust 2.13 introduces the first ABI break since. I don't
> plan on doing any ABI break in lttng-ust in the foreseeable future.
>
> ABI breaks require that our users recompile all their instrumented
> applications, which is really cumbersome for large software deployments.
> We don't break ABI lightly.

Yeah, I understand.


> >> Also, I'm not so keen on adding an indirect call on the fast-path
> >> when it's not absolutely needed.
> >
> > Code seems pretty similar: https://godbolt.org/z/oK1WhWqGT
>
> By fast-path, I also mean:
>
> +        (*callback)(source->file, source->line, source->func, msg, len,
> +                LTTNG_UST_CALLER_IP());
>
> Which introduces an indirect call which needs to be taken when tracing
> is active.

The worst thing is that it would tax branch-predictors. Indirect jumps aren't
that horrible, and if you have public interpose-able ELF symbols you
have more of them than you might know...

And that's a function that calls a printf variant, and did alloc memory.

> >> What is wrong with having one symbol per loglevel ?
> >
> > Macro-magic is cumbersome to edit, more code, more relocations.
>
> If it was still time for ABI breaks, I would be tempted to consider it
> especially given that tracelog and tracef are not expected to be "high-speed",
> but now is too late for breaking ABI.
>
> >
> > Easier to adapt aswell, could roll my own tracelog functions while
> > using lttng_ust__tracelog_printf (started soind that as I don't want
> > to link to lttng-ust.so)
>
> What prevents you from linking against lttng-ust.so again ?

I did not poke around enough with Lttng to be confident it wont have
side effects,
I really don't want it active in production. It doesn't seem there is
much public knowledge with Xenomai either.
lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,
So Id want a dynamic tracepoint-provider than i can dlopen (so that
the signal masks are inherited,
I hope you dont touch them).

Of course I could just remove all lttng libraries on the production
system aswell. Still doesnt change that
tracelog and tracef doesnt work that way.

I implemented my own tracelog/tracef using the normal lttng
tracepoints for now, they totally break on source level with 2.13
aswell ;)
is it ok if I do this to access them:

#define TRACEPOINT_DEFINE
#define TRACEPOINT_PROBE_DYNAMIC_LINKAGE
// 2.12
// #include <lttng/lttng-ust-tracelog.h>
// #include <lttng/lttng-ust-tracef.h>
// 2.13
#include <lttng/tp/lttng-ust-tracelog.h>
#include <lttng/tp/lttng-ust-tracef.h>

ie. I would load lttng-ust.so later and can then use those tracepoints.

Norbert
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Mathieu Desnoyers via lttng-dev]

----- On May 20, 2021, at 11:54 AM, Norbert Lange nolange79@gmail.com wrote:
[...]

>> What prevents you from linking against lttng-ust.so again ?
> 
> I did not poke around enough with Lttng to be confident it wont have
> side effects,
> I really don't want it active in production. It doesn't seem there is
> much public knowledge with Xenomai either.
> lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,

There is indeed a split between instrumentation and runtime threads done
with lttng-ust-tracepoint.so vs lttng-ust.so.

I understand that this split is missing for tracelog and tracef, and
would be a good thing to have.

I would be interested to move the tracelog and tracef implementation
from liblttng-ust.so to liblttng-ust-tracepoint.so, even this late
in the -rc cycle, because all users of tracelog/tracef need to link
against liblttng-ust-tracepoint.so anyway. So moving these symbols
should not affect anyone.

Can you give it a try and let me know if it works for you ?

> So Id want a dynamic tracepoint-provider than i can dlopen (so that
> the signal masks are inherited,
> I hope you dont touch them).

The signals are all blocked for lttng-ust listener threads. We don't
modify the signal masks in the tracepoint probes. Not sure which is
the target of your question though.

> 
> Of course I could just remove all lttng libraries on the production
> system aswell. Still doesnt change that
> tracelog and tracef doesnt work that way.

Would moving the tracelog/tracef implementation to liblttng-ust-tracepoint.so
solve your issues ?

> 
> I implemented my own tracelog/tracef using the normal lttng
> tracepoints for now, they totally break on source level with 2.13
> aswell ;)
> is it ok if I do this to access them:
> 
> #define TRACEPOINT_DEFINE
> #define TRACEPOINT_PROBE_DYNAMIC_LINKAGE
> // 2.12
> // #include <lttng/lttng-ust-tracelog.h>
> // #include <lttng/lttng-ust-tracef.h>
> // 2.13
> #include <lttng/tp/lttng-ust-tracelog.h>
> #include <lttng/tp/lttng-ust-tracef.h>
> 
> ie. I would load lttng-ust.so later and can then use those tracepoints.

Reimplementing the tracelog/tracef providers is not an intended use-case.
I'd very much prefer if we move their implementation to
liblttng-ust-tracepoint.so.

Thanks,

Mathieu

-- 
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

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Norbert Lange via lttng-dev]

Am Do., 20. Mai 2021 um 18:25 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
> ----- On May 20, 2021, at 11:54 AM, Norbert Lange nolange79@gmail.com wrote:
> [...]
>
> >> What prevents you from linking against lttng-ust.so again ?
> >
> > I did not poke around enough with Lttng to be confident it wont have
> > side effects,
> > I really don't want it active in production. It doesn't seem there is
> > much public knowledge with Xenomai either.
> > lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,
>
> There is indeed a split between instrumentation and runtime threads done
> with lttng-ust-tracepoint.so vs lttng-ust.so.
>
> I understand that this split is missing for tracelog and tracef, and
> would be a good thing to have.
>
> I would be interested to move the tracelog and tracef implementation
> from liblttng-ust.so to liblttng-ust-tracepoint.so, even this late
> in the -rc cycle, because all users of tracelog/tracef need to link
> against liblttng-ust-tracepoint.so anyway. So moving these symbols
> should not affect anyone.
>
> Can you give it a try and let me know if it works for you ?

Will take some time, whats the timeframe you need for feedback?

> > So Id want a dynamic tracepoint-provider than i can dlopen (so that
> > the signal masks are inherited,
> > I hope you dont touch them).
>
> The signals are all blocked for lttng-ust listener threads. We don't
> modify the signal masks in the tracepoint probes. Not sure which is
> the target of your question though.

The first one, if i'd preloaded lttng-ust and you dont mask signals,
then those could end up in the realtime threads.
Every Xenomai Thread has a background Linux Thread that's idle when
realtime is active,
ironically making them perfect targets for signal delivery

>
> >
> > Of course I could just remove all lttng libraries on the production
> > system aswell. Still doesnt change that
> > tracelog and tracef doesnt work that way.
>
> Would moving the tracelog/tracef implementation to liblttng-ust-tracepoint.so
> solve your issues ?

Yes, definitely. They should then work identically to other tracepoints
with TRACEPOINT_PROBE_DYNAMIC_LINKAGE.

>
> >
> > I implemented my own tracelog/tracef using the normal lttng
> > tracepoints for now, they totally break on source level with 2.13
> > aswell ;)
> > is it ok if I do this to access them:
> >
> > #define TRACEPOINT_DEFINE
> > #define TRACEPOINT_PROBE_DYNAMIC_LINKAGE
> > // 2.12
> > // #include <lttng/lttng-ust-tracelog.h>
> > // #include <lttng/lttng-ust-tracef.h>
> > // 2.13
> > #include <lttng/tp/lttng-ust-tracelog.h>
> > #include <lttng/tp/lttng-ust-tracef.h>
> >
> > ie. I would load lttng-ust.so later and can then use those tracepoints.
>
> Reimplementing the tracelog/tracef providers is not an intended use-case.
> I'd very much prefer if we move their implementation to
> liblttng-ust-tracepoint.so.

FWIW it works, and ill use it for a while (cant just swap out
libraries everywhere now).
Of course Id love a upstream solution.

Norbert
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Mathieu Desnoyers via lttng-dev]

----- On May 20, 2021, at 12:51 PM, Norbert Lange nolange79@gmail.com wrote:

> Am Do., 20. Mai 2021 um 18:25 Uhr schrieb Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com>:
>>
>> ----- On May 20, 2021, at 11:54 AM, Norbert Lange nolange79@gmail.com wrote:
>> [...]
>>
>> >> What prevents you from linking against lttng-ust.so again ?
>> >
>> > I did not poke around enough with Lttng to be confident it wont have
>> > side effects,
>> > I really don't want it active in production. It doesn't seem there is
>> > much public knowledge with Xenomai either.
>> > lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,
>>
>> There is indeed a split between instrumentation and runtime threads done
>> with lttng-ust-tracepoint.so vs lttng-ust.so.
>>
>> I understand that this split is missing for tracelog and tracef, and
>> would be a good thing to have.
>>
>> I would be interested to move the tracelog and tracef implementation
>> from liblttng-ust.so to liblttng-ust-tracepoint.so, even this late
>> in the -rc cycle, because all users of tracelog/tracef need to link
>> against liblttng-ust-tracepoint.so anyway. So moving these symbols
>> should not affect anyone.
>>
>> Can you give it a try and let me know if it works for you ?
> 
> Will take some time, whats the timeframe you need for feedback?

Here is the tentative commit:

https://review.lttng.org/c/lttng-ust/+/5927 Move tracef/tracelog symbols to liblttng-ust-tracepoint.so

Thanks,

Mathieu

-- 
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

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Norbert Lange via lttng-dev]

Am Do., 20. Mai 2021 um 19:18 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
>
>
> ----- On May 20, 2021, at 12:51 PM, Norbert Lange nolange79@gmail.com wrote:
>
> > Am Do., 20. Mai 2021 um 18:25 Uhr schrieb Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com>:
> >>
> >> ----- On May 20, 2021, at 11:54 AM, Norbert Lange nolange79@gmail.com wrote:
> >> [...]
> >>
> >> >> What prevents you from linking against lttng-ust.so again ?
> >> >
> >> > I did not poke around enough with Lttng to be confident it wont have
> >> > side effects,
> >> > I really don't want it active in production. It doesn't seem there is
> >> > much public knowledge with Xenomai either.
> >> > lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,
> >>
> >> There is indeed a split between instrumentation and runtime threads done
> >> with lttng-ust-tracepoint.so vs lttng-ust.so.
> >>
> >> I understand that this split is missing for tracelog and tracef, and
> >> would be a good thing to have.
> >>
> >> I would be interested to move the tracelog and tracef implementation
> >> from liblttng-ust.so to liblttng-ust-tracepoint.so, even this late
> >> in the -rc cycle, because all users of tracelog/tracef need to link
> >> against liblttng-ust-tracepoint.so anyway. So moving these symbols
> >> should not affect anyone.
> >>
> >> Can you give it a try and let me know if it works for you ?
> >
> > Will take some time, whats the timeframe you need for feedback?
>
> Here is the tentative commit:
>
> https://review.lttng.org/c/lttng-ust/+/5927 Move tracef/tracelog symbols to liblttng-ust-tracepoint.so

Well... this is certainly an improvement. I am not completely happy
though: "... users now link against
liblttng-ust-tracepoint.so explicitly"

My homecooked solution currently works like this:

*) define the probes from <lttng/lttng-ust-tracelog.h> with
TRACEPOINT_PROBE_DYNAMIC_LINKAGE,
    link them in the application, together with other dynamic probes
*) build a separate library with *other* tracepoints, lets call it
libtracepoint.so
*) don't link the application to any lttng library.

Which means:

1) the application works without lttng libraries. tracepoints are no-ops
2) if available then liblttng-ust-tracepoint.so is loaded (constructor
function from your headers). tracepoints are no-ops
3) if the application dlopen's libtracepoint.so and in turn
liblttng-ust.so then tracepoints work.

I'd lose option 1 compared to reimplementing tracelog using homecooked
lttng-ust-tracelog tracepoints.

So, are there any issues using <lttng/lttng-ust-tracelog.h> that way,
it seems to work fine,
are there mutliple competing instances now?
(I am not re-using any bit from tracelog.h, I am just after using the
tracepoint definition).

I mean I could dlsym all the functions, but tracelog has 1 per
loglevel and really ugly long names ;)

Norbert
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Mathieu Desnoyers via lttng-dev]

----- On May 20, 2021, at 1:43 PM, Norbert Lange nolange79@gmail.com wrote:

> Am Do., 20. Mai 2021 um 19:18 Uhr schrieb Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com>:
>>
>>
>>
>> ----- On May 20, 2021, at 12:51 PM, Norbert Lange nolange79@gmail.com wrote:
>>
>> > Am Do., 20. Mai 2021 um 18:25 Uhr schrieb Mathieu Desnoyers
>> > <mathieu.desnoyers@efficios.com>:
>> >>
>> >> ----- On May 20, 2021, at 11:54 AM, Norbert Lange nolange79@gmail.com wrote:
>> >> [...]
>> >>
>> >> >> What prevents you from linking against lttng-ust.so again ?
>> >> >
>> >> > I did not poke around enough with Lttng to be confident it wont have
>> >> > side effects,
>> >> > I really don't want it active in production. It doesn't seem there is
>> >> > much public knowledge with Xenomai either.
>> >> > lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,
>> >>
>> >> There is indeed a split between instrumentation and runtime threads done
>> >> with lttng-ust-tracepoint.so vs lttng-ust.so.
>> >>
>> >> I understand that this split is missing for tracelog and tracef, and
>> >> would be a good thing to have.
>> >>
>> >> I would be interested to move the tracelog and tracef implementation
>> >> from liblttng-ust.so to liblttng-ust-tracepoint.so, even this late
>> >> in the -rc cycle, because all users of tracelog/tracef need to link
>> >> against liblttng-ust-tracepoint.so anyway. So moving these symbols
>> >> should not affect anyone.
>> >>
>> >> Can you give it a try and let me know if it works for you ?
>> >
>> > Will take some time, whats the timeframe you need for feedback?
>>
>> Here is the tentative commit:
>>
>> https://review.lttng.org/c/lttng-ust/+/5927 Move tracef/tracelog symbols to
>> liblttng-ust-tracepoint.so
> 
> Well... this is certainly an improvement. I am not completely happy
> though: "... users now link against
> liblttng-ust-tracepoint.so explicitly"

I'm abandoning this change for now.

It's too late in the rc cycle for doing an ABI breaking change. Also, this
can eventually be done gradually by introducing a new .so with new symbols,
and mapping the tracelog/tracef APIs to those new symbols in a future release.
So I don't think it justifies breaking ABI at this stage.

> 
> My homecooked solution currently works like this:
> 
> *) define the probes from <lttng/lttng-ust-tracelog.h> with
> TRACEPOINT_PROBE_DYNAMIC_LINKAGE,
>    link them in the application, together with other dynamic probes
> *) build a separate library with *other* tracepoints, lets call it
> libtracepoint.so
> *) don't link the application to any lttng library.
> 
> Which means:
> 
> 1) the application works without lttng libraries. tracepoints are no-ops
> 2) if available then liblttng-ust-tracepoint.so is loaded (constructor
> function from your headers). tracepoints are no-ops
> 3) if the application dlopen's libtracepoint.so and in turn
> liblttng-ust.so then tracepoints work.
> 
> I'd lose option 1 compared to reimplementing tracelog using homecooked
> lttng-ust-tracelog tracepoints.
> 
> So, are there any issues using <lttng/lttng-ust-tracelog.h> that way,
> it seems to work fine,
> are there mutliple competing instances now?
> (I am not re-using any bit from tracelog.h, I am just after using the
> tracepoint definition).
> 
> I mean I could dlsym all the functions, but tracelog has 1 per
> loglevel and really ugly long names ;)

I recommend that you re-use the parts of tracelog which are useful to
you, but that you implement your own events within your provider .so
with your own event names, so there is no clash with upstream lttng-ust.

Thanks,

Mathieu

-- 
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

Re: [lttng-dev] [PATCH lttng-ust] Improve tracelog handling, reduce exported functions

[Norbert Lange via lttng-dev]

Am Do., 20. Mai 2021 um 19:18 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
>
>
> ----- On May 20, 2021, at 12:51 PM, Norbert Lange nolange79@gmail.com wrote:
>
> > Am Do., 20. Mai 2021 um 18:25 Uhr schrieb Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com>:
> >>
> >> ----- On May 20, 2021, at 11:54 AM, Norbert Lange nolange79@gmail.com wrote:
> >> [...]
> >>
> >> >> What prevents you from linking against lttng-ust.so again ?
> >> >
> >> > I did not poke around enough with Lttng to be confident it wont have
> >> > side effects,
> >> > I really don't want it active in production. It doesn't seem there is
> >> > much public knowledge with Xenomai either.
> >> > lttng-ust.so will spawn threads, lttng-ust-tracepoint.so is mostly passive,
> >>
> >> There is indeed a split between instrumentation and runtime threads done
> >> with lttng-ust-tracepoint.so vs lttng-ust.so.
> >>
> >> I understand that this split is missing for tracelog and tracef, and
> >> would be a good thing to have.
> >>
> >> I would be interested to move the tracelog and tracef implementation
> >> from liblttng-ust.so to liblttng-ust-tracepoint.so, even this late
> >> in the -rc cycle, because all users of tracelog/tracef need to link
> >> against liblttng-ust-tracepoint.so anyway. So moving these symbols
> >> should not affect anyone.
> >>
> >> Can you give it a try and let me know if it works for you ?
> >
> > Will take some time, whats the timeframe you need for feedback?
>
> Here is the tentative commit:
>
> https://review.lttng.org/c/lttng-ust/+/5927 Move tracef/tracelog symbols to liblttng-ust-tracepoint.so
>

Make a own thread for this?
I testet this, but get errors as liblttng-ust-tracepoint.so depends on
symbols from liblttng-ust.so.

/usr/lib/gcc/x86_64-linux-gnu/10/../../../../x86_64-linux-gnu/bin/ld:
/tmp/opt/hipase2/buildroot-acpu/host/x86_64-buildroot-linux-gnu/sysroot/lib/../lib64/liblttng-ust-tracepoint.so:
undefined reference to `lttng_ust_probe_register'
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../x86_64-linux-gnu/bin/ld:
/tmp/opt/hipase2/buildroot-acpu/host/x86_64-buildroot-linux-gnu/sysroot/lib/../lib64/liblttng-ust-tracepoint.so:
undefined reference to `lttng_ust_probe_unregister'
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev