[libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety annotations
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Oct 28 10:22:52 CEST 2022
Hi Nicholas,
Quoting Jacopo Mondi via libcamera-devel (2022-10-27 17:08:33)
> Hi Nicholas
>
> On Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via libcamera-devel wrote:
> > From: Nicholas Roth <nicholas at rothemail.net>
>
> How come this gets inserted here by git when the commit author and the
> sender have the same identity ?
>
> >
> > The raspberrypi IPA is missing thread-safety annotations, which breaks
> > the build.
> >
> > Add required thread-safety annotations.
> >
> > ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex
> > 'mutex_' is still held at the end of function [-Werror,-Wthread-safety-analysis]
> > void lock() { mutex_.lock(); }
> > ^
> > ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex
> > acquired here
> > void lock() { mutex_.lock(); }
> > ^
> > ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing
> > mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]
> > void unlock() { mutex_.unlock(); }
> > ^
>
> nit: double empty line
>
> >
> > Signed-off-by: Nicholas Roth <nicholas at rothemail.net>
>
> I've not followed much the thread annotation thing, but this seems
> correct to me, even more so if it silences a compilation error.
>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
These tags are valuable to you. We desire two Reviewed-by (or Acked-by:)
tags to get a patch merged.
So when you're given one, please add it to your patch so that it gets
included when you repost a later version.
I also think this patch is good, but I'll add my tag to the latest
version, however in this instance - because it's under
src/ipa/raspberrypi/ - we'll want an ack from RPi too.
--
Kieran
>
> > ---
> > src/ipa/raspberrypi/controller/metadata.h | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/metadata.h b/src/ipa/raspberrypi/controller/metadata.h
> > index 0f7ebfaf..870b6e26 100644
> > --- a/src/ipa/raspberrypi/controller/metadata.h
> > +++ b/src/ipa/raspberrypi/controller/metadata.h
> > @@ -13,9 +13,11 @@
> > #include <mutex>
> > #include <string>
> >
> > +#include <libcamera/base/thread_annotations.h>
> > +
> > namespace RPiController {
> >
> > -class Metadata
> > +class LIBCAMERA_TSA_CAPABILITY("mutex") Metadata
> > {
> > public:
> > Metadata() = default;
> > @@ -103,8 +105,8 @@ public:
> > * locks with the standard lock classes.
> > * e.g. std::lock_guard<RPiController::Metadata> lock(metadata)
> > */
> > - void lock() { mutex_.lock(); }
> > - void unlock() { mutex_.unlock(); }
> > + void lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }
> > + void unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }
> >
> > private:
> > mutable std::mutex mutex_;
> > --
> > 2.34.1
> >
More information about the libcamera-devel
mailing list