[v1,3/6] test: fence: Fix race condition
diff mbox series

Message ID 20240424234224.9658-4-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • vimc scaling improvements
Related show

Commit Message

Laurent Pinchart April 24, 2024, 11:42 p.m. UTC
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 <laurent.pinchart@ideasonboard.com>
---
 test/fence.cpp | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Patch
diff mbox series

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> fence =
 			std::make_unique<Fence>(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> fence =
 				std::make_unique<Fence>(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