[{"id":25625,"web_url":"https://patchwork.libcamera.org/comment/25625/","msgid":"<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>","date":"2022-10-27T16:08:33","subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Nicholas\n\nOn Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via libcamera-devel wrote:\n> From: Nicholas Roth \n\nHow come this gets inserted here by git when the commit author and the\nsender have the same identity ?\n\n>\n> The raspberrypi IPA is missing thread-safety annotations, which breaks\n> the build.\n>\n> Add required thread-safety annotations.\n>\n> ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex\n> 'mutex_' is still held at the end of function [-Werror,-Wthread-safety-analysis]\n> void lock() { mutex_.lock(); }\n> ^\n> ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex\n> acquired here\n> void lock() { mutex_.lock(); }\n> ^\n> ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing\n> mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]\n> void unlock() { mutex_.unlock(); }\n> ^\n\nnit: double empty line\n\n>\n> Signed-off-by: Nicholas Roth \n\nI've not followed much the thread annotation thing, but this seems\ncorrect to me, even more so if it silences a compilation error.\n\nReviewed-by: Jacopo Mondi \n\n> ---\n> src/ipa/raspberrypi/controller/metadata.h | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/controller/metadata.h b/src/ipa/raspberrypi/controller/metadata.h\n> index 0f7ebfaf..870b6e26 100644\n> --- a/src/ipa/raspberrypi/controller/metadata.h\n> +++ b/src/ipa/raspberrypi/controller/metadata.h\n> @@ -13,9 +13,11 @@\n> #include \n> #include \n>\n> +#include \n> +\n> namespace RPiController {\n>\n> -class Metadata\n> +class LIBCAMERA_TSA_CAPABILITY(\"mutex\") Metadata\n> {\n> public:\n> \tMetadata() = default;\n> @@ -103,8 +105,8 @@ public:\n> \t * locks with the standard lock classes.\n> \t * e.g. std::lock_guard lock(metadata)\n> \t */\n> -\tvoid lock() { mutex_.lock(); }\n> -\tvoid unlock() { mutex_.unlock(); }\n> +\tvoid lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }\n> +\tvoid unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }\n>\n> private:\n> \tmutable std::mutex mutex_;\n> --\n> 2.34.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 7B46DBD16B\n\tfor ;\n\tThu, 27 Oct 2022 16:08:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 980FB62F9A;\n\tThu, 27 Oct 2022 18:08:37 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0A9C62F89\n\tfor ;\n\tThu, 27 Oct 2022 18:08:36 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id A5A34E000E;\n\tThu, 27 Oct 2022 16:08:35 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666886917;\n\tbh=HV4bSXRh+xYGa81U0y07s07Rd9z3RfY/sT0lKF76Fmc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tFda9iW/Z3rQY2M3wBG6tO2ARh6SXeZcDKXJAoUiVBFc33beTJTYO6Vb3VIYkjaMO\n\t+K1DhkioCyi9odf5P4F8Y5Q/SAgP2wOV3N0+u/xSkoUK+bj0qAUTR75LIewaRWm1lU\n\tolgQb4kDM/Hh1Qmx/H6S5JtwjDGnp0gmbEKJzK51Hvl4rRJ6N1Pg9iBXYf6cC6sJrq\n\tcNztXdqbT3M5zsKkDZ50C5UE50urHh3hROzR4jvyBolABymk66GFeMEGFWjVt3IOWg\n\tJyXKnWY3HJ985JqYr8Iv5QRIF6GKTf/jhTjPVvNa9ROioPupghUt1kgDzagTDDxk0+\n\tLWSmR1REtskhA==","Date":"Thu, 27 Oct 2022 18:08:33 +0200","To":"Nicholas Roth via libcamera-devel ","Message-ID":"<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>","References":"\n\t<20221027055515.321791-1-nicholas@rothemail.net>\n\t<20221027055515.321791-3-nicholas@rothemail.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221027055515.321791-3-nicholas@rothemail.net>","Subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"nicholas@rothemail.net","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25633,"web_url":"https://patchwork.libcamera.org/comment/25633/","msgid":"","date":"2022-10-27T22:04:31","subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","submitter":{"id":137,"url":"https://patchwork.libcamera.org/api/people/137/","name":"Nicholas Roth","email":"nicholas@rothemail.net"},"content":"> How come this gets inserted here by git when the commit author and the\nsender have the same identity ?\n\nI'm really not sure. Maybe because I specify \"--from\" in git-send-email? Is\nthis not a standard thing to do?\ngit send-email --compose --from=nicholas@rothemail.net --reply-to=\nlibcamera-devel@lists.libcamera.org --to=libcamera-devel@lists.libcamera.org\n--subject=\"libcamera Android Enhancements\" --thread --no-chain-reply-to\n--annotate -v2 main\n\n> nit: double empty line\nIf you're referring to the commit message, the line below the error is for\nthe carat that Clang uses to indicate the position of the error. Happy to\nnot skip a line before \"Signed-off-by\" if it would help.\n\n> this seems correct to me, even more so if it silences a compilation error.\nDo I need an LGTM from anyone else? What would the next step be here?\n\nOn Thu, Oct 27, 2022 at 11:08 AM Jacopo Mondi wrote:\n\n> Hi Nicholas\n>\n> On Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via\n> libcamera-devel wrote:\n> > From: Nicholas Roth \n>\n> How come this gets inserted here by git when the commit author and the\n> sender have the same identity ?\n>\n> >\n> > The raspberrypi IPA is missing thread-safety annotations, which breaks\n> > the build.\n> >\n> > Add required thread-safety annotations.\n> >\n> > ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex\n> > 'mutex_' is still held at the end of function\n> [-Werror,-Wthread-safety-analysis]\n> > void lock() { mutex_.lock(); }\n> > ^\n> > ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex\n> > acquired here\n> > void lock() { mutex_.lock(); }\n> > ^\n> > ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing\n> > mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]\n> > void unlock() { mutex_.unlock(); }\n> > ^\n>\n> nit: double empty line\n>\n> >\n> > Signed-off-by: Nicholas Roth \n>\n> I've not followed much the thread annotation thing, but this seems\n> correct to me, even more so if it silences a compilation error.\n>\n> Reviewed-by: Jacopo Mondi \n>\n> > ---\n> > src/ipa/raspberrypi/controller/metadata.h | 8 +++++---\n> > 1 file changed, 5 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/metadata.h\n> b/src/ipa/raspberrypi/controller/metadata.h\n> > index 0f7ebfaf..870b6e26 100644\n> > --- a/src/ipa/raspberrypi/controller/metadata.h\n> > +++ b/src/ipa/raspberrypi/controller/metadata.h\n> > @@ -13,9 +13,11 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > +\n> > namespace RPiController {\n> >\n> > -class Metadata\n> > +class LIBCAMERA_TSA_CAPABILITY(\"mutex\") Metadata\n> > {\n> > public:\n> > Metadata() = default;\n> > @@ -103,8 +105,8 @@ public:\n> > * locks with the standard lock classes.\n> > * e.g. std::lock_guard lock(metadata)\n> > */\n> > - void lock() { mutex_.lock(); }\n> > - void unlock() { mutex_.unlock(); }\n> > + void lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }\n> > + void unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }\n> >\n> > private:\n> > mutable std::mutex mutex_;\n> > --\n> > 2.34.1\n> >\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 B3135BD16B\n\tfor ;\n\tThu, 27 Oct 2022 22:04:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 150E662FA3;\n\tFri, 28 Oct 2022 00:04:45 +0200 (CEST)","from mail-io1-xd33.google.com (mail-io1-xd33.google.com\n\t[IPv6:2607:f8b0:4864:20::d33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4613F61F4B\n\tfor ;\n\tFri, 28 Oct 2022 00:04:43 +0200 (CEST)","by mail-io1-xd33.google.com with SMTP id 63so3026085iov.8\n\tfor ;\n\tThu, 27 Oct 2022 15:04:43 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666908285;\n\tbh=I7znLi26rcFjaEGPVDC8f4W04rzmJ46slCJJquXAyLs=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=wsBxJ9/2XW7+i/2L/tMBdtSnDLSw9vZVEAOCXdSCDEAHHxrcmeeVWa7am4U3VkDsT\n\tsp69i644nyrC9B8OGrazr15TJD9XgI7KI06xY8JYxP7lQXc24arkVFMXspmuiVHQyF\n\tC7TQpyOL2fYwfLu/YbWFTQu/WDeAq4yAFAKx7gCbFLTUMSDeXklNFwB5XjIAKU/1xe\n\tB404Qc2G0DsO/gQyKiNWhy4BIwswf3xWrEWKC/FMObFljO1YbVS7/C7/dEk6+KUFf4\n\tkrXe7QBTCOvK3FYmWTQ3p9IbHo7VN0yoVczvOEBtbF3ItV8GKzPP5lunaMGZtChP3n\n\tL2D9ZzyAjpfEA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=rothemail-net.20210112.gappssmtp.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=jJR7V613+7aEw1CE6rcRQjO10nRS0TN++BjWwxkHklU=;\n\tb=BQkKXRd9c46MgcZugjMgmP00VVh6CSOGiRBUDc5RX0Pu1oXsHfDKXIOEcvdmKaYUUM\n\tNRMNpRBokuNseQ6MvQ7c1kXGzATgTnD/rPSuriUaG26rKG3H78YGV/qKlZqib+M9oN5Y\n\t+hqxgotHqFipYSyJLCqzuS2leclqrwQCA4NKPL9BoWJcTj4K2PvKPz/9jtk6XfTkeUT4\n\tf/rwkrydmJpCvKf2+8oFJWCMmrOEv/7A384GrpdPUN6jQFgrghEB98MynhsNl1zZStuv\n\trNbZD/4ENsaOkBE32zX/0N+WbDQxOEMoVLdOVKV7bfEcDloQWKfeCNRoqhgZ9J1sW/ae\n\tOvFw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=rothemail-net.20210112.gappssmtp.com\n\theader.i=@rothemail-net.20210112.gappssmtp.com header.b=\"BQkKXRd9\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=jJR7V613+7aEw1CE6rcRQjO10nRS0TN++BjWwxkHklU=;\n\tb=XqLX+hLOUTQRh/wn7Y7B6joGJdHx1RP7rstq+WFT5E3h1HrTGx0iV2TJoWZ9yYUrLm\n\tpbj65xYqbidXkDfibTbkILKT9c05rpV4rsaNYk6YBXq7Z0jY09nS859YrhlW1Fw+r+vh\n\t3PFl5K5ohKFBC/cvVKzw0YGs1I6T58PqpiLVipgyjvUzTI/pMnro6pKkj724c4j5FMst\n\tNrZgNYyOoYH9mD7ga82TS5vuQrVDmyECAFTPa1CTz45G93BeGY+HbW9F3OB8ZM5cWn6L\n\tRJ+FT/WyNn5shqPbkrmEZVGwsmbblVa+LqW26ShgFX87JOX+6ymoLG986quRQInZTarw\n\tpD8A==","X-Gm-Message-State":"ACrzQf23OUn2hw6yjCQFr+DszcWYDuyCVo3XSpnd67CvzSD93n95ydoc\n\t59Qoza86yc4IXUq1NpcxRRSs+ARqggGJ/m6v4qegxzWYcGbTXRNU","X-Google-Smtp-Source":"AMsMyM6Wt6e7gd4QdNgsyQcMG2JwdJZ4VHndN5QeD5ObkU26A0Ct13JXsftQ+pwzOYlJIhvmoz46TtxUMGX9c4l8u38=","X-Received":"by 2002:a05:6602:2e8d:b0:6bd:543c:6fd7 with SMTP id\n\tm13-20020a0566022e8d00b006bd543c6fd7mr14495976iow.134.1666908281889;\n\tThu, 27 Oct 2022 15:04:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20221027055515.321791-1-nicholas@rothemail.net>\n\t<20221027055515.321791-3-nicholas@rothemail.net>\n\t<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>","In-Reply-To":"<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>","Date":"Thu, 27 Oct 2022 17:04:31 -0500","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000000400fd05ec0b542e\"","Subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","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","From":"Nicholas Roth via libcamera-devel ","Reply-To":"Nicholas Roth ","Cc":"Nicholas Roth via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25636,"web_url":"https://patchwork.libcamera.org/comment/25636/","msgid":"<20221028073231.vsj7rdn624tmdzro@uno.localdomain>","date":"2022-10-28T07:32:31","subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Nicholas\n\nOn Thu, Oct 27, 2022 at 05:04:31PM -0500, Nicholas Roth wrote:\n> > How come this gets inserted here by git when the commit author and the\n> sender have the same identity ?\n>\n> I'm really not sure. Maybe because I specify \"--from\" in git-send-email? Is\n> this not a standard thing to do?\n> git send-email --compose --from=nicholas@rothemail.net --reply-to=\n> libcamera-devel@lists.libcamera.org --to=libcamera-devel@lists.libcamera.org\n> --subject=\"libcamera Android Enhancements\" --thread --no-chain-reply-to\n> --annotate -v2 main\n\nOh, so many options :)\n\nI'm not sure if --from is the culprit, it might be, but be aware the\nsender can be picked up automatically by git global config variables\nand you most probably don't need this option.\n\nI would discourage --reply-to, it mangles the \"Reply-To\" field of your\npatches, something that might even break someone's scripts. Just\nspcify the mailing list in the receiver or cc list and people will\nreply to it.\n\n--thread should be on by default if I get it right, same for\n--no-chain-reply-to\n\nTry to drop the --from and --reply-to options and see how it goes\n\n\n>\n> > nit: double empty line\n> If you're referring to the commit message, the line below the error is for\n> the carat that Clang uses to indicate the position of the error. Happy to\n> not skip a line before \"Signed-off-by\" if it would help.\n\nAh sorry, missed it\n\n>\n> > this seems correct to me, even more so if it silences a compilation error.\n> Do I need an LGTM from anyone else? What would the next step be here?\n\nThe policy is usually to get at least two reviewed-by tags. Once the\nseries is fully reviewed it will be picked up, run through a few\ncompiler matrices and if it touches the Android HAL through CTS, then\nmerged.\n\nThe process is still a bit fuzzy, we don't have per-component\nmaintainers nor someone designed specifically for the\n'collect-test-merge' part, but it's usually Laurent and Kieran taking\ncare of that.\n\n>\n> On Thu, Oct 27, 2022 at 11:08 AM Jacopo Mondi wrote:\n>\n> > Hi Nicholas\n> >\n> > On Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via\n> > libcamera-devel wrote:\n> > > From: Nicholas Roth \n> >\n> > How come this gets inserted here by git when the commit author and the\n> > sender have the same identity ?\n> >\n> > >\n> > > The raspberrypi IPA is missing thread-safety annotations, which breaks\n> > > the build.\n> > >\n> > > Add required thread-safety annotations.\n> > >\n> > > ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex\n> > > 'mutex_' is still held at the end of function\n> > [-Werror,-Wthread-safety-analysis]\n> > > void lock() { mutex_.lock(); }\n> > > ^\n> > > ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex\n> > > acquired here\n> > > void lock() { mutex_.lock(); }\n> > > ^\n> > > ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing\n> > > mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]\n> > > void unlock() { mutex_.unlock(); }\n> > > ^\n> >\n> > nit: double empty line\n> >\n> > >\n> > > Signed-off-by: Nicholas Roth \n> >\n> > I've not followed much the thread annotation thing, but this seems\n> > correct to me, even more so if it silences a compilation error.\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > ---\n> > > src/ipa/raspberrypi/controller/metadata.h | 8 +++++---\n> > > 1 file changed, 5 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/metadata.h\n> > b/src/ipa/raspberrypi/controller/metadata.h\n> > > index 0f7ebfaf..870b6e26 100644\n> > > --- a/src/ipa/raspberrypi/controller/metadata.h\n> > > +++ b/src/ipa/raspberrypi/controller/metadata.h\n> > > @@ -13,9 +13,11 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > namespace RPiController {\n> > >\n> > > -class Metadata\n> > > +class LIBCAMERA_TSA_CAPABILITY(\"mutex\") Metadata\n> > > {\n> > > public:\n> > > Metadata() = default;\n> > > @@ -103,8 +105,8 @@ public:\n> > > * locks with the standard lock classes.\n> > > * e.g. std::lock_guard lock(metadata)\n> > > */\n> > > - void lock() { mutex_.lock(); }\n> > > - void unlock() { mutex_.unlock(); }\n> > > + void lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }\n> > > + void unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }\n> > >\n> > > private:\n> > > mutable std::mutex mutex_;\n> > > --\n> > > 2.34.1\n> > >\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 627D1BDB16\n\tfor ;\n\tFri, 28 Oct 2022 07:32:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9CCEC62FB4;\n\tFri, 28 Oct 2022 09:32:34 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B47762F41\n\tfor ;\n\tFri, 28 Oct 2022 09:32:33 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id A8AD31BF20E;\n\tFri, 28 Oct 2022 07:32:32 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666942354;\n\tbh=sUgjgDHWVQMJ+YXxDKUlbLwGRXBgavG9iRuX8pyGTrc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QSV0Mw07mCFmBohVEkSxtgRMdY8yrEKcwrYYQoHOqN4ybhNxaPXOYDbNka8RgcuKC\n\teoT1to07bZEXannCFF/OrTImvjmNXd6iN9nuv0rSjtKs4dFd0wjHXz8XyXneTalxfe\n\ti0xEggw07DpK66v24z7fFO6dcV8W6K2m0RyyakPbnlwgKjKJ2RsfgxSwael1CV40S8\n\tuoJ1k/bQkgTeB2aE270QfAZwJClrHhyUnOBem33q6mjrHzmMXRdlNqH624yIIlR5cu\n\t4v7L4BnOCcroZHxoCrV1EtbhcjnG3F4S6eqgp57z3Ap1SOKUQhi4dT0ZQHN24Xmjf5\n\tPs6ru0nw3Z51Q==","Date":"Fri, 28 Oct 2022 09:32:31 +0200","To":"Nicholas Roth ","Message-ID":"<20221028073231.vsj7rdn624tmdzro@uno.localdomain>","References":"<20221027055515.321791-1-nicholas@rothemail.net>\n\t<20221027055515.321791-3-nicholas@rothemail.net>\n\t<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Nicholas Roth via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25639,"web_url":"https://patchwork.libcamera.org/comment/25639/","msgid":"<166694537231.2677993.799010278553359671@Monstersaurus>","date":"2022-10-28T08:22:52","subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Nicholas,\n\nQuoting Jacopo Mondi via libcamera-devel (2022-10-27 17:08:33)\n> Hi Nicholas\n> \n> On Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via libcamera-devel wrote:\n> > From: Nicholas Roth \n> \n> How come this gets inserted here by git when the commit author and the\n> sender have the same identity ?\n> \n> >\n> > The raspberrypi IPA is missing thread-safety annotations, which breaks\n> > the build.\n> >\n> > Add required thread-safety annotations.\n> >\n> > ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex\n> > 'mutex_' is still held at the end of function [-Werror,-Wthread-safety-analysis]\n> > void lock() { mutex_.lock(); }\n> > ^\n> > ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex\n> > acquired here\n> > void lock() { mutex_.lock(); }\n> > ^\n> > ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing\n> > mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]\n> > void unlock() { mutex_.unlock(); }\n> > ^\n> \n> nit: double empty line\n> \n> >\n> > Signed-off-by: Nicholas Roth \n> \n> I've not followed much the thread annotation thing, but this seems\n> correct to me, even more so if it silences a compilation error.\n> \n> Reviewed-by: Jacopo Mondi \n\nThese tags are valuable to you. We desire two Reviewed-by (or Acked-by:)\ntags to get a patch merged.\n\nSo when you're given one, please add it to your patch so that it gets\nincluded when you repost a later version.\n\nI also think this patch is good, but I'll add my tag to the latest\nversion, however in this instance - because it's under\nsrc/ipa/raspberrypi/ - we'll want an ack from RPi too.\n\n--\nKieran\n\n\n> \n> > ---\n> > src/ipa/raspberrypi/controller/metadata.h | 8 +++++---\n> > 1 file changed, 5 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/metadata.h b/src/ipa/raspberrypi/controller/metadata.h\n> > index 0f7ebfaf..870b6e26 100644\n> > --- a/src/ipa/raspberrypi/controller/metadata.h\n> > +++ b/src/ipa/raspberrypi/controller/metadata.h\n> > @@ -13,9 +13,11 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > +\n> > namespace RPiController {\n> >\n> > -class Metadata\n> > +class LIBCAMERA_TSA_CAPABILITY(\"mutex\") Metadata\n> > {\n> > public:\n> > Metadata() = default;\n> > @@ -103,8 +105,8 @@ public:\n> > * locks with the standard lock classes.\n> > * e.g. std::lock_guard lock(metadata)\n> > */\n> > - void lock() { mutex_.lock(); }\n> > - void unlock() { mutex_.unlock(); }\n> > + void lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }\n> > + void unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }\n> >\n> > private:\n> > mutable std::mutex mutex_;\n> > --\n> > 2.34.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 70A94BD16B\n\tfor ;\n\tFri, 28 Oct 2022 08:22:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA3A862FB8;\n\tFri, 28 Oct 2022 10:22:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0DA362F41\n\tfor ;\n\tFri, 28 Oct 2022 10:22:55 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E5F66BE;\n\tFri, 28 Oct 2022 10:22:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666945376;\n\tbh=uVJWAjEzksMCrcamGmmp1tM9vwlwOoAuio6FTsKG9rQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ntVXNpKGETZ5OiiImb8QW3PaILwmAZ5ezYibpVUqfz19cAD68NbcUdQ5gq1MmSulM\n\tPHCsnz1v4Oh7A0js+/Fk2uw6Jub1V5gJwrtb033WXzFK6o+DQtpfKvdib5hCaDIPRl\n\t2r99N/yk8CsJIJoKhkzXA+pjtzIiVsSEZKyn/Wff7e1GyfIVgXFE+kp98KDf/BLMBr\n\tg94sV1lIkfZoSC41CDq0zZrPrCmTMNJmRSvESU+pBEXZk637VKyuQdTiIzDj8hukB5\n\tZMRpit90wK8mwC5XGfz3NXB5irWWGg8bjpyAfN28yzhcclrtFfzPTGmljcAVJ9lgkI\n\tWB9cxFWszMVSA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666945375;\n\tbh=uVJWAjEzksMCrcamGmmp1tM9vwlwOoAuio6FTsKG9rQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=NaPGBVIziTXejMcZil62gcVHdbsrw5fCWiEYVAnb8BPY/oGNZr5AasbM9+/PvMuJf\n\t9Qw1TLz42e4BErZS8Rat39wdB1dBpU1vceiQf66AUGnPBZEAhOYjvfLCJjKJssrdpP\n\tD0eFsbGt67QycHXHJLp9XzVNpEzLxupynhySLGO0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NaPGBVIz\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>","References":"\n\t<20221027055515.321791-1-nicholas@rothemail.net>\n\t<20221027055515.321791-3-nicholas@rothemail.net>\n\t<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>","To":"Jacopo Mondi , Nicholas Roth via libcamera-devel\n\t","Date":"Fri, 28 Oct 2022 09:22:52 +0100","Message-ID":"<166694537231.2677993.799010278553359671@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"nicholas@rothemail.net","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25640,"web_url":"https://patchwork.libcamera.org/comment/25640/","msgid":"<166694577425.1769154.794136028596173926@Monstersaurus>","date":"2022-10-28T08:29:34","subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2022-10-28 08:32:31)\n> Hi Nicholas\n> \n> On Thu, Oct 27, 2022 at 05:04:31PM -0500, Nicholas Roth wrote:\n> > > How come this gets inserted here by git when the commit author and the\n> > sender have the same identity ?\n> >\n> > I'm really not sure. Maybe because I specify \"--from\" in git-send-email? Is\n> > this not a standard thing to do?\n> > git send-email --compose --from=nicholas@rothemail.net --reply-to=\n> > libcamera-devel@lists.libcamera.org --to=libcamera-devel@lists.libcamera.org\n> > --subject=\"libcamera Android Enhancements\" --thread --no-chain-reply-to\n> > --annotate -v2 main\n> \n> Oh, so many options :)\n> \n> I'm not sure if --from is the culprit, it might be, but be aware the\n> sender can be picked up automatically by git global config variables\n> and you most probably don't need this option.\n> \n> I would discourage --reply-to, it mangles the \"Reply-To\" field of your\n> patches, something that might even break someone's scripts. Just\n> spcify the mailing list in the receiver or cc list and people will\n> reply to it.\n> \n> --thread should be on by default if I get it right, same for\n> --no-chain-reply-to\n> \n> Try to drop the --from and --reply-to options and see how it goes\n> \n> \n> >\n> > > nit: double empty line\n> > If you're referring to the commit message, the line below the error is for\n> > the carat that Clang uses to indicate the position of the error. Happy to\n> > not skip a line before \"Signed-off-by\" if it would help.\n> \n> Ah sorry, missed it\n> \n> >\n> > > this seems correct to me, even more so if it silences a compilation error.\n> > Do I need an LGTM from anyone else? What would the next step be here?\n> \n> The policy is usually to get at least two reviewed-by tags. Once the\n> series is fully reviewed it will be picked up, run through a few\n> compiler matrices and if it touches the Android HAL through CTS, then\n> merged.\n\nYes, we need to write this down clearly somewhere.\n\n> The process is still a bit fuzzy, we don't have per-component\n> maintainers nor someone designed specifically for the\n> 'collect-test-merge' part, but it's usually Laurent and Kieran taking\n> care of that.\n\nI suspect that's mostly because I have infrastructure to run all the\ntests in the lab here. But if we could get that more automated, with CTS\nand testing on RPi/ other boards - we could hook up better means for\nintegration. Just seems to take so much time to do CI development etc.\n\n\nThe only other missing piece here, is that patches touching code\nsupporting the RPi should be either reviewed or Acked by a developer at\nRPi. (The same would be said later of other platforms supported by their\nvendors, but we cover the rest at the moment).\n\n--\nKieran\n\n\n> > On Thu, Oct 27, 2022 at 11:08 AM Jacopo Mondi wrote:\n> >\n> > > Hi Nicholas\n> > >\n> > > On Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via\n> > > libcamera-devel wrote:\n> > > > From: Nicholas Roth \n> > >\n> > > How come this gets inserted here by git when the commit author and the\n> > > sender have the same identity ?\n> > >\n> > > >\n> > > > The raspberrypi IPA is missing thread-safety annotations, which breaks\n> > > > the build.\n> > > >\n> > > > Add required thread-safety annotations.\n> > > >\n> > > > ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex\n> > > > 'mutex_' is still held at the end of function\n> > > [-Werror,-Wthread-safety-analysis]\n> > > > void lock() { mutex_.lock(); }\n> > > > ^\n> > > > ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex\n> > > > acquired here\n> > > > void lock() { mutex_.lock(); }\n> > > > ^\n> > > > ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing\n> > > > mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]\n> > > > void unlock() { mutex_.unlock(); }\n> > > > ^\n> > >\n> > > nit: double empty line\n> > >\n> > > >\n> > > > Signed-off-by: Nicholas Roth \n> > >\n> > > I've not followed much the thread annotation thing, but this seems\n> > > correct to me, even more so if it silences a compilation error.\n> > >\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > ---\n> > > > src/ipa/raspberrypi/controller/metadata.h | 8 +++++---\n> > > > 1 file changed, 5 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/raspberrypi/controller/metadata.h\n> > > b/src/ipa/raspberrypi/controller/metadata.h\n> > > > index 0f7ebfaf..870b6e26 100644\n> > > > --- a/src/ipa/raspberrypi/controller/metadata.h\n> > > > +++ b/src/ipa/raspberrypi/controller/metadata.h\n> > > > @@ -13,9 +13,11 @@\n> > > > #include \n> > > > #include \n> > > >\n> > > > +#include \n> > > > +\n> > > > namespace RPiController {\n> > > >\n> > > > -class Metadata\n> > > > +class LIBCAMERA_TSA_CAPABILITY(\"mutex\") Metadata\n> > > > {\n> > > > public:\n> > > > Metadata() = default;\n> > > > @@ -103,8 +105,8 @@ public:\n> > > > * locks with the standard lock classes.\n> > > > * e.g. std::lock_guard lock(metadata)\n> > > > */\n> > > > - void lock() { mutex_.lock(); }\n> > > > - void unlock() { mutex_.unlock(); }\n> > > > + void lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }\n> > > > + void unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }\n> > > >\n> > > > private:\n> > > > mutable std::mutex mutex_;\n> > > > --\n> > > > 2.34.1\n> > > >\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 14467BDB16\n\tfor ;\n\tFri, 28 Oct 2022 08:29:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D39162FB9;\n\tFri, 28 Oct 2022 10:29:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 025AD62F41\n\tfor ;\n\tFri, 28 Oct 2022 10:29:36 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8710B6BE;\n\tFri, 28 Oct 2022 10:29:36 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666945778;\n\tbh=GiVaXS3/ako9L7UABm6ZVa/ppvtb25s/OIOWad1t4Uw=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dqBR3AuhCnvvDu8YhJT18mHIzovzA1jGHjytMZeR6+h6HbiZtLK2iTMIAmRwjvtpM\n\tZBSiF7e8TaKhDnqvQr6TihK21Qqq/QZt6CpDy6L1rJj6roIoeyvQNmkzPNGu6QKAOn\n\tPZnzgeFe+vAT/5nWH5rHJ9MdR0l/L9dhVCyGo31wDCBwwjLB3pQNLdT7g76xDd0tGq\n\tOPFekqMJj41CMn6ziT4wXRJN7qXEEGklgI7+COqovqjt73VbqJ4prWDOVq7yQyUzdQ\n\tSNah9s+8O0WTvIPr0QvdU4WyprIa//5ayQO3TA3PuUtSSN25ewxqxMG0++Uy0nd/IS\n\topXjPHWqN+hyw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666945776;\n\tbh=GiVaXS3/ako9L7UABm6ZVa/ppvtb25s/OIOWad1t4Uw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Ph6wZLVHFFrPZXnN3XQgO1wpLIOlUTVLyHda/AvHk+nIVRIWdFicdNvxED9j+PfX6\n\tLzmh9rro/RIP/JCGDL1q+iNbA983MQSfjrm7VfPxEDwr+2+UzksfXJfLJ7CThiHxRO\n\tBF5EWtMD+zZc/lkhl0DJv5/1mL7mYORZwXq86SzA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Ph6wZLVH\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221028073231.vsj7rdn624tmdzro@uno.localdomain>","References":"<20221027055515.321791-1-nicholas@rothemail.net>\n\t<20221027055515.321791-3-nicholas@rothemail.net>\n\t<20221027160833.vqoivtjoxkd42i3v@uno.localdomain>\n\t\n\t<20221028073231.vsj7rdn624tmdzro@uno.localdomain>","To":"Jacopo Mondi , Jacopo Mondi via libcamera-devel\n\t, \n\tNicholas Roth ","Date":"Fri, 28 Oct 2022 09:29:34 +0100","Message-ID":"<166694577425.1769154.794136028596173926@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety\n\tannotations","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Nicholas Roth via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]