[{"id":13571,"web_url":"https://patchwork.libcamera.org/comment/13571/","msgid":"<20201103001950.GF30506@pendragon.ideasonboard.com>","date":"2020-11-03T00:19:50","subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: request: Add\n\ttracepoints","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Oct 30, 2020 at 05:57:53PM +0900, Paul Elder wrote:\n> Add and use tracepoints in Request. Requests are core to libcamera\n> operation, thus detecting delays in their processing is important, and\n> serves as a good usage example of tracepoints.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v4:\n> - move enum tp definitions to \"header\" tp files\n> - add them to meson accordingly\n> - remove cancelled field from request_complete_buffer tracepoint\n> \n> Changes in v3:\n> - expand the changelog\n> - add enum tracepoint values so that request status and buffer status\n> are strings instead of ints in the trace\n> - remove cancelled from all request tracepoints, except complete_buffer\n> - expand the complete_buffer tracepoint to include information on the\n> buffer\n> - display the address of the request in all tracepoints\n> - remove tracepoints for add_buffer and find_buffer\n> \n> Changes in v2:\n> - remove tracepoints from uvcvideo\n> - remove comment in changelog that this is only used for demonstration\n> - use Request pointers instead of feeding the fields manually to the\n> tracepoint\n> ---\n> .../internal/tracepoints/buffer_enums.tp | 9 +++\n> .../internal/tracepoints/meson.build | 5 ++\n> .../libcamera/internal/tracepoints/request.tp | 68 +++++++++++++++++++\n> .../internal/tracepoints/request_enums.tp | 9 +++\n> src/libcamera/request.cpp | 11 +++\n> 5 files changed, 102 insertions(+)\n> create mode 100644 include/libcamera/internal/tracepoints/buffer_enums.tp\n> create mode 100644 include/libcamera/internal/tracepoints/request.tp\n> create mode 100644 include/libcamera/internal/tracepoints/request_enums.tp\n> \n> diff --git a/include/libcamera/internal/tracepoints/buffer_enums.tp b/include/libcamera/internal/tracepoints/buffer_enums.tp\n> new file mode 100644\n> index 00000000..fdbea69a\n> --- /dev/null\n> +++ b/include/libcamera/internal/tracepoints/buffer_enums.tp\n> @@ -0,0 +1,9 @@\n> +TRACEPOINT_ENUM(\n> +\tlibcamera,\n> +\tbuffer_status,\n> +\tTP_ENUM_VALUES(\n> +\t\tctf_enum_value(\"FrameSuccess\", 0)\n> +\t\tctf_enum_value(\"FrameError\", 1)\n> +\t\tctf_enum_value(\"FrameCancelled\", 2)\n> +\t)\n> +)\n> diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build\n> index 2dd6733e..fee0758f 100644\n> --- a/include/libcamera/internal/tracepoints/meson.build\n> +++ b/include/libcamera/internal/tracepoints/meson.build\n> @@ -1,4 +1,9 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> +# enum files must go first\n> tracepoint_files = files([\n> + 'buffer_enums.tp',\n> + 'request_enums.tp',\n> +\n> + 'request.tp',\n> ])\n\nHow about\n\n# enum files must go first\ntracepoint_files = files([\n 'buffer_enums.tp',\n 'request_enums.tp',\n])\n\ntracepoint_files += files([\n 'request.tp',\n])\n\n?\n\n> diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp\n> new file mode 100644\n> index 00000000..dd28a714\n> --- /dev/null\n> +++ b/include/libcamera/internal/tracepoints/request.tp\n> @@ -0,0 +1,68 @@\n> +#include \n> +#include \n> +\n> +TRACEPOINT_EVENT_CLASS(\n> +\tlibcamera,\n> +\trequest,\n> +\tTP_ARGS(\n> +\t\tlibcamera::Request *, req\n> +\t),\n> +\tTP_FIELDS(\n> +\t\tctf_integer_hex(uintptr_t, request, reinterpret_cast(req))\n> +\t\tctf_integer(uint64_t, cookie, req->cookie())\n> +\t\tctf_enum(libcamera, request_status, uint32_t, status, req->status())\n> +\t)\n> +)\n> +\n> +TRACEPOINT_EVENT_INSTANCE(\n> +\tlibcamera,\n> +\trequest,\n> +\trequest_construct,\n> +\tTP_ARGS(\n> +\t\tlibcamera::Request *, req\n> +\t)\n> +)\n> +\n> +TRACEPOINT_EVENT_INSTANCE(\n> +\tlibcamera,\n> +\trequest,\n> +\trequest_destroy,\n> +\tTP_ARGS(\n> +\t\tlibcamera::Request *, req\n> +\t)\n> +)\n> +\n> +TRACEPOINT_EVENT_INSTANCE(\n> +\tlibcamera,\n> +\trequest,\n> +\trequest_reuse,\n> +\tTP_ARGS(\n> +\t\tlibcamera::Request *, req\n> +\t)\n> +)\n> +\n> +TRACEPOINT_EVENT_INSTANCE(\n> +\tlibcamera,\n> +\trequest,\n> +\trequest_complete,\n> +\tTP_ARGS(\n> +\t\tlibcamera::Request *, req\n> +\t)\n> +)\n> +\n> +\n> +TRACEPOINT_EVENT(\n> +\tlibcamera,\n> +\trequest_complete_buffer,\n> +\tTP_ARGS(\n> +\t\tlibcamera::Request *, req,\n> +\t\tlibcamera::FrameBuffer *, buf\n> +\t),\n> +\tTP_FIELDS(\n> +\t\tctf_integer_hex(uintptr_t, request, reinterpret_cast(req))\n> +\t\tctf_integer(uint64_t, cookie, req->cookie())\n> +\t\tctf_integer(int, status, req->status())\n> +\t\tctf_integer_hex(uintptr_t, buffer, reinterpret_cast(buf))\n> +\t\tctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status)\n> +\t)\n> +)\n> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n> new file mode 100644\n> index 00000000..371f6544\n> --- /dev/null\n> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n> @@ -0,0 +1,9 @@\n> +TRACEPOINT_ENUM(\n> +\tlibcamera,\n> +\trequest_status,\n> +\tTP_ENUM_VALUES(\n> +\t\tctf_enum_value(\"RequestPending\", 0)\n> +\t\tctf_enum_value(\"RequestComplete\", 1)\n> +\t\tctf_enum_value(\"RequestCancelled\", 2)\n> +\t)\n> +)\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index ae8b1660..a68684ef 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -16,6 +16,7 @@\n> \n> #include \"libcamera/internal/camera_controls.h\"\n> #include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/tracepoints.h\"\n> \n> /**\n> * \\file request.h\n> @@ -85,10 +86,14 @@ Request::Request(Camera *camera, uint64_t cookie)\n> \t * \\todo: Add a validator for metadata controls.\n> \t */\n> \tmetadata_ = new ControlList(controls::controls);\n> +\n> +\tLIBCAMERA_TRACEPOINT(request_construct, this);\n> }\n> \n> Request::~Request()\n> {\n> +\tLIBCAMERA_TRACEPOINT(request_destroy, this);\n> +\n> \tdelete metadata_;\n> \tdelete controls_;\n> \tdelete validator_;\n> @@ -106,6 +111,8 @@ Request::~Request()\n> */\n> void Request::reuse(ReuseFlag flags)\n> {\n> +\tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> +\n> \tpending_.clear();\n> \tif (flags & ReuseBuffers) {\n> \t\tfor (auto pair : bufferMap_) {\n> @@ -259,6 +266,8 @@ void Request::complete()\n> \tLOG(Request, Debug)\n> \t\t<< \"Request has completed - cookie: \" << cookie_\n> \t\t<< (cancelled_ ? \" [Cancelled]\" : \"\");\n> +\n> +\tLIBCAMERA_TRACEPOINT(request_complete, this);\n> }\n> \n> /**\n> @@ -276,6 +285,8 @@ void Request::complete()\n> */\n> bool Request::completeBuffer(FrameBuffer *buffer)\n> {\n> +\tLIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer);\n> +\n> \tint ret = pending_.erase(buffer);\n> \tASSERT(ret == 1);\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B8566BDB89\n\tfor ;\n\tTue, 3 Nov 2020 00:20:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3242762B91;\n\tTue, 3 Nov 2020 01:20:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B4FB62B90\n\tfor ;\n\tTue, 3 Nov 2020 01:20:40 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 89D9B332;\n\tTue, 3 Nov 2020 01:20:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"P+nghjEh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604362838;\n\tbh=SeALN/6npd1/ZayIZ2hPXttrTd/UYaoUHOAAURWLVlU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P+nghjEhrMUQHQjg8Ghm7Zy3ljNgeEC+vbEKzJkoeDEI53ww3Jhgk8aQzHX5SuzFP\n\tAcuYWd8ZamCwcfixKPwDChvtx/4l15TO5iaNqbr1WvydXQpZNoI2Jcq1Znfclov31o\n\ttiaTZEJdaAK8v0cSfNc8mt1vDhPTyj5Hu8yIjmD0=","Date":"Tue, 3 Nov 2020 02:19:50 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20201103001950.GF30506@pendragon.ideasonboard.com>","References":"<20201030085756.79329-1-paul.elder@ideasonboard.com>\n\t<20201030085756.79329-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201030085756.79329-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: request: Add\n\ttracepoints","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]