From patchwork Sun Dec 26 23:12:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 15220 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 2C917C3258 for ; Sun, 26 Dec 2021 23:13:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 41E7F60912; Mon, 27 Dec 2021 00:13:08 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DSpz6UOU"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DB8BD608E6 for ; Mon, 27 Dec 2021 00:13:05 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 81607A15 for ; Mon, 27 Dec 2021 00:13:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1640560385; bh=2fBi/E7xfJUsa6NW8y2N4x7gxRYz3bcVIvyPZkjOPb8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=DSpz6UOU7DUvX6YmNK3QEwe1PYSUnwdCWHAMUg2rw+sMO78wsP/LhKNc0jWIOee5B hjqzUsumQ9NbXt5Ahy6mr9HUIAox/ebuBxChr2C88qyPMU7A4h+Vmj2JaDHVC4YRvJ dIPwNKx3vmk0LrPmMPrrsbxJe/DUfgg43AAUxn1A= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 27 Dec 2021 01:12:54 +0200 Message-Id: <20211226231255.18653-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211226231255.18653-1-laurent.pinchart@ideasonboard.com> References: <20211226231255.18653-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/2] libcamera: media_device: Move recursive lock handling to pipeline handler 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" The MediaDevice lock is meant to prevent concurrent usage of multiple cameras from the same pipeline handlers. As media devices are acquired by pipeline handlers, we can't have multiple pipeline handlers trying to lock the same media device. The recursive locking detection can thus be moved to the pipeline handler. This simplifies the media device implementation that now implements true lock semantics, and prepares for support of concurrent camera usage. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham Reviewed-by: Umang Jain --- Changes since v1: - Fix lock owner check un PipelineHandler::unlock() --- include/libcamera/internal/media_device.h | 1 - include/libcamera/internal/pipeline_handler.h | 2 ++ src/libcamera/media_device.cpp | 14 +------------- src/libcamera/pipeline_handler.cpp | 13 ++++++++++++- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h index af0b25b731eb..6e2a63f38229 100644 --- a/include/libcamera/internal/media_device.h +++ b/include/libcamera/internal/media_device.h @@ -86,7 +86,6 @@ private: UniqueFD fd_; bool valid_; bool acquired_; - bool lockOwner_; std::map objects_; std::vector entities_; diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index ec986a518b5c..a7331cedfb16 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -88,6 +88,8 @@ private: const char *name_; + bool lockOwner_; + friend class PipelineHandlerFactory; }; diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 0b7940182d0c..941f86c25f66 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -63,8 +63,7 @@ LOG_DEFINE_CATEGORY(MediaDevice) * populate() before the media graph can be queried. */ MediaDevice::MediaDevice(const std::string &deviceNode) - : deviceNode_(deviceNode), valid_(false), acquired_(false), - lockOwner_(false) + : deviceNode_(deviceNode), valid_(false), acquired_(false) { } @@ -145,15 +144,9 @@ bool MediaDevice::lock() if (!fd_.isValid()) return false; - /* Do not allow nested locking in the same libcamera instance. */ - if (lockOwner_) - return false; - if (lockf(fd_.get(), F_TLOCK, 0)) return false; - lockOwner_ = true; - return true; } @@ -171,11 +164,6 @@ void MediaDevice::unlock() if (!fd_.isValid()) return; - if (!lockOwner_) - return; - - lockOwner_ = false; - lockf(fd_.get(), F_ULOCK, 0); } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 0bc0e341b9e7..a9241ac62979 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -67,7 +67,7 @@ LOG_DEFINE_CATEGORY(Pipeline) * respective factories. */ PipelineHandler::PipelineHandler(CameraManager *manager) - : manager_(manager) + : manager_(manager), lockOwner_(false) { } @@ -155,6 +155,10 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, */ bool PipelineHandler::lock() { + /* Do not allow nested locking in the same libcamera instance. */ + if (lockOwner_) + return false; + for (std::shared_ptr &media : mediaDevices_) { if (!media->lock()) { unlock(); @@ -162,6 +166,8 @@ bool PipelineHandler::lock() } } + lockOwner_ = true; + return true; } @@ -177,8 +183,13 @@ bool PipelineHandler::lock() */ void PipelineHandler::unlock() { + if (!lockOwner_) + return; + for (std::shared_ptr &media : mediaDevices_) media->unlock(); + + lockOwner_ = false; } /** From patchwork Sun Dec 26 23:12:55 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 15221 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 45829BF415 for ; Sun, 26 Dec 2021 23:13:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E81D360918; Mon, 27 Dec 2021 00:13:09 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Xc+kGqp5"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E981608E6 for ; Mon, 27 Dec 2021 00:13:06 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DA3C5148B for ; Mon, 27 Dec 2021 00:13:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1640560386; bh=t3SMdk0VGcMS6YGie3R8h/BleNltHY+0ZzOxkUkRncs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Xc+kGqp59cMmCktum+H7FwOofiUpXyPM1b+UNlRbx73wuEYhQqlA5isV6Ml04ML/y RfFdCwo6tLT3rUYN+47JRy/jUEfqXV943CSMfy96BUiSF76ddHM1QHUGOvvWS8Ha6o 6qX9rFnBqTA+sYXdnVmoMhT1Uklu99jhiabOEXqY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 27 Dec 2021 01:12:55 +0200 Message-Id: <20211226231255.18653-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211226231255.18653-1-laurent.pinchart@ideasonboard.com> References: <20211226231255.18653-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/2] libcamera: pipeline_handler: Make lock() and unlock() thread-safe 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" The PipelineHandler lock() and unlock() functions are documented as thread-safe, but they're not. Fix them using a mutex. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham Reviewed-by: Jacopo Mondi Reviewed-by: Umang Jain --- Changes since v1: - Add thread safety annotation - Comment that lockOwner_ does not indicate ownership of lock_ --- include/libcamera/internal/pipeline_handler.h | 4 +++- src/libcamera/pipeline_handler.cpp | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index a7331cedfb16..e5b8ffb4db3d 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -88,7 +89,8 @@ private: const char *name_; - bool lockOwner_; + Mutex lock_; + bool lockOwner_ LIBCAMERA_TSA_GUARDED_BY(lock_); /* *Not* ownership of lock_ */ friend class PipelineHandlerFactory; }; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index a9241ac62979..03e4b9e66aa6 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -155,6 +156,8 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, */ bool PipelineHandler::lock() { + MutexLocker locker(lock_); + /* Do not allow nested locking in the same libcamera instance. */ if (lockOwner_) return false; @@ -183,6 +186,8 @@ bool PipelineHandler::lock() */ void PipelineHandler::unlock() { + MutexLocker locker(lock_); + if (!lockOwner_) return;