[libcamera-devel,v2,05/12] test: fence: Add test for the Fence class

14673 diff mbox series

Add a test for the Fence class by testing a Fence failure case, and
by testing a successfully signalled fence capture cycle.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 test/fence.cpp   | 339 +++++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build |   1 +
 2 files changed, 340 insertions(+)
 create mode 100644 test/fence.cpp

Hi Jacopo,

Thank you for the patch.

On Sat, Nov 20, 2021 at 12:13:06PM +0100, Jacopo Mondi wrote:
> Add a test for the Fence class by testing a Fence failure case, and
> by testing a successfully signalled fence capture cycle.

This makes me think that it would be useful to inject fence timeouts in
order to test applications. Beside unit tests (to test individual
objects) and lc-compliance (to test higher-level use cases and pipeline
handlers), a mode where libcamera would periodically return various
types of errors would help hardening applications.

This is certainly not a candidate for this patch series, but I'd like to
hear feedback on the idea.

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  test/fence.cpp   | 339 +++++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build |   1 +
>  2 files changed, 340 insertions(+)
>  create mode 100644 test/fence.cpp
> 
> diff --git a/test/fence.cpp b/test/fence.cpp
> new file mode 100644
> index 000000000000..7434542a89f8
> --- /dev/null
> +++ b/test/fence.cpp
> @@ -0,0 +1,339 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * fence.cpp - Fence test
> + */
> +
> +#include <iostream>
> +#include <memory>
> +#include <sys/eventfd.h>
> +#include <unistd.h>
> +
> +#include <libcamera/base/event_dispatcher.h>
> +#include <libcamera/base/thread.h>
> +#include <libcamera/base/timer.h>
> +#include <libcamera/base/utils.h>
> +
> +#include <libcamera/framebuffer_allocator.h>
> +
> +#include <libcamera/internal/fence.h>
> +
> +#include "camera_test.h"
> +#include "test.h"
> +
> +using namespace std::chrono_literals;
> +using namespace libcamera;
> +using namespace std;
> +
> +class FenceTest : public CameraTest, public Test
> +{
> +public:
> +	FenceTest();
> +
> +protected:
> +	int init();
> +	int run();
> +	void cleanup();
> +
> +private:
> +	int validateExpiredRequest(Request *request);
> +	void requestComplete(Request *request);
> +	void signalFence();
> +
> +	std::unique_ptr<Fence> fence_;
> +	EventDispatcher *dispatcher_;
> +	FileDescriptor eventFd_;
> +	Timer fenceTimer_;
> +
> +	std::vector<std::unique_ptr<Request>> requests_;
> +	std::unique_ptr<CameraConfiguration> config_;
> +	FrameBufferAllocator *allocator_;
> +
> +	Stream *stream_;
> +
> +	bool expectedCompletionResult_ = true;
> +	bool expectFenceFailure_ = true;
> +
> +	unsigned int completedRequest_;
> +
> +	unsigned int signalledRequestId_;
> +	unsigned int expiredRequestId_;
> +	unsigned int nbuffers_;
> +
> +	int efd2Copy_;
> +	int efdCopy_;

I'd drop the "Copy" suffix, those are not really copies. Maybe
efdFenceTimeout and efdFenceSignalled would be better names.

> +};
> +
> +FenceTest::FenceTest()
> +	: CameraTest("platform/vimc.0 Sensor B")
> +{
> +}
> +
> +int FenceTest::init()
> +{
> +	dispatcher_ = Thread::current()->eventDispatcher();
> +
> +	int efd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
> +	if (efd < 0) {
> +		cerr << "Unable to create eventfd" << endl;
> +		return TestFail;
> +	}

It likely makes no difference for now, but if we wanted to test the real
fence API, we could create a fence object using the
SW_SYNC_IOC_CREATE_FENCE ioctl. It needs to be called on the
sync/sw_sync debugfs file. The downside is that we could depend on
debugfs.

> +
> +	/*
> +	 * Keep a copy of the original fd value to validate that an expired
> +	 * fence has the same fd value.
> +	 */
> +	efdCopy_ = efd;
> +
> +	FileDescriptor eventFd(std::move(efd));
> +	fence_ = std::make_unique<Fence>(eventFd);
> +	if (!fence_->isValid()) {
> +		cerr << "Fence should be valid when created with a valid FileDescriptor" << endl;
> +		return TestFail;
> +	}
> +
> +	if (fence_->fd().fd() != efdCopy_) {
> +		cerr << "Fence creation should not duplicate file descriptor" << endl;
> +		return TestFail;
> +	}

These two tests should go to run().

> +
> +	config_ = camera_->generateConfiguration({ StreamRole::Viewfinder });
> +	if (!config_ || config_->size() != 1) {
> +		cerr << "Failed to generate default configuration" << endl;
> +		return TestFail;
> +	}
> +
> +	if (camera_->acquire()) {
> +		cerr << "Failed to acquire the camera" << endl;
> +		return TestFail;
> +	}
> +
> +	if (camera_->configure(config_.get())) {
> +		cerr << "Failed to set default configuration" << endl;
> +		return TestFail;
> +	}
> +
> +	StreamConfiguration &cfg = config_->at(0);
> +	stream_ = cfg.stream();
> +
> +	allocator_ = new FrameBufferAllocator(camera_);
> +	if (allocator_->allocate(stream_) < 0)
> +		return TestFail;
> +
> +	nbuffers_ = allocator_->buffers(stream_).size();

You may want to check that nbuffers_ is high enough.

> +	signalledRequestId_ = nbuffers_ - 2;
> +	expiredRequestId_ = nbuffers_ - 1;
> +
> +	return TestPass;
> +}
> +
> +int FenceTest::validateExpiredRequest(Request *request)
> +{
> +	FrameBuffer *buffer = request->buffers().begin()->second;
> +
> +	Fence *fence = buffer->fence();
> +	if (!fence) {
> +		cerr << "The expired fence should be present" << endl;
> +		return TestFail;
> +	}
> +
> +	if (!fence->isValid()) {
> +		cerr << "The expired fence should be valid" << endl;
> +		return TestFail;
> +	}
> +
> +	if (fence->fd().fd() != efdCopy_) {
> +		cerr << "The expired fence file descriptor should not change" << endl;
> +		return TestFail;
> +	}
> +
> +	fence->close();
> +
> +	expectFenceFailure_ = false;
> +
> +	return 0;
> +}
> +
> +/* Callback to signal a fence waiting on the eventfd file descriptor. */
> +void FenceTest::signalFence()
> +{
> +	uint64_t value = 1;
> +	write(efd2Copy_, &value, sizeof(value));
> +	dispatcher_->processEvents();
> +}
> +
> +void FenceTest::requestComplete(Request *request)
> +{
> +	uint64_t cookie = request->cookie();
> +	completedRequest_ = cookie;
> +
> +	/* We expect the last request to fail, if it doesn't happen, test fails. */
> +	if (expectFenceFailure_ && cookie == expiredRequestId_ &&
> +	    request->status() != Request::RequestCancelled) {
> +		cerr << "The last request should have failed: " << cookie << endl;
> +
> +		expectedCompletionResult_ = false;
> +		dispatcher_->interrupt();
> +		return;
> +	}
> +
> +	/* All other requests but the last one should be completed. */
> +	if (expectFenceFailure_ && cookie < expiredRequestId_ &&
> +	    request->status() != Request::RequestComplete) {
> +		cerr << "All requests but last should complete: " << cookie << endl;
> +
> +		expectedCompletionResult_ = false;
> +		dispatcher_->interrupt();
> +		return;
> +	}
> +
> +	/*
> +	 * If the last request has failed already, all the ones queued after
> +	 * that shall complete.
> +	 */
> +	if (!expectFenceFailure_ &&
> +	    request->status() != Request::RequestComplete) {
> +		cerr << "Unexpected request failure: " << cookie << endl;
> +
> +		expectedCompletionResult_ = false;
> +		dispatcher_->interrupt();
> +		return;
> +	}
> +
> +	/*
> +	 * The last request has failed.
> +	 * Validate its fence status and do not re-queue it.
> +	 */
> +	if (cookie == expiredRequestId_) {
> +		int ret = validateExpiredRequest(request);
> +		if (ret)
> +			expectedCompletionResult_ = false;
> +
> +		dispatcher_->interrupt();
> +		return;
> +	}
> +
> +	const Request::BufferMap &buffers = request->buffers();
> +	const Stream *stream = buffers.begin()->first;
> +	FrameBuffer *buffer = buffers.begin()->second;
> +
> +	/* A succesfully completed request should have the Fence closed. */
> +	if (buffer->fence()) {
> +		cerr << "Unexpected valid fence in completed request" << endl;
> +
> +		expectedCompletionResult_ = false;
> +		dispatcher_->interrupt();
> +		return;
> +	}
> +
> +	request->reuse();
> +
> +	if (cookie == signalledRequestId_) {
> +		/*
> +		 * The second time this request is about to be queued add
> +		 * a fence to it.
> +		 *
> +		 * The main loop should signal it by using a timer to write to
> +		 * the eventfd file descriptor before the fence expires.
> +		 */
> +		int efd2 = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
> +		if (efd2 < 0) {
> +			cerr << "Unable to create eventfd" << endl;
> +			expectedCompletionResult_ = false;
> +			return;
> +		}
> +		efd2Copy_ = efd2;

I wonder if this could be simplified by queuing a request with the fence
to be signalled in run().

> +
> +		FileDescriptor eventFd(std::move(efd2));
> +		std::unique_ptr<Fence> fence = std::make_unique<Fence>(eventFd);
> +		request->addBuffer(stream, buffer, std::move(fence));
> +	} else {
> +		/* All the other requests continue to operate without fences. */
> +		request->addBuffer(stream, buffer);
> +	}
> +
> +	camera_->queueRequest(request);
> +
> +	/*
> +	 * Interrupt the dispatcher to return control to the main loop and
> +	 * activate the fenceTimer
> +	 . */
> +	dispatcher_->interrupt();
> +}
> +
> +int FenceTest::run()
> +{
> +	for (const auto &[i, buffer] : utils::enumerate(allocator_->buffers(stream_))) {
> +		std::unique_ptr<Request> request = camera_->createRequest(i);
> +		if (!request) {
> +			cerr << "Failed to create request" << endl;
> +			return TestFail;
> +		}
> +
> +		int ret;
> +		if (i == expiredRequestId_)
> +			/* This request will have a fence, and it will expire. */
> +			ret = request->addBuffer(stream_, buffer.get(), std::move(fence_));
> +		else
> +			ret = request->addBuffer(stream_, buffer.get());
> +
> +		if (ret) {
> +			cerr << "Failed to associate buffer with request" << endl;
> +			return TestFail;
> +		}
> +
> +		requests_.push_back(std::move(request));
> +	}
> +
> +	camera_->requestCompleted.connect(this, &FenceTest::requestComplete);
> +
> +	if (camera_->start()) {
> +		cerr << "Failed to start camera" << endl;
> +		return TestFail;
> +	}
> +
> +	for (std::unique_ptr<Request> &request : requests_) {
> +		if (camera_->queueRequest(request.get())) {
> +			cerr << "Failed to queue request" << endl;
> +			return TestFail;
> +		}
> +	}
> +
> +	expectedCompletionResult_ = true;
> +
> +	/* This timer serves to signal fences associated with "signalledRequestId_" */
> +	Timer fenceTimer;
> +	fenceTimer.timeout.connect(this, &FenceTest::signalFence);
> +
> +	/* Loop for one second. */
> +	Timer timer;
> +	timer.start(1000);
> +	while (timer.isRunning() && expectedCompletionResult_) {
> +		if (completedRequest_ == signalledRequestId_)
> +			/*
> +			 * signalledRequestId_ has just completed and it has
> +			 * been re-queued with a fence. Start the timer to
> +			 * signal the fence in 10 msec.
> +			 */
> +			fenceTimer.start(10);
> +
> +		dispatcher_->processEvents();
> +	}
> +
> +	camera_->requestCompleted.disconnect();
> +
> +	if (camera_->stop()) {
> +		cerr << "Failed to stop camera" << endl;
> +		return TestFail;
> +	}
> +
> +	return expectedCompletionResult_ ? TestPass : TestFail;
> +}
> +
> +void FenceTest::cleanup()
> +{
> +	delete allocator_;
> +}
> +
> +TEST_REGISTER(FenceTest)
> diff --git a/test/meson.build b/test/meson.build
> index d0466f17d7b6..377e392628bf 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -40,6 +40,7 @@ internal_tests = [
>      ['event-dispatcher',                'event-dispatcher.cpp'],
>      ['event-thread',                    'event-thread.cpp'],
>      ['file',                            'file.cpp'],
> +    ['fence',                           'fence.cpp'],
>      ['file-descriptor',                 'file-descriptor.cpp'],
>      ['flags',                           'flags.cpp'],
>      ['hotplug-cameras',                 'hotplug-cameras.cpp'],