From patchwork Thu Jul 4 01:30:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1601 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A7CC60BEE for ; Thu, 4 Jul 2019 03:30:10 +0200 (CEST) X-Halon-ID: 31a845c4-9dfb-11e9-8601-0050569116f7 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [145.14.112.32]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 31a845c4-9dfb-11e9-8601-0050569116f7; Thu, 04 Jul 2019 03:29:43 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Thu, 4 Jul 2019 03:30:04 +0200 Message-Id: <20190704013004.14600-1-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.22.0 MIME-Version: 1.0 Subject: [libcamera-devel] [RFC] test: Move resource locking to test cases X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Jul 2019 01:30:10 -0000 Instead of blocking any test which require access to a kernel resource (vimc or vivid) from running in parallel move the resource locking into test library and allow meson to execute them in parallel. Resource locking is achieve using a lockfile for each resource in /tmp and lockf(). The call to lockf() blocks until it can acquire the resource and it keeps to lock until the test executable terminates closing the file descriptor to the lock file and freeing the lock. This design ensures the process never deadlocks. The reason a separate lock file is needed instead of locking the media device associated with the resource is that such a locking scheme is already used by libcamera itself. With this change I cut down the runtime of tests by 50%. The largest increase is from allowing self-contained tests in parallel with tests which require a kernel resource. This is because most of the tests who needs a kernel resource needs vimc so there is heavy contention for that single resource and little gain from meson to execute them in parallel. Signed-off-by: Niklas Söderlund --- test/camera/camera_test.h | 2 +- test/camera/meson.build | 2 +- test/controls/control_list.cpp | 4 ++ test/controls/meson.build | 2 +- test/libtest/test.cpp | 37 ++++++++++++++++++- test/libtest/test.h | 15 +++++++- test/media_device/media_device_test.h | 2 +- test/media_device/meson.build | 2 +- test/v4l2_subdevice/meson.build | 2 +- test/v4l2_subdevice/v4l2_subdevice_test.h | 2 +- test/v4l2_videodevice/buffer_sharing.cpp | 2 +- test/v4l2_videodevice/capture_async.cpp | 2 +- test/v4l2_videodevice/double_open.cpp | 2 +- test/v4l2_videodevice/formats.cpp | 2 +- test/v4l2_videodevice/meson.build | 2 +- test/v4l2_videodevice/request_buffers.cpp | 2 +- test/v4l2_videodevice/stream_on_off.cpp | 2 +- .../v4l2_videodevice_test.cpp | 16 +++++++- test/v4l2_videodevice/v4l2_videodevice_test.h | 4 +- 19 files changed, 85 insertions(+), 19 deletions(-) diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h index ffc8a485bfaff836..0f3c15cc3dd34f3c 100644 --- a/test/camera/camera_test.h +++ b/test/camera/camera_test.h @@ -17,7 +17,7 @@ class CameraTest : public Test { public: CameraTest() - : cm_(nullptr) {} + : Test(VIMC), cm_(nullptr) {} protected: int init(); diff --git a/test/camera/meson.build b/test/camera/meson.build index 35e97ce5de1a72ca..b10b1e76aaec0233 100644 --- a/test/camera/meson.build +++ b/test/camera/meson.build @@ -12,5 +12,5 @@ foreach t : camera_tests dependencies : libcamera_dep, link_with : test_libraries, include_directories : test_includes_internal) - test(t[0], exe, suite : 'camera', is_parallel : false) + test(t[0], exe, suite : 'camera') endforeach diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index c834edc352f5d7d4..0fc3ded2545df2e1 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -18,6 +18,10 @@ using namespace libcamera; class ControlListTest : public Test { +public: + ControlListTest() + : Test(VIMC) {} + protected: int init() { diff --git a/test/controls/meson.build b/test/controls/meson.build index f4fc7b947dd9e2a6..724871e3136f369b 100644 --- a/test/controls/meson.build +++ b/test/controls/meson.build @@ -9,5 +9,5 @@ foreach t : control_tests dependencies : libcamera_dep, link_with : test_libraries, include_directories : test_includes_internal) - test(t[0], exe, suite : 'controls', is_parallel : false) + test(t[0], exe, suite : 'controls') endforeach diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp index b119cf19273900d2..cf13fa42dd9fdc5a 100644 --- a/test/libtest/test.cpp +++ b/test/libtest/test.cpp @@ -5,11 +5,15 @@ * test.cpp - libcamera test base class */ +#include +#include #include +#include #include "test.h" -Test::Test() +Test::Test(TestResource resource) + : resource_(resource) { } @@ -25,6 +29,9 @@ int Test::execute() if (ret) return errno; + if (!resourceLock()) + return TestFail; + ret = init(); if (ret) return ret; @@ -35,3 +42,31 @@ int Test::execute() return ret; } + +bool Test::resourceLock() +{ + const char *path; + + switch (resource_) { + case None: + return true; + case VIMC: + path = "/tmp/libcamera-test-vimc"; + break; + case VIVID: + path = "/tmp/libcamera-test-vivid"; + break; + } + + int fd = ::open(path, O_CREAT | O_RDWR, 0666); + if (fd < 0) { + std::cerr << "Failed to open resource lockfile" << std::endl; + perror("FOO"); + return false; + } + + if (lockf(fd, F_LOCK, 0)) + return false; + + return true; +} diff --git a/test/libtest/test.h b/test/libtest/test.h index ec08bf97c03d563e..259fb589eb621489 100644 --- a/test/libtest/test.h +++ b/test/libtest/test.h @@ -15,10 +15,16 @@ enum TestStatus { TestSkip = 77, }; +enum TestResource { + None, + VIMC, + VIVID, +}; + class Test { public: - Test(); + Test(TestResource resource = None); virtual ~Test(); int execute(); @@ -27,6 +33,13 @@ protected: virtual int init() { return 0; } virtual int run() = 0; virtual void cleanup() { } + + TestResource resource() { return resource_; } + +private: + bool resourceLock(); + + TestResource resource_; }; #define TEST_REGISTER(klass) \ diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h index cdbd14841d5ccca2..9e34ba86c397a99c 100644 --- a/test/media_device/media_device_test.h +++ b/test/media_device/media_device_test.h @@ -20,7 +20,7 @@ class MediaDeviceTest : public Test { public: MediaDeviceTest() - : media_(nullptr), enumerator_(nullptr) {} + : Test(VIMC), media_(nullptr), enumerator_(nullptr) {} protected: int init(); diff --git a/test/media_device/meson.build b/test/media_device/meson.build index 6a0e468434b58efe..6a5b24d27aa0f618 100644 --- a/test/media_device/meson.build +++ b/test/media_device/meson.build @@ -18,5 +18,5 @@ foreach t : media_device_tests link_with : [test_libraries, lib_mdev_test], include_directories : test_includes_internal) - test(t[0], exe, suite : 'media_device', is_parallel : false) + test(t[0], exe, suite : 'media_device') endforeach diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build index 0521984b2a787ab9..47676adbe5a33f7e 100644 --- a/test/v4l2_subdevice/meson.build +++ b/test/v4l2_subdevice/meson.build @@ -8,5 +8,5 @@ foreach t : v4l2_subdevice_tests dependencies : libcamera_dep, link_with : test_libraries, include_directories : test_includes_internal) - test(t[0], exe, suite : 'v4l2_subdevice', is_parallel : false) + test(t[0], exe, suite : 'v4l2_subdevice') endforeach diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.h b/test/v4l2_subdevice/v4l2_subdevice_test.h index 96646a155536aadf..a9002834bf8959b2 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.h +++ b/test/v4l2_subdevice/v4l2_subdevice_test.h @@ -21,7 +21,7 @@ class V4L2SubdeviceTest : public Test { public: V4L2SubdeviceTest() - : scaler_(nullptr){}; + : Test(VIMC), scaler_(nullptr){}; protected: int init() override; diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 1bc478fe8f8e1163..d619713257055fd5 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -23,7 +23,7 @@ class BufferSharingTest : public V4L2VideoDeviceTest { public: BufferSharingTest() - : V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap"), + : V4L2VideoDeviceTest(VIVID, "vivid-000-vid-cap"), output_(nullptr), framesCaptured_(0), framesOutput_(0) {} protected: diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index cea4fffbf7a5603d..9ae8043b31a68f0e 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -18,7 +18,7 @@ class CaptureAsyncTest : public V4L2VideoDeviceTest { public: CaptureAsyncTest() - : V4L2VideoDeviceTest("vimc", "Raw Capture 0"), frames(0) {} + : V4L2VideoDeviceTest(VIMC, "Raw Capture 0"), frames(0) {} void receiveBuffer(Buffer *buffer) { diff --git a/test/v4l2_videodevice/double_open.cpp b/test/v4l2_videodevice/double_open.cpp index 5768d4043d0ba03f..ff1c7b136e803929 100644 --- a/test/v4l2_videodevice/double_open.cpp +++ b/test/v4l2_videodevice/double_open.cpp @@ -15,7 +15,7 @@ class DoubleOpen : public V4L2VideoDeviceTest { public: DoubleOpen() - : V4L2VideoDeviceTest("vimc", "Raw Capture 0") {} + : V4L2VideoDeviceTest(VIMC, "Raw Capture 0") {} protected: int run() { diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp index ee7d357de0752cae..25cd147f804ba0d5 100644 --- a/test/v4l2_videodevice/formats.cpp +++ b/test/v4l2_videodevice/formats.cpp @@ -19,7 +19,7 @@ class Format : public V4L2VideoDeviceTest { public: Format() - : V4L2VideoDeviceTest("vimc", "Raw Capture 0") {} + : V4L2VideoDeviceTest(VIMC, "Raw Capture 0") {} protected: int run() { diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build index 76be5e142bb63aa6..dc8204872efdc038 100644 --- a/test/v4l2_videodevice/meson.build +++ b/test/v4l2_videodevice/meson.build @@ -14,5 +14,5 @@ foreach t : v4l2_videodevice_tests dependencies : libcamera_dep, link_with : test_libraries, include_directories : test_includes_internal) - test(t[0], exe, suite : 'v4l2_videodevice', is_parallel : false) + test(t[0], exe, suite : 'v4l2_videodevice') endforeach diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp index c4aedf7b3cd61e80..bbc4073c363c128a 100644 --- a/test/v4l2_videodevice/request_buffers.cpp +++ b/test/v4l2_videodevice/request_buffers.cpp @@ -11,7 +11,7 @@ class RequestBuffersTest : public V4L2VideoDeviceTest { public: RequestBuffersTest() - : V4L2VideoDeviceTest("vimc", "Raw Capture 0") {} + : V4L2VideoDeviceTest(VIMC, "Raw Capture 0") {} protected: int run() diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp index 7664adc4c1f07046..fe021667c44559fa 100644 --- a/test/v4l2_videodevice/stream_on_off.cpp +++ b/test/v4l2_videodevice/stream_on_off.cpp @@ -11,7 +11,7 @@ class StreamOnStreamOffTest : public V4L2VideoDeviceTest { public: StreamOnStreamOffTest() - : V4L2VideoDeviceTest("vimc", "Raw Capture 0") {} + : V4L2VideoDeviceTest(VIMC, "Raw Capture 0") {} protected: int run() { diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp index b26d06ad45197c8f..aa39e455360c868c 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp @@ -28,6 +28,20 @@ bool exists(const std::string &path) int V4L2VideoDeviceTest::init() { + const char *driver; + + switch (resource()) { + case VIMC: + driver = "vimc"; + break; + case VIVID: + driver = "vivid"; + break; + default: + std::cerr << "Unkown driver for resource" << std::endl; + return TestFail; + } + enumerator_ = DeviceEnumerator::create(); if (!enumerator_) { cerr << "Failed to create device enumerator" << endl; @@ -39,7 +53,7 @@ int V4L2VideoDeviceTest::init() return TestFail; } - DeviceMatch dm(driver_); + DeviceMatch dm(driver); dm.add(entity_); media_ = enumerator_->search(dm); diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h index 3321b5a4f98fdb1d..b90a5fca6ce48fdd 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.h +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h @@ -22,8 +22,8 @@ using namespace libcamera; class V4L2VideoDeviceTest : public Test { public: - V4L2VideoDeviceTest(const char *driver, const char *entity) - : driver_(driver), entity_(entity), capture_(nullptr) + V4L2VideoDeviceTest(TestResource resource, const char *entity) + : Test(resource), entity_(entity), capture_(nullptr) { }