| Message ID | 20200224173455.15900-1-maxime.roussinbelanger@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3,lttng-ust,1/2] Introduce vtracef | expand |
----- On Feb 24, 2020, at 12:34 PM, Maxime Roussin-Belanger maxime.roussinbelanger at gmail.com wrote: [...] > diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c > index ea98e43e..1311f0d5 100644 > --- a/liblttng-ust/tracef.c > +++ b/liblttng-ust/tracef.c > @@ -29,20 +29,26 @@ > #define TRACEPOINT_DEFINE > #include "lttng-ust-tracef-provider.h" > > -void _lttng_ust_tracef(const char *fmt, ...) > +void _lttng_ust_vtracef(const char *fmt, va_list ap) > { > - va_list ap; > char *msg; > - int len; > - > - va_start(ap, fmt); > - len = vasprintf(&msg, fmt, ap); > + const int len = vasprintf(&msg, fmt, ap); > /* len does not include the final \0 */ > if (len < 0) > goto end; > __tracepoint_cb_lttng_ust_tracef___event(msg, len, > LTTNG_UST_CALLER_IP()); > free(msg); > end: > + return; > +} > + > +void _lttng_ust_tracef(const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + _lttng_ust_vtracef(fmt, ap); > + > va_end(ap); > } Can you check the result of the generated binary (compiled with -O2) to see if there is an additional call added for _lttng_ust_trace() compared to the prior generated code ? I would hate to add an extra call indirection there. Thanks, Mathieu
----- On Feb 24, 2020, at 12:34 PM, Maxime Roussin-Belanger maxime.roussinbelanger at gmail.com wrote: > diff --git a/doc/examples/demo-tracef/demo-vtracef.c > b/doc/examples/demo-tracef/demo-vtracef.c > new file mode 100644 > index 00000000..224e9292 > --- /dev/null > +++ b/doc/examples/demo-tracef/demo-vtracef.c > @@ -0,0 +1,59 @@ > +/* > + * Copyright (C) 2020 Maxime Roussin-Belanger > <maxime.roussinbelanger at gmail.com> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; version 2.1 of > + * the License. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +#include <stdarg.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <unistd.h> > + > +#include <lttng/tracef.h> > + > + Remove one whiteline here. > +void print_debug(const char* msg, ...) > +{ > + va_list ap; > + va_start(ap, msg); > + > + vtracef(msg, ap); > + > + va_end(ap); > +} > + > +int main(int argc, char **argv) > +{ > + int i; > + int delay = 0; > + const char *str = "mystring test"; > + long l = 0x42; > + > + if (argc == 2) Should handle cases where argc > 2 as well. > + delay = atoi(argv[1]); > + > + fprintf(stderr, "Demo program starting.\n"); > + > + sleep(delay); > + > + fprintf(stderr, "Tracing... "); > + > + for (i = 0; i < 5; i++) { > + print_debug("This is a \"%s\" formatted %d event %lx", str, i, l); > + } > + > + fprintf(stderr, " done.\n"); > + return 0; > +} Thanks, Mathieu
----- On Feb 24, 2020, at 12:34 PM, Maxime Roussin-Belanger maxime.roussinbelanger at gmail.com wrote: [...] > diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c > index ea98e43e..1311f0d5 100644 > --- a/liblttng-ust/tracef.c > +++ b/liblttng-ust/tracef.c > @@ -29,20 +29,26 @@ > #define TRACEPOINT_DEFINE > #include "lttng-ust-tracef-provider.h" > > -void _lttng_ust_tracef(const char *fmt, ...) > +void _lttng_ust_vtracef(const char *fmt, va_list ap) > { > - va_list ap; > char *msg; > - int len; > - > - va_start(ap, fmt); > - len = vasprintf(&msg, fmt, ap); > + const int len = vasprintf(&msg, fmt, ap); > /* len does not include the final \0 */ > if (len < 0) > goto end; > __tracepoint_cb_lttng_ust_tracef___event(msg, len, > LTTNG_UST_CALLER_IP()); I don't think LTTNG_UST_CALLER_IP() has the behavior we would expect for _lttng_ust_tracef() anymore here. We want the callsite in the application or library which has been instrumented, not an IP within lttng-ust. Thanks, Mathieu > free(msg); > end: > + return; > +} > + > +void _lttng_ust_tracef(const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + _lttng_ust_vtracef(fmt, ap); > + > va_end(ap); > } > -- > 2.20.1 > > _______________________________________________ > lttng-dev mailing list > lttng-dev at lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
----- On Feb 26, 2020, at 4:14 PM, Mathieu Desnoyers mathieu.desnoyers at efficios.com wrote: > ----- On Feb 24, 2020, at 12:34 PM, Maxime Roussin-Belanger > maxime.roussinbelanger at gmail.com wrote: > [...] >> diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c >> index ea98e43e..1311f0d5 100644 >> --- a/liblttng-ust/tracef.c >> +++ b/liblttng-ust/tracef.c >> @@ -29,20 +29,26 @@ >> #define TRACEPOINT_DEFINE >> #include "lttng-ust-tracef-provider.h" >> >> -void _lttng_ust_tracef(const char *fmt, ...) >> +void _lttng_ust_vtracef(const char *fmt, va_list ap) >> { >> - va_list ap; >> char *msg; >> - int len; >> - >> - va_start(ap, fmt); >> - len = vasprintf(&msg, fmt, ap); >> + const int len = vasprintf(&msg, fmt, ap); >> /* len does not include the final \0 */ >> if (len < 0) >> goto end; >> __tracepoint_cb_lttng_ust_tracef___event(msg, len, >> LTTNG_UST_CALLER_IP()); > > I don't think LTTNG_UST_CALLER_IP() has the behavior we would expect > for _lttng_ust_tracef() anymore here. We want the callsite in the application > or library which has been instrumented, not an IP within lttng-ust. The following should fix both inlining and CALLER_IP() issues: static inline __attribute__((always_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; __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) { __lttng_ust_vtracef(fmt, ap); } void _lttng_ust_tracef(const char *fmt, ...) { va_list ap; va_start(ap, fmt); __lttng_ust_vtracef(fmt, ap); va_end(ap); } Thanks, Mathieu > > Thanks, > > Mathieu > >> free(msg); >> end: >> + return; >> +} >> + >> +void _lttng_ust_tracef(const char *fmt, ...) >> +{ >> + va_list ap; >> + >> + va_start(ap, fmt); >> + _lttng_ust_vtracef(fmt, ap); >> + >> va_end(ap); >> } >> -- >> 2.20.1 >> >> _______________________________________________ >> lttng-dev mailing list >> lttng-dev at 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 at lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
diff --git a/doc/examples/demo-tracef/Makefile b/doc/examples/demo-tracef/Makefile index ee207843..809aac19 100644 --- a/doc/examples/demo-tracef/Makefile +++ b/doc/examples/demo-tracef/Makefile @@ -20,18 +20,28 @@ LIBS = -ldl -llttng-ust # On Linux LOCAL_CPPFLAGS += -I. AM_V_P := : -all: demo-tracef +all: demo-tracef demo-vtracef demo-tracef.o: demo-tracef.c @if $(AM_V_P); then set -x; else echo " CC $@"; fi; \ $(CC) $(CPPFLAGS) $(LOCAL_CPPFLAGS) $(AM_CFLAGS) $(AM_CPPFLAGS) \ $(CFLAGS) -c -o $@ $< demo-tracef: demo-tracef.o @if $(AM_V_P); then set -x; else echo " CCLD $@"; fi; \ $(CC) $(LDFLAGS) $(AM_CFLAGS) $(AM_LDFLAGS) $(CFLAGS) \ -o $@ $< $(LIBS) +demo-vtracef.o: demo-vtracef.c + @if $(AM_V_P); then set -x; else echo " CC $@"; fi; \ + $(CC) $(CPPFLAGS) $(LOCAL_CPPFLAGS) $(AM_CFLAGS) $(AM_CPPFLAGS) \ + $(CFLAGS) -c -o $@ $< + +demo-vtracef: demo-vtracef.o + @if $(AM_V_P); then set -x; else echo " CCLD $@"; fi; \ + $(CC) $(LDFLAGS) $(AM_CFLAGS) $(AM_LDFLAGS) $(CFLAGS) \ + -o $@ $< $(LIBS) + .PHONY: clean clean: - rm -f *.o *.a demo-tracef + rm -f *.o *.a demo-tracef demo-vtracef diff --git a/doc/examples/demo-tracef/demo-vtracef.c b/doc/examples/demo-tracef/demo-vtracef.c new file mode 100644 index 00000000..224e9292 --- /dev/null +++ b/doc/examples/demo-tracef/demo-vtracef.c @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2020 Maxime Roussin-Belanger <maxime.roussinbelanger at gmail.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; version 2.1 of + * the License. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stdarg.h> +#include <stdlib.h> +#include <stdio.h> +#include <unistd.h> + +#include <lttng/tracef.h> + + +void print_debug(const char* msg, ...) +{ + va_list ap; + va_start(ap, msg); + + vtracef(msg, ap); + + va_end(ap); +} + +int main(int argc, char **argv) +{ + int i; + int delay = 0; + const char *str = "mystring test"; + long l = 0x42; + + if (argc == 2) + delay = atoi(argv[1]); + + fprintf(stderr, "Demo program starting.\n"); + + sleep(delay); + + fprintf(stderr, "Tracing... "); + + for (i = 0; i < 5; i++) { + print_debug("This is a \"%s\" formatted %d event %lx", str, i, l); + } + + fprintf(stderr, " done.\n"); + return 0; +} diff --git a/doc/man/tracef-tracelog-limitations.txt b/doc/man/tracef-tracelog-limitations.txt index a9491903..ad4130dd 100644 --- a/doc/man/tracef-tracelog-limitations.txt +++ b/doc/man/tracef-tracelog-limitations.txt @@ -21,4 +21,7 @@ Thus, +{macro-name}()+ is useful for quick prototyping and debugging, but should not be considered for any permanent/serious application instrumentation. ++v{macro-name}()+ does not have a `STAP_PROBEV()` call, because `STAP_PROBEV()` +does not support `va_list`. If you need it, you should emit this call yourself. + See man:lttng-ust(3) to learn more about custom tracepoint providers. diff --git a/doc/man/tracef.3.txt b/doc/man/tracef.3.txt index 1068afad..2894ca45 100644 --- a/doc/man/tracef.3.txt +++ b/doc/man/tracef.3.txt @@ -5,43 +5,44 @@ tracef(3) NAME ---- -tracef - LTTng-UST printf(3)-like interface +tracef, vtracef - LTTng-UST printf(3)-like interface SYNOPSIS -------- [verse] *#include <lttng/tracef.h>* [verse] #define *tracef*('fmt', ...) +#define *vtracef*('fmt', 'va_list' ap) Link with `-llttng-ust`. DESCRIPTION ----------- The LTTng-UST `tracef()` API allows you to trace your application with the help of a simple man:printf(3)-like macro. The 'fmt' argument is passed directly to the 'fmt' parameter of man:vasprintf(3), as well as the optional parameters following 'fmt'. -To use `tracef()`, include `<lttng/tracef.h>` where you need it, and -link your application with `liblttng-ust`. See the <<example,EXAMPLE>> +To use `tracef()` or `vtracef()`, include `<lttng/tracef.h>` where you need it, +and link your application with `liblttng-ust`. See the <<example,EXAMPLE>> section below for a complete usage example. -Once your application is instrumented with `tracef()` calls and +Once your application is instrumented with `tracef()` or `vtracef()` calls and ready to run, use man:lttng-enable-event(1) to enable the `lttng_ust_tracef:*` event. The `tracef()` events contain a single field, named `msg`, which is the formatted string output. If you need to attach a specific log level to a `tracef()` call, use man:tracelog(3) instead. See also the <<limitations,LIMITATIONS>> section below for important -limitations to consider when using `tracef()`. +limitations to consider when using `tracef()` or `vtracef()`. [[example]] diff --git a/include/lttng/tracef.h b/include/lttng/tracef.h index 0c59c9ae..854ccdce 100644 --- a/include/lttng/tracef.h +++ b/include/lttng/tracef.h @@ -32,13 +32,21 @@ extern "C" { extern void _lttng_ust_tracef(const char *fmt, ...); +extern +void _lttng_ust_vtracef(const char *fmt, va_list ap); + #define tracef(fmt, ...) \ do { \ LTTNG_STAP_PROBEV(tracepoint_lttng_ust_tracef, event, ## __VA_ARGS__); \ if (caa_unlikely(__tracepoint_lttng_ust_tracef___event.state)) \ _lttng_ust_tracef(fmt, ## __VA_ARGS__); \ } while (0) +#define vtracef(fmt, ap) \ + do { \ + if (caa_unlikely(__tracepoint_lttng_ust_tracef___event.state)) \ + _lttng_ust_vtracef(fmt, ap); \ + } while (0) #ifdef __cplusplus } #endif diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c index ea98e43e..1311f0d5 100644 --- a/liblttng-ust/tracef.c +++ b/liblttng-ust/tracef.c @@ -29,20 +29,26 @@ #define TRACEPOINT_DEFINE #include "lttng-ust-tracef-provider.h" -void _lttng_ust_tracef(const char *fmt, ...) +void _lttng_ust_vtracef(const char *fmt, va_list ap) { - va_list ap; char *msg; - int len; - - va_start(ap, fmt); - len = vasprintf(&msg, fmt, ap); + const int len = vasprintf(&msg, fmt, ap); /* len does not include the final \0 */ if (len < 0) goto end; __tracepoint_cb_lttng_ust_tracef___event(msg, len, LTTNG_UST_CALLER_IP()); free(msg); end: + return; +} + +void _lttng_ust_tracef(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + _lttng_ust_vtracef(fmt, ap); + va_end(ap); }
vtracef accepts a va_list argument to simplify tracing functions which use a va_list Here's an example from wpa_supplicant that I wanted to trace: void wpa_debug(int level, const char* fmt, ...) { va_list ap; va_start(ap, fmt); ... // The call I want to easily trace with vtracef vprintf(fmt, ap); ... va_end(ap); } wpa_debug is used a fair amount and it would be annoying to replace all the wpa_debug calls with tracef. With vtracef, it simplifies the find and replace effort by only changing it at one place. Signed-off-by: Maxime Roussin-Belanger <maxime.roussinbelanger at gmail.com> --- Changes in v3: * Fix coding style patch doc/examples/demo-tracef/Makefile | 14 +++++- doc/examples/demo-tracef/demo-vtracef.c | 59 +++++++++++++++++++++++++ doc/man/tracef-tracelog-limitations.txt | 3 ++ doc/man/tracef.3.txt | 11 ++--- include/lttng/tracef.h | 8 ++++ liblttng-ust/tracef.c | 18 +++++--- 6 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 doc/examples/demo-tracef/demo-vtracef.c