NekoCWD/libcamera
NekoCWD/libcamera
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 7 Wiki Activity Actions

test: fence: Fix race condition

Browse source 
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, which 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>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
This commit is contained in:
Laurent Pinchart 2024-04-25 00:35:04 +03:00
parent c44457957e
commit 13dcc7fc5f
1 changed files with 36 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

57
test/fence.cpp
View file

@ -57,8 +57,11 @@ private:
bool expectedCompletionResult_ = true; bool expectedCompletionResult_ = true;
bool setFence_ = true; bool setFence_ = true;
unsigned int completedRequest_; /*
* Request IDs track the number of requests that have completed. They
* are one-based, and don't wrap.
*/
unsigned int completedRequestId_;
unsigned int signalledRequestId_; unsigned int signalledRequestId_;
unsigned int expiredRequestId_; unsigned int expiredRequestId_;
unsigned int nbuffers_; unsigned int nbuffers_;
@ -126,8 +129,19 @@ int FenceTest::init()
return TestFail; return TestFail;
} }
signalledRequestId_ = nbuffers_ - 2; completedRequestId_ = 0;
expiredRequestId_ = nbuffers_ - 1;
/*
* All but two requests are queued without a fence. Request
* expiredRequestId_ will be queued with a fence that we won't signal
* (which is then expected to expire), and request signalledRequestId_
* will be queued with a fence that gets signalled. Select nbuffers_
* and nbuffers_ * 2 for those two requests, to space them by a few
* frames while still not requiring a long time for the test to
* complete.
*/
expiredRequestId_ = nbuffers_;
signalledRequestId_ = nbuffers_ * 2;
return TestPass; return TestPass;
} }
@ -189,16 +203,16 @@ void FenceTest::requestRequeue(Request *request)
const Request::BufferMap &buffers = request->buffers(); const Request::BufferMap &buffers = request->buffers();
const Stream *stream = buffers.begin()->first; const Stream *stream = buffers.begin()->first;
FrameBuffer *buffer = buffers.begin()->second; FrameBuffer *buffer = buffers.begin()->second;
uint64_t cookie = request->cookie();
request->reuse(); request->reuse();
if (cookie == signalledRequestId_ && setFence_) { if (completedRequestId_ == signalledRequestId_ - nbuffers_ && setFence_) {
/* /*
* The second time this request is queued add a fence to it. * This is the request that will be used to test fence
* * signalling when it completes next time. Add a fence to it,
* The main loop signals it by using a timer to write to the * using efd2_. The main loop will signal the fence by using a
* efd2_ file descriptor before the fence expires. * timer to write to the efd2_ file descriptor before the fence
* expires.
*/ */
std::unique_ptr<Fence> fence = std::unique_ptr<Fence> fence =
std::make_unique<Fence>(std::move(eventFd2_)); std::make_unique<Fence>(std::move(eventFd2_));
@ -213,16 +227,15 @@ void FenceTest::requestRequeue(Request *request)
void FenceTest::requestComplete(Request *request) void FenceTest::requestComplete(Request *request)
{ {
uint64_t cookie = request->cookie(); completedRequestId_++;
completedRequest_ = cookie;
/* /*
* The last request is expected to fail as its fence has not been * Request expiredRequestId_ is expected to fail as its fence has not
* signaled. * been signalled.
* *
* Validate the fence status but do not re-queue it. * Validate the fence status but do not re-queue it.
*/ */
if (cookie == expiredRequestId_) { if (completedRequestId_ == expiredRequestId_) {
if (validateExpiredRequest(request) != TestPass) if (validateExpiredRequest(request) != TestPass)
expectedCompletionResult_ = false; expectedCompletionResult_ = false;
@ -230,7 +243,7 @@ void FenceTest::requestComplete(Request *request)
return; return;
} }
/* Validate all requests but the last. */ /* Validate all other requests. */
if (validateRequest(request) != TestPass) { if (validateRequest(request) != TestPass) {
expectedCompletionResult_ = false; expectedCompletionResult_ = false;
@ -271,7 +284,7 @@ int FenceTest::run()
} }
int ret; int ret;
if (i == expiredRequestId_) { if (i == expiredRequestId_ - 1) {
/* This request will have a fence, and it will expire. */ /* This request will have a fence, and it will expire. */
std::unique_ptr<Fence> fence = std::unique_ptr<Fence> fence =
std::make_unique<Fence>(std::move(eventFd_)); std::make_unique<Fence>(std::move(eventFd_));
@ -318,11 +331,13 @@ int FenceTest::run()
Timer timer; Timer timer;
timer.start(1000ms); timer.start(1000ms);
while (timer.isRunning() && expectedCompletionResult_) { while (timer.isRunning() && expectedCompletionResult_) {
if (completedRequest_ == signalledRequestId_ && setFence_) if (completedRequestId_ == signalledRequestId_ - 1 && setFence_)
/* /*
* signalledRequestId_ has just completed and it has * The request just before signalledRequestId_ has just
* been re-queued with a fence. Start the timer to * completed. Request signalledRequestId_ has been
* signal the fence in 10 msec. * queued with a fence, and libcamera is likely already
* waiting on the fence, or will soon. Start the timer
* to signal the fence in 10 msec.
*/ */
fenceTimer.start(10ms); fenceTimer.start(10ms);