From patchwork Thu Dec 23 02:33:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 15213 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 1D1EDC3258 for ; Thu, 23 Dec 2021 02:33:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0737560905; Thu, 23 Dec 2021 03:33:41 +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="e3ToY9V3"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A22FD605A8 for ; Thu, 23 Dec 2021 03:33:38 +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 4B03AB7E for ; Thu, 23 Dec 2021 03:33:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1640226818; bh=37oRnMtXLu06mgpjv2ANvG+8Qw2DHBzZ4MJw07lCcgs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=e3ToY9V3yqpJus9qXHbPKRV426PrFidFH+htJueauGIfBalBCQt2U5LaSihNxdiBd LOHe3hcxJct6pGzwMPZbKRiOIuZ7DArMQ6OKIcElACGpYAkhVb+rUVdssGAqlvlPZQ yd9WoYwqtn577KGlxuqJUv1k5iLSsVAhd+JoNeB4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 23 Dec 2021 04:33:30 +0200 Message-Id: <20211223023331.13505-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211223023331.13505-1-laurent.pinchart@ideasonboard.com> References: <20211223023331.13505-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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 --- 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..b2ee4ec0ce87 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 Thu Dec 23 02:33:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 15214 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 E2168C3259 for ; Thu, 23 Dec 2021 02:33:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8C9B46090B; Thu, 23 Dec 2021 03:33:42 +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="NY07XJxi"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 08D7D605A8 for ; Thu, 23 Dec 2021 03:33:39 +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 A5E112C5 for ; Thu, 23 Dec 2021 03:33:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1640226818; bh=Jmee5JD33DGh0kY0Nf5KZfvRNfrLPYEuIOYaPZU1bR8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=NY07XJxi2lmTY4AmE6iY1KrEDJ13dL+CMVmEGRAwHOSrVjiutTbIPPFnoA52fsuWH +Z5YAIDvGqVC/fTA9MQNRuR+0pMBU3ZlghdToidOqVXIA720RR7d5+93KnOaPpKJyT QH7Kxs/r21FrfEr/6OgH6hky+rs4EyDS96BxNe4Q= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 23 Dec 2021 04:33:31 +0200 Message-Id: <20211223023331.13505-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211223023331.13505-1-laurent.pinchart@ideasonboard.com> References: <20211223023331.13505-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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 --- include/libcamera/internal/pipeline_handler.h | 2 ++ src/libcamera/pipeline_handler.cpp | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index a7331cedfb16..3768bb8f6cb6 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,6 +89,7 @@ private: const char *name_; + Mutex lock_; bool lockOwner_; friend class PipelineHandlerFactory; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index b2ee4ec0ce87..07d9d387daff 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;