[libcamera-devel] [PATCH v3 1/6] libcamera: tracing: Implement tracing infrastructure
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Oct 29 11:37:18 CET 2020
Hi Laurent,
On 29/10/2020 10:34, Laurent Pinchart wrote:
> 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.
Aha - that's clear indeed, it is the effect on the location that matters
of course.
Fine by me either way.
>> 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.
Well, clearly we need the arguments, so that doesn't work ;-)
>>
>> But that's fairly minor, and the rest looks reasonable to me.
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Still stands.
Thanks
Kieran
>>
>>> +
>>> +#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
--
Kieran
More information about the libcamera-devel
mailing list