[libcamera-devel] [PATCH v2 1/6] libcamera: tracing: Implement tracing infrastructure
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Oct 29 00:10:55 CET 2020
Hi Paul,
Thank you for the patch.
General comment, a cover letter could be nice to give some context and a
high-level summary of changes since the previous version.
On Wed, Oct 28, 2020 at 07:31:46PM +0900, 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>
>
> ---
> 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 | 40 +++++++++++++++++++
> .../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, 127 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..b925b8e8 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: [ tracepoints_header_generator, '@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..9b6c3907
> --- /dev/null
> +++ b/include/libcamera/internal/tracepoints.h.in
> @@ -0,0 +1,40 @@
> +/* 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__)
> +#else
> +#define LIBCAMERA_TRACEPOINT(...) do {} while (0)
This could cause compiler warnings because of unused variables, if some
of the arguments to the tracepoint are otherwise not used. I wonder if
this could help.
namespace {
template <typename ...Args>
inline void unused([[maybe_unused]] Args&& ...args)
{
}
} /* namespace */
#define LIBCAMERA_TRACEPOINT(category, ...) unused(__VA_ARGS__)
> +#endif
> +
> +#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 */
I wonder if we could generate one header per tracepoint, to avoid
including everything in all compilation units which could slow down
compilation, but this can be addressed later.
> 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..7e7d76b2
> --- /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:
That should be < 4 as you need at least one entry in tp_files.
> + 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').read() + '\n\n'
I wonder, should we specify the encoding explicitly for the files ? Not
that I expect much else than ascii in the templates, but if it happens,
we would depend on the default encoding of the platform, which could
possibly lead to unexpected results. Risks are small, but maybe setting
encode='utf-8' would be best ? Same for the other files below.
> +
> + 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..46946f36
> --- /dev/null
> +++ b/utils/tracepoints/meson.build
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +py_modules += ['jinja2']
> +
> +tracepoints_header_generator = find_program('./gen-tp-header.py')
I may have called this gen_tracepoints to match other generators in the
parent directory, but it doesn't matter that much.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list