From patchwork Wed Apr 24 23:42:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19948 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 4DF80C328D for ; Wed, 24 Apr 2024 23:42:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 598D46340D; Thu, 25 Apr 2024 01:42:41 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hGiWKRL7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BC1F261A94 for ; Thu, 25 Apr 2024 01:42:33 +0200 (CEST) Received: from pendragon.ideasonboard.com (117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8F707B1 for ; Thu, 25 Apr 2024 01:41:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1714002101; bh=R5qiJqECiD7VBLdiDw+N7e4/3NwEWQ2P/sQZAkMqGwM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=hGiWKRL7Cq0Q58TNUnGxuE3kO2fp/OYUEX4OQhJr4I3yG3L48/gyK32+M+jmiZi1J YmLy35etQ4oVQ9cO9d7Ij06mqI1Z57N7vX/VtTaioQqBZXrpu6DXv5swAPBhaDN0Kn JTyuah74GLND8hNwlRQm/o1LUtwOAXQs5YfZ+K2E= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH v1 3/6] test: fence: Fix race condition Date: Thu, 25 Apr 2024 02:42:21 +0300 Message-ID: <20240424234224.9658-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.2 In-Reply-To: <20240424234224.9658-1-laurent.pinchart@ideasonboard.com> References: <20240424234224.9658-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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 fence test is racy, as it relies on the main loop being executed between completion of request signalledRequestId_ and signalledRequestId_ + 1. This usually happens, but is not guaranteed. To fix the race condition, change the request identification logic by replacing usage of the cookie value, is zero-based and wraps around at nbuffers_ - 1, with a completed request counter that is one-based and doesn't wrap. The completedRequestId_, expiredRequestId_ and signalledRequestId_ variables now track the identifier of the last request that has completed, the request whose fence will time out, and the request whose fence will be signalled. Signed-off-by: Laurent Pinchart --- test/fence.cpp | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/test/fence.cpp b/test/fence.cpp index e6f79d2faa21..7949bfbb176b 100644 --- a/test/fence.cpp +++ b/test/fence.cpp @@ -57,8 +57,7 @@ private: bool expectedCompletionResult_ = true; bool setFence_ = true; - unsigned int completedRequest_; - + unsigned int completedRequestId_; unsigned int signalledRequestId_; unsigned int expiredRequestId_; unsigned int nbuffers_; @@ -126,8 +125,9 @@ int FenceTest::init() return TestFail; } - signalledRequestId_ = nbuffers_ - 2; - expiredRequestId_ = nbuffers_ - 1; + completedRequestId_ = 0; + expiredRequestId_ = nbuffers_; + signalledRequestId_ = nbuffers_ * 2; return TestPass; } @@ -189,16 +189,16 @@ void FenceTest::requestRequeue(Request *request) const Request::BufferMap &buffers = request->buffers(); const Stream *stream = buffers.begin()->first; FrameBuffer *buffer = buffers.begin()->second; - uint64_t cookie = request->cookie(); request->reuse(); - if (cookie == signalledRequestId_ && setFence_) { + if (completedRequestId_ == signalledRequestId_ - nbuffers_ && setFence_) { /* - * The second time this request is queued add a fence to it. - * - * The main loop signals it by using a timer to write to the - * efd2_ file descriptor before the fence expires. + * This is the request that will be used to test fence + * signalling when it completes next time. Add a fence to it, + * using efd2_. The main loop will signal the fence by using a + * timer to write to the efd2_ file descriptor before the fence + * expires. */ std::unique_ptr fence = std::make_unique(std::move(eventFd2_)); @@ -213,8 +213,7 @@ void FenceTest::requestRequeue(Request *request) void FenceTest::requestComplete(Request *request) { - uint64_t cookie = request->cookie(); - completedRequest_ = cookie; + completedRequestId_++; /* * The last request is expected to fail as its fence has not been @@ -222,7 +221,7 @@ void FenceTest::requestComplete(Request *request) * * Validate the fence status but do not re-queue it. */ - if (cookie == expiredRequestId_) { + if (completedRequestId_ == expiredRequestId_) { if (validateExpiredRequest(request) != TestPass) expectedCompletionResult_ = false; @@ -271,7 +270,7 @@ int FenceTest::run() } int ret; - if (i == expiredRequestId_) { + if (i == expiredRequestId_ - 1) { /* This request will have a fence, and it will expire. */ std::unique_ptr fence = std::make_unique(std::move(eventFd_)); @@ -318,7 +317,7 @@ int FenceTest::run() Timer timer; timer.start(1000ms); while (timer.isRunning() && expectedCompletionResult_) { - if (completedRequest_ == signalledRequestId_ && setFence_) + if (completedRequestId_ == signalledRequestId_ - 1 && setFence_) /* * signalledRequestId_ has just completed and it has * been re-queued with a fence. Start the timer to