[libcamera-devel] [PATCH v3 1/6] libcamera: tracing: Implement tracing infrastructure
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Oct 29 11:34:31 CET 2020
Hi Kieran,
On Thu, Oct 29, 2020 at 10:30:51AM +0000, Kieran Bingham wrote:
> On 29/10/2020 10:16, Paul Elder wrote:
> > Implement tracing infrastructure in libcamera. It takes .tp files, as
> > required by lttng, and generates a tracepoint header and C file, as lttng
> > requires. meson is updated accordingly to get it to compile with the
> > rest of libcamera. Update the documentation accordingly.
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > ---
> > Changes in v3:
> > - rename the tracepoints header generateor in meson
> > - add protection against accidental unused variables when tracing is
> > disabled
> > - make gen-tp-header.py read files as utf-8
> > - add macros LIBCAMERA_TRACEPOINT_IPA_{BEGIN,END}
> >
> > Changes in v2:
> > - move header.tmpl to tracepoints.h.in
> > - rename meson option 'tracepoints' to 'tracing'
> > - make sure C++ objects work in tracepoint arguments
> > - rename generate_header.py to gen-tp-header.py
> > ---
> > Documentation/Doxyfile.in | 2 +
> > README.rst | 3 +
> > include/libcamera/internal/meson.build | 9 +++
> > include/libcamera/internal/tracepoints.h.in | 62 +++++++++++++++++++
> > .../internal/tracepoints/meson.build | 4 ++
> > meson.build | 3 +
> > meson_options.txt | 5 ++
> > src/libcamera/meson.build | 7 +++
> > src/libcamera/tracepoints.cpp | 10 +++
> > utils/meson.build | 1 +
> > utils/tracepoints/gen-tp-header.py | 38 ++++++++++++
> > utils/tracepoints/meson.build | 5 ++
> > 12 files changed, 149 insertions(+)
> > create mode 100644 include/libcamera/internal/tracepoints.h.in
> > create mode 100644 include/libcamera/internal/tracepoints/meson.build
> > create mode 100644 src/libcamera/tracepoints.cpp
> > create mode 100644 utils/tracepoints/gen-tp-header.py
> > create mode 100644 utils/tracepoints/meson.build
> >
> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > index 5ccd0d05..9a7a3fb4 100644
> > --- a/Documentation/Doxyfile.in
> > +++ b/Documentation/Doxyfile.in
> > @@ -842,6 +842,8 @@ EXCLUDE = @TOP_SRCDIR@/include/libcamera/span.h \
> > @TOP_SRCDIR@/src/libcamera/ipa_ipc_unixsocket.cpp \
> > @TOP_SRCDIR@/src/libcamera/pipeline/ \
> > @TOP_SRCDIR@/src/libcamera/proxy/ \
> > + @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
> > + @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
> > @TOP_BUILDDIR@/include/libcamera/ipa/ \
> > @TOP_BUILDDIR@/src/libcamera/proxy/
> >
> > diff --git a/README.rst b/README.rst
> > index fc8f4948..5c4b6989 100644
> > --- a/README.rst
> > +++ b/README.rst
> > @@ -81,6 +81,9 @@ for gstreamer: [optional]
> > for qcam: [optional]
> > qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev
> >
> > +for tracing with lttng: [optional]
> > + lttng-ust-dev python3-jinja2
> > +
> > Using GStreamer plugin
> > ~~~~~~~~~~~~~~~~~~~~~~
> >
> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > index 8dd744fd..592635a9 100644
> > --- a/include/libcamera/internal/meson.build
> > +++ b/include/libcamera/internal/meson.build
> > @@ -1,5 +1,14 @@
> > # SPDX-License-Identifier: CC0-1.0
> >
> > +subdir('tracepoints')
> > +
> > +libcamera_tracepoint_header = custom_target(
> > + 'tp_header',
> > + input: [ 'tracepoints.h.in', tracepoint_files ],
> > + output: 'tracepoints.h',
> > + command: [ gen_tracepoints_header, '@OUTPUT@', '@INPUT@' ],
> > +)
> > +
> > libcamera_internal_headers = files([
> > 'byte_stream_buffer.h',
> > 'camera_controls.h',
> > diff --git a/include/libcamera/internal/tracepoints.h.in b/include/libcamera/internal/tracepoints.h.in
> > new file mode 100644
> > index 00000000..830c61d7
> > --- /dev/null
> > +++ b/include/libcamera/internal/tracepoints.h.in
> > @@ -0,0 +1,62 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) {{year}}, Google Inc.
> > + *
> > + * tracepoints.h - Tracepoints with lttng
> > + *
> > + * This file is auto-generated. Do not edit.
> > + */
> > +#ifndef __LIBCAMERA_INTERNAL_TRACEPOINTS_H__
> > +#define __LIBCAMERA_INTERNAL_TRACEPOINTS_H__
> > +
> > +#if HAVE_TRACING
> > +#define LIBCAMERA_TRACEPOINT(...) tracepoint(libcamera, __VA_ARGS__)
> > +
> > +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \
> > +tracepoint(libcamera, ipa_call_begin, #pipe, #func)
> > +
> > +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \
> > +tracepoint(libcamera, ipa_call_end, #pipe, #func)
> > +
> > +#else
> > +
> > +namespace {
> > +
> > +template <typename ...Args>
> > +inline void unused([[maybe_unused]] Args&& ...args)
> > +{
> > +}
>
> Do you need this ununsed()?
>
> > +
> > +} /* namespace */
> > +
> > +#define LIBCAMERA_TRACEPOINT(category, ...) unused(__VA_ARGS__)
>
> You could just define the following to declare that they don't do anything:
>
> #define LIBCAMERA_TRACEPOINT(category, ...) do { } while (0)
> #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) do { } while (0)
> #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) do { } while (0)
>
> But perhaps that's the C developer approach.
That's Paul had in the previous version, but it causes unused variable
warnings if arguments to the tracepoints are stored in variables that
are otherwise unused.
> There's no requirement to change this by the way, I'm only wondering if
> removing the template would be faster at build time.
>
> Otherwise, at least simplifying the function to
> inline void unused(void)
> {
> }
>
> and not passing the arguments, would remove the need for any templating
> also.
>
> But that's fairly minor, and the rest looks reasonable to me.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > +
> > +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) unused(#pipe, #func)
> > +
> > +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) unused(#pipe, #func)
These two are however not needed, as they're just identifiers treated as
strings, not variables.
#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func)
#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func)
is good enough.
> > +
> > +#endif /* HAVE_TRACING */
> > +
> > +#endif /* __LIBCAMERA_INTERNAL_TRACEPOINTS_H__ */
> > +
> > +
> > +#if HAVE_TRACING
> > +
> > +#undef TRACEPOINT_PROVIDER
> > +#define TRACEPOINT_PROVIDER libcamera
> > +
> > +#undef TRACEPOINT_INCLUDE
> > +#define TRACEPOINT_INCLUDE "{{path}}"
> > +
> > +#if !defined(INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H) || defined(TRACEPOINT_HEADER_MULTI_READ)
> > +#define INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H
> > +
> > +#include <lttng/tracepoint.h>
> > +
> > +{{source}}
> > +
> > +#endif /* INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H */
> > +
> > +#include <lttng/tracepoint-event.h>
> > +
> > +#endif /* HAVE_TRACING */
> > diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build
> > new file mode 100644
> > index 00000000..2dd6733e
> > --- /dev/null
> > +++ b/include/libcamera/internal/tracepoints/meson.build
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +tracepoint_files = files([
> > +])
> > diff --git a/meson.build b/meson.build
> > index e6f6c84a..55cf36e1 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -108,6 +108,9 @@ libcamera_includes = include_directories('include')
> > # Sub-directories fill py_modules with their dependencies.
> > py_modules = []
> >
> > +# Libraries used by multiple components
> > +liblttng = cc.find_library('lttng-ust', required : get_option('tracing'))
> > +
> > # Utilities are parsed first to provide support for other components.
> > subdir('utils')
> >
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 7f7b3e59..53f2675e 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -28,6 +28,11 @@ option('test',
> > type : 'boolean',
> > description: 'Compile and include the tests')
> >
> > +option('tracing',
> > + type : 'feature',
> > + value : 'auto',
> > + description: 'Enable tracing (based on lttng)')
> > +
> > option('v4l2',
> > type : 'boolean',
> > value : false,
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 24f7ea3d..51925bd6 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -58,6 +58,7 @@ libcamera_sources = files([
> >
> > libcamera_sources += libcamera_public_headers
> > libcamera_sources += libcamera_generated_ipa_headers
> > +libcamera_sources += libcamera_tracepoint_header
> >
> > includes = [
> > libcamera_includes,
> > @@ -75,6 +76,11 @@ if libgnutls.found()
> > config_h.set('HAVE_GNUTLS', 1)
> > endif
> >
> > +if liblttng.found()
> > + config_h.set('HAVE_TRACING', 1)
> > + libcamera_sources += files(['tracepoints.cpp'])
> > +endif
> > +
> > if libudev.found()
> > config_h.set('HAVE_LIBUDEV', 1)
> > libcamera_sources += files([
> > @@ -117,6 +123,7 @@ libcamera_deps = [
> > libatomic,
> > libdl,
> > libgnutls,
> > + liblttng,
> > libudev,
> > dependency('threads'),
> > ]
> > diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/tracepoints.cpp
> > new file mode 100644
> > index 00000000..0173b75a
> > --- /dev/null
> > +++ b/src/libcamera/tracepoints.cpp
> > @@ -0,0 +1,10 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * tracepoints.cpp - Tracepoints with lttng
> > + */
> > +#define TRACEPOINT_CREATE_PROBES
> > +#define TRACEPOINT_DEFINE
> > +
> > +#include "libcamera/internal/tracepoints.h"
> > diff --git a/utils/meson.build b/utils/meson.build
> > index 383d5285..8e28ada7 100644
> > --- a/utils/meson.build
> > +++ b/utils/meson.build
> > @@ -2,6 +2,7 @@
> >
> > subdir('ipc')
> > subdir('ipu3')
> > +subdir('tracepoints')
> >
> > ## Code generation
> > py_modules += ['yaml']
> > diff --git a/utils/tracepoints/gen-tp-header.py b/utils/tracepoints/gen-tp-header.py
> > new file mode 100644
> > index 00000000..9500e474
> > --- /dev/null
> > +++ b/utils/tracepoints/gen-tp-header.py
> > @@ -0,0 +1,38 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (C) 2020, Google Inc.
> > +#
> > +# Author: Paul Elder <paul.elder at ideasonboard.com>
> > +#
> > +# gen-tp-header.py - Generate header file to contain lttng tracepoints
> > +
> > +import datetime
> > +import jinja2
> > +import os
> > +import sys
> > +
> > +def main(argv):
> > + if len(argv) < 3:
> > + print(f'Usage: {argv[0]} output template tp_files...')
> > + return 1
> > +
> > + output = argv[1]
> > + template = argv[2]
> > +
> > + year = datetime.datetime.now().year
> > + path = output.replace('include/', '', 1)
> > +
> > + source = ''
> > + for fname in argv[3:]:
> > + source += open(fname, 'r', encoding='utf-8').read() + '\n\n'
> > +
> > + template = jinja2.Template(open(template, 'r').read())
> > + string = template.render(year=year, path=path, source=source)
> > +
> > + f = open(output, 'w').write(string)
> > +
> > + return 0
> > +
> > +
> > +if __name__ == '__main__':
> > + sys.exit(main(sys.argv))
> > diff --git a/utils/tracepoints/meson.build b/utils/tracepoints/meson.build
> > new file mode 100644
> > index 00000000..807230fc
> > --- /dev/null
> > +++ b/utils/tracepoints/meson.build
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +py_modules += ['jinja2']
> > +
> > +gen_tracepoints_header = find_program('./gen-tp-header.py')
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list