| Message ID | 20201030085756.79329-3-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Fri, Oct 30, 2020 at 05:57:53PM +0900, Paul Elder wrote: > Add and use tracepoints in Request. Requests are core to libcamera > operation, thus detecting delays in their processing is important, and > serves as a good usage example of tracepoints. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > Changes in v4: > - move enum tp definitions to "header" tp files > - add them to meson accordingly > - remove cancelled field from request_complete_buffer tracepoint > > Changes in v3: > - expand the changelog > - add enum tracepoint values so that request status and buffer status > are strings instead of ints in the trace > - remove cancelled from all request tracepoints, except complete_buffer > - expand the complete_buffer tracepoint to include information on the > buffer > - display the address of the request in all tracepoints > - remove tracepoints for add_buffer and find_buffer > > Changes in v2: > - remove tracepoints from uvcvideo > - remove comment in changelog that this is only used for demonstration > - use Request pointers instead of feeding the fields manually to the > tracepoint > --- > .../internal/tracepoints/buffer_enums.tp | 9 +++ > .../internal/tracepoints/meson.build | 5 ++ > .../libcamera/internal/tracepoints/request.tp | 68 +++++++++++++++++++ > .../internal/tracepoints/request_enums.tp | 9 +++ > src/libcamera/request.cpp | 11 +++ > 5 files changed, 102 insertions(+) > create mode 100644 include/libcamera/internal/tracepoints/buffer_enums.tp > create mode 100644 include/libcamera/internal/tracepoints/request.tp > create mode 100644 include/libcamera/internal/tracepoints/request_enums.tp > > diff --git a/include/libcamera/internal/tracepoints/buffer_enums.tp b/include/libcamera/internal/tracepoints/buffer_enums.tp > new file mode 100644 > index 00000000..fdbea69a > --- /dev/null > +++ b/include/libcamera/internal/tracepoints/buffer_enums.tp > @@ -0,0 +1,9 @@ > +TRACEPOINT_ENUM( > + libcamera, > + buffer_status, > + TP_ENUM_VALUES( > + ctf_enum_value("FrameSuccess", 0) > + ctf_enum_value("FrameError", 1) > + ctf_enum_value("FrameCancelled", 2) > + ) > +) > diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build > index 2dd6733e..fee0758f 100644 > --- a/include/libcamera/internal/tracepoints/meson.build > +++ b/include/libcamera/internal/tracepoints/meson.build > @@ -1,4 +1,9 @@ > # SPDX-License-Identifier: CC0-1.0 > > +# enum files must go first > tracepoint_files = files([ > + 'buffer_enums.tp', > + 'request_enums.tp', > + > + 'request.tp', > ]) How about # enum files must go first tracepoint_files = files([ 'buffer_enums.tp', 'request_enums.tp', ]) tracepoint_files += files([ 'request.tp', ]) ? > diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp > new file mode 100644 > index 00000000..dd28a714 > --- /dev/null > +++ b/include/libcamera/internal/tracepoints/request.tp > @@ -0,0 +1,68 @@ > +#include <libcamera/buffer.h> > +#include <libcamera/request.h> > + > +TRACEPOINT_EVENT_CLASS( > + libcamera, > + request, > + TP_ARGS( > + libcamera::Request *, req > + ), > + TP_FIELDS( > + ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req)) > + ctf_integer(uint64_t, cookie, req->cookie()) > + ctf_enum(libcamera, request_status, uint32_t, status, req->status()) > + ) > +) > + > +TRACEPOINT_EVENT_INSTANCE( > + libcamera, > + request, > + request_construct, > + TP_ARGS( > + libcamera::Request *, req > + ) > +) > + > +TRACEPOINT_EVENT_INSTANCE( > + libcamera, > + request, > + request_destroy, > + TP_ARGS( > + libcamera::Request *, req > + ) > +) > + > +TRACEPOINT_EVENT_INSTANCE( > + libcamera, > + request, > + request_reuse, > + TP_ARGS( > + libcamera::Request *, req > + ) > +) > + > +TRACEPOINT_EVENT_INSTANCE( > + libcamera, > + request, > + request_complete, > + TP_ARGS( > + libcamera::Request *, req > + ) > +) > + > + > +TRACEPOINT_EVENT( > + libcamera, > + request_complete_buffer, > + TP_ARGS( > + libcamera::Request *, req, > + libcamera::FrameBuffer *, buf > + ), > + TP_FIELDS( > + ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req)) > + ctf_integer(uint64_t, cookie, req->cookie()) > + ctf_integer(int, status, req->status()) > + ctf_integer_hex(uintptr_t, buffer, reinterpret_cast<uintptr_t>(buf)) > + ctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status) > + ) > +) > diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp > new file mode 100644 > index 00000000..371f6544 > --- /dev/null > +++ b/include/libcamera/internal/tracepoints/request_enums.tp > @@ -0,0 +1,9 @@ > +TRACEPOINT_ENUM( > + libcamera, > + request_status, > + TP_ENUM_VALUES( > + ctf_enum_value("RequestPending", 0) > + ctf_enum_value("RequestComplete", 1) > + ctf_enum_value("RequestCancelled", 2) > + ) > +) > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index ae8b1660..a68684ef 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -16,6 +16,7 @@ > > #include "libcamera/internal/camera_controls.h" > #include "libcamera/internal/log.h" > +#include "libcamera/internal/tracepoints.h" > > /** > * \file request.h > @@ -85,10 +86,14 @@ Request::Request(Camera *camera, uint64_t cookie) > * \todo: Add a validator for metadata controls. > */ > metadata_ = new ControlList(controls::controls); > + > + LIBCAMERA_TRACEPOINT(request_construct, this); > } > > Request::~Request() > { > + LIBCAMERA_TRACEPOINT(request_destroy, this); > + > delete metadata_; > delete controls_; > delete validator_; > @@ -106,6 +111,8 @@ Request::~Request() > */ > void Request::reuse(ReuseFlag flags) > { > + LIBCAMERA_TRACEPOINT(request_reuse, this); > + > pending_.clear(); > if (flags & ReuseBuffers) { > for (auto pair : bufferMap_) { > @@ -259,6 +266,8 @@ void Request::complete() > LOG(Request, Debug) > << "Request has completed - cookie: " << cookie_ > << (cancelled_ ? " [Cancelled]" : ""); > + > + LIBCAMERA_TRACEPOINT(request_complete, this); > } > > /** > @@ -276,6 +285,8 @@ void Request::complete() > */ > bool Request::completeBuffer(FrameBuffer *buffer) > { > + LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer); > + > int ret = pending_.erase(buffer); > ASSERT(ret == 1); >
diff --git a/include/libcamera/internal/tracepoints/buffer_enums.tp b/include/libcamera/internal/tracepoints/buffer_enums.tp new file mode 100644 index 00000000..fdbea69a --- /dev/null +++ b/include/libcamera/internal/tracepoints/buffer_enums.tp @@ -0,0 +1,9 @@ +TRACEPOINT_ENUM( + libcamera, + buffer_status, + TP_ENUM_VALUES( + ctf_enum_value("FrameSuccess", 0) + ctf_enum_value("FrameError", 1) + ctf_enum_value("FrameCancelled", 2) + ) +) diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build index 2dd6733e..fee0758f 100644 --- a/include/libcamera/internal/tracepoints/meson.build +++ b/include/libcamera/internal/tracepoints/meson.build @@ -1,4 +1,9 @@ # SPDX-License-Identifier: CC0-1.0 +# enum files must go first tracepoint_files = files([ + 'buffer_enums.tp', + 'request_enums.tp', + + 'request.tp', ]) diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp new file mode 100644 index 00000000..dd28a714 --- /dev/null +++ b/include/libcamera/internal/tracepoints/request.tp @@ -0,0 +1,68 @@ +#include <libcamera/buffer.h> +#include <libcamera/request.h> + +TRACEPOINT_EVENT_CLASS( + libcamera, + request, + TP_ARGS( + libcamera::Request *, req + ), + TP_FIELDS( + ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req)) + ctf_integer(uint64_t, cookie, req->cookie()) + ctf_enum(libcamera, request_status, uint32_t, status, req->status()) + ) +) + +TRACEPOINT_EVENT_INSTANCE( + libcamera, + request, + request_construct, + TP_ARGS( + libcamera::Request *, req + ) +) + +TRACEPOINT_EVENT_INSTANCE( + libcamera, + request, + request_destroy, + TP_ARGS( + libcamera::Request *, req + ) +) + +TRACEPOINT_EVENT_INSTANCE( + libcamera, + request, + request_reuse, + TP_ARGS( + libcamera::Request *, req + ) +) + +TRACEPOINT_EVENT_INSTANCE( + libcamera, + request, + request_complete, + TP_ARGS( + libcamera::Request *, req + ) +) + + +TRACEPOINT_EVENT( + libcamera, + request_complete_buffer, + TP_ARGS( + libcamera::Request *, req, + libcamera::FrameBuffer *, buf + ), + TP_FIELDS( + ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req)) + ctf_integer(uint64_t, cookie, req->cookie()) + ctf_integer(int, status, req->status()) + ctf_integer_hex(uintptr_t, buffer, reinterpret_cast<uintptr_t>(buf)) + ctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status) + ) +) diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp new file mode 100644 index 00000000..371f6544 --- /dev/null +++ b/include/libcamera/internal/tracepoints/request_enums.tp @@ -0,0 +1,9 @@ +TRACEPOINT_ENUM( + libcamera, + request_status, + TP_ENUM_VALUES( + ctf_enum_value("RequestPending", 0) + ctf_enum_value("RequestComplete", 1) + ctf_enum_value("RequestCancelled", 2) + ) +) diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index ae8b1660..a68684ef 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -16,6 +16,7 @@ #include "libcamera/internal/camera_controls.h" #include "libcamera/internal/log.h" +#include "libcamera/internal/tracepoints.h" /** * \file request.h @@ -85,10 +86,14 @@ Request::Request(Camera *camera, uint64_t cookie) * \todo: Add a validator for metadata controls. */ metadata_ = new ControlList(controls::controls); + + LIBCAMERA_TRACEPOINT(request_construct, this); } Request::~Request() { + LIBCAMERA_TRACEPOINT(request_destroy, this); + delete metadata_; delete controls_; delete validator_; @@ -106,6 +111,8 @@ Request::~Request() */ void Request::reuse(ReuseFlag flags) { + LIBCAMERA_TRACEPOINT(request_reuse, this); + pending_.clear(); if (flags & ReuseBuffers) { for (auto pair : bufferMap_) { @@ -259,6 +266,8 @@ void Request::complete() LOG(Request, Debug) << "Request has completed - cookie: " << cookie_ << (cancelled_ ? " [Cancelled]" : ""); + + LIBCAMERA_TRACEPOINT(request_complete, this); } /** @@ -276,6 +285,8 @@ void Request::complete() */ bool Request::completeBuffer(FrameBuffer *buffer) { + LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer); + int ret = pending_.erase(buffer); ASSERT(ret == 1);