From patchwork Fri Dec 3 19:55:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 15029 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A2C7CBDB13 for ; Fri, 3 Dec 2021 19:56:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 59FB26084A; Fri, 3 Dec 2021 20:56:00 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="ocLvvMZA"; dkim-atps=neutral Received: from mail-pg1-x532.google.com (mail-pg1-x532.google.com [IPv6:2607:f8b0:4864:20::532]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E1B4607DE for ; Fri, 3 Dec 2021 20:55:58 +0100 (CET) Received: by mail-pg1-x532.google.com with SMTP id m15so4080771pgu.11 for ; Fri, 03 Dec 2021 11:55:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ghAeGg8cM3/5PK0Jss5JhV2ixIi408VmPvgJ1SVYROA=; b=ocLvvMZAgXXjFD1GhR4asgIUMyulS82ldyptdahxuh/MwM7BjQRZVAEW7VDd7k3Iqi dMdlEtg9VBxY2MaTeJBZD4JzlpgmfOHJH56PrVQNZ4rvt7hQOTIc4IQxdm8oEL8WPxXK oW0B+xuo894QkcR/igOll9wdT/jrRYx9BKkgU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ghAeGg8cM3/5PK0Jss5JhV2ixIi408VmPvgJ1SVYROA=; b=jxFTBs4r3eOYyof7Ux2+3a1ZPjy9uft5bt6owna3/IpkwzP5QnmK47liTYY156CcB+ c+UwxrVpVz7ySJbn3dthtV1tkobePFVVFbalMcJumbdEOJcYdFmE3YjJmkFhB0u4DpZl F+9Lfpvq830iHmVWkXuUqGeozW9kMCV6n5RDc5CJjTbWFBFyKJqiQYLMljFdVPepwYry 87nopFUVxZdzM97n+XG0LyPR/LFjSrtdXARMxjstXUyHfQlmDJzVU5D9fNPGJ1XX/066 6F3YevEr2PB0WiK25QLJAV1eEnlbfMI+eEylEVMCAtFFLtzqzTmu6DZIe2KwyE4wAXhI wquA== X-Gm-Message-State: AOAM5320HL2yq62/qlHJaYvcApMeJyLv8KnwV3FmoFua/qVjA1ZKaudm cwUEw4CO3ZK/1w7thdYloxyU52wfTgnThA== X-Google-Smtp-Source: ABdhPJzifGFsJsoUIrT+UnWZdLpjhc/QUae0zzSQ0758kooSF041NWtVEOA3GJ0vBy9KL2RLK0bMYg== X-Received: by 2002:a63:1b02:: with SMTP id b2mr5961242pgb.263.1638561356863; Fri, 03 Dec 2021 11:55:56 -0800 (PST) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:203:cd78:3a5f:d792:6177]) by smtp.gmail.com with ESMTPSA id t3sm4265891pfj.207.2021.12.03.11.55.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Dec 2021 11:55:56 -0800 (PST) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Sat, 4 Dec 2021 04:55:39 +0900 Message-Id: <20211203195539.1574621-8-hiroh@chromium.org> X-Mailer: git-send-email 2.34.1.400.ga245620fadb-goog In-Reply-To: <20211203195539.1574621-1-hiroh@chromium.org> References: <20211203195539.1574621-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 7/7] ipa: raspberrypi: metadata: Apply clang thread safety annotation X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This annotates member variable and functions of Metadata by clang thread safety annotations. Metadata has lock and unlock functions so that it can be used by std::unique_lock. I replace them by a function of returning Mutex reference. It eases to annotate clang thread safety annotations. Signed-off-by: Hirokazu Honda --- src/ipa/raspberrypi/controller/metadata.hpp | 22 ++++++++++++--------- src/ipa/raspberrypi/controller/rpi/agc.cpp | 3 ++- src/ipa/raspberrypi/controller/rpi/ccm.cpp | 3 ++- src/ipa/raspberrypi/raspberrypi.cpp | 4 ++-- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/ipa/raspberrypi/controller/metadata.hpp b/src/ipa/raspberrypi/controller/metadata.hpp index ff2e1ae3..65dad8fb 100644 --- a/src/ipa/raspberrypi/controller/metadata.hpp +++ b/src/ipa/raspberrypi/controller/metadata.hpp @@ -36,6 +36,7 @@ public: template void Set(std::string const &tag, T const &value) + LIBCAMERA_TSA_EXCLUDES(mutex_) { libcamera::MutexLocker lock(mutex_); data_[tag] = value; @@ -43,6 +44,7 @@ public: template int Get(std::string const &tag, T &value) const + LIBCAMERA_TSA_EXCLUDES(mutex_) { libcamera::MutexLocker lock(mutex_); auto it = data_.find(tag); @@ -52,13 +54,14 @@ public: return 0; } - void Clear() + void Clear() LIBCAMERA_TSA_EXCLUDES(mutex_) { libcamera::MutexLocker lock(mutex_); data_.clear(); } Metadata &operator=(Metadata const &other) + LIBCAMERA_TSA_EXCLUDES(mutex_, other.mutex_) { libcamera::MutexLocker lock(mutex_); libcamera::MutexLocker other_lock(other.mutex_); @@ -67,6 +70,7 @@ public: } Metadata &operator=(Metadata &&other) + LIBCAMERA_TSA_EXCLUDES(mutex_, other.mutex_) { libcamera::MutexLocker lock(mutex_); libcamera::MutexLocker other_lock(other.mutex_); @@ -75,7 +79,7 @@ public: return *this; } - void Merge(Metadata &other) + void Merge(Metadata &other) LIBCAMERA_TSA_EXCLUDES(mutex_, other.mutex_) { libcamera::MutexLocker lock(mutex_); libcamera::MutexLocker other_lock(other.mutex_); @@ -83,7 +87,7 @@ public: } template - T *GetLocked(std::string const &tag) + T *GetLocked(std::string const &tag) LIBCAMERA_TSA_REQUIRES(mutex_) { // This allows in-place access to the Metadata contents, // for which you should be holding the lock. @@ -95,20 +99,20 @@ public: template void SetLocked(std::string const &tag, T const &value) + LIBCAMERA_TSA_REQUIRES(mutex_) { // Use this only if you're holding the lock yourself. data_[tag] = value; } - // Note: use of (lowercase) lock and unlock means you can create scoped - // locks with the standard lock classes. - // e.g. std::lock_guard lock(metadata) - void lock() LIBCAMERA_TSA_ACQUIRE(mutex_) { mutex_.lock(); } - void unlock() LIBCAMERA_TSA_RELEASE(mutex_) { mutex_.unlock(); } + libcamera::Mutex &mutex() LIBCAMERA_TSA_RETURN_CAPABILITY(mutex_) + { + return mutex_; + } private: mutable libcamera::Mutex mutex_; - std::map data_; + std::map data_ LIBCAMERA_TSA_GUARDED_BY(mutex_); }; } // namespace RPiController diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index f6a9cb0a..870909af 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -485,7 +485,8 @@ void Agc::housekeepConfig() void Agc::fetchCurrentExposure(Metadata *image_metadata) { - std::unique_lock lock(*image_metadata); + MutexLocker locker(image_metadata->mutex()); + DeviceStatus *device_status = image_metadata->GetLocked("device.status"); if (!device_status) diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp b/src/ipa/raspberrypi/controller/rpi/ccm.cpp index 821a4c7c..989b15dd 100644 --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp @@ -85,6 +85,7 @@ void Ccm::Initialise() {} template static bool get_locked(Metadata *metadata, std::string const &tag, T &value) + LIBCAMERA_TSA_REQUIRES(metadata->mutex()) { T *ptr = metadata->GetLocked(tag); if (ptr == nullptr) @@ -128,7 +129,7 @@ void Ccm::Prepare(Metadata *image_metadata) lux.lux = 400; // in case no metadata { // grab mutex just once to get everything - std::lock_guard lock(*image_metadata); + MutexLocker locker(image_metadata->mutex()); awb_ok = get_locked(image_metadata, "awb.status", awb); lux_ok = get_locked(image_metadata, "lux.status", lux); } diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index fed82e22..b8d22dec 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -465,7 +465,7 @@ void IPARPi::signalIspPrepare(const ipa::RPi::ISPConfig &data) void IPARPi::reportMetadata() { - std::unique_lock lock(rpiMetadata_); + MutexLocker locker(rpiMetadata_.mutex()); /* * Certain information about the current frame and how it will be @@ -977,7 +977,7 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data) controller_.Prepare(&rpiMetadata_); /* Lock the metadata buffer to avoid constant locks/unlocks. */ - std::unique_lock lock(rpiMetadata_); + MutexLocker locker(rpiMetadata_.mutex()); AwbStatus *awbStatus = rpiMetadata_.GetLocked("awb.status"); if (awbStatus)