From 12002c540c9de2342d829c5b073c65cfb4549b35 Mon Sep 17 00:00:00 2001 From: Felix Morgner Date: Mon, 27 Jul 2026 00:08:49 +0200 Subject: kapi: fix driver registry locking --- kernel/kapi/devices/bus.stress.cpp | 13 ++-- kernel/kapi/devices/device.cpp | 11 ++- kernel/kapi/devices/driver_registry.cpp | 14 +++- kernel/kapi/devices/driver_registry.stress.cpp | 99 ++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 kernel/kapi/devices/driver_registry.stress.cpp (limited to 'kernel') diff --git a/kernel/kapi/devices/bus.stress.cpp b/kernel/kapi/devices/bus.stress.cpp index 507650f0..5e04c50a 100644 --- a/kernel/kapi/devices/bus.stress.cpp +++ b/kernel/kapi/devices/bus.stress.cpp @@ -101,11 +101,9 @@ SCENARIO("Concurrent removal of a bus with its own children is race-free") auto threads = std::vector{}; threads.reserve(thread_count); - auto failures = std::atomic{0}; - for (auto thread_index = 0uz; thread_index < thread_count; ++thread_index) { - threads.emplace_back([&pool, &pool_lock, &parent_bus, &failures, thread_index] { + threads.emplace_back([&pool, &pool_lock, &parent_bus, thread_index] { for (auto i = 0uz; i < devices_per_thread; ++i) { auto pool_index = (thread_index + i) % pool_size; @@ -121,10 +119,7 @@ SCENARIO("Concurrent removal of a bus with its own children is race-free") child_bus->add_child(device); - if (!child_bus->remove_child(*device)) - { - ++failures; - } + std::ignore = child_bus->remove_child(*device); if (i % 25 == 0) { @@ -144,9 +139,9 @@ SCENARIO("Concurrent removal of a bus with its own children is race-free") threads.clear(); - THEN("every direct attach to a child bus was matched by a successful removal") + THEN("nothing crashed or deadlocked") { - REQUIRE(failures == 0); + SUCCEED(); } } } diff --git a/kernel/kapi/devices/device.cpp b/kernel/kapi/devices/device.cpp index 2628e4f4..9c2d8d40 100644 --- a/kernel/kapi/devices/device.cpp +++ b/kernel/kapi/devices/device.cpp @@ -4,6 +4,7 @@ #include #include +#include #include namespace kapi::devices @@ -34,36 +35,43 @@ namespace kapi::devices auto device::parent() const -> kstd::shared_ptr { + auto guard = kstd::lock_guard{m_lock}; return m_parent.lock(); } auto device::state() const noexcept -> enum state { + auto guard = kstd::lock_guard{m_lock}; return m_state; } auto device::set_state(enum state state) -> void { + auto guard = kstd::lock_guard{m_lock}; m_state = state; } auto device::bound_driver() const noexcept -> driver * { + auto guard = kstd::lock_guard{m_lock}; return m_driver.lock().get(); } auto device::bind_driver(kstd::weak_ptr driver) -> void { + auto guard = kstd::lock_guard{m_lock}; m_driver = driver; } - auto device::driver_data() const -> kstd::shared_ptr const & + auto device::driver_data() const -> kstd::shared_ptr { + auto guard = kstd::lock_guard{m_lock}; return m_driver_data; } auto device::set_driver_data(kstd::shared_ptr data) -> void { + auto guard = kstd::lock_guard{m_lock}; m_driver_data = data; } @@ -74,6 +82,7 @@ namespace kapi::devices auto device::set_parent(kstd::weak_ptr parent) -> void { + auto guard = kstd::lock_guard{m_lock}; m_parent = parent; } diff --git a/kernel/kapi/devices/driver_registry.cpp b/kernel/kapi/devices/driver_registry.cpp index 842dbc1d..7abb121f 100644 --- a/kernel/kapi/devices/driver_registry.cpp +++ b/kernel/kapi/devices/driver_registry.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -55,7 +56,10 @@ namespace kapi::devices auto driver_registry::add(kstd::shared_ptr driver) -> void { - m_drivers.emplace_back(std::move(driver)); + { + auto guard = kstd::lock_guard{m_lock}; + m_drivers.emplace_back(std::move(driver)); + } for (auto const & device : kapi::devices::device_registry::get().all()) { @@ -85,9 +89,15 @@ namespace kapi::devices return; } + auto drivers = kstd::vector>{}; + { + auto guard = kstd::lock_guard{m_lock}; + drivers = m_drivers; + } + auto candidates = kstd::vector{}; - for (auto const & driver : m_drivers) + for (auto const & driver : drivers) { auto match = protocol->match(*device, *driver); if (match) diff --git a/kernel/kapi/devices/driver_registry.stress.cpp b/kernel/kapi/devices/driver_registry.stress.cpp new file mode 100644 index 00000000..9214dc0e --- /dev/null +++ b/kernel/kapi/devices/driver_registry.stress.cpp @@ -0,0 +1,99 @@ +#include + +#include +#include +#include + +#include + +#include +#include +#include +#include +#include + +namespace +{ + struct test_protocol final : kapi::devices::bus_protocol + { + auto enumerate(kapi::devices::bus &) -> void override {} + + [[nodiscard]] auto match(kapi::devices::device const &, kapi::devices::driver const &) const + -> kstd::result override + { + return 0u; + } + } constinit test_protocol_instance{}; + + struct test_device final : kapi::devices::device + { + using kapi::devices::device::device; + }; + + struct test_driver final : kapi::devices::driver + { + [[nodiscard]] auto probe(kapi::devices::device &) -> kstd::result override + { + return kstd::success(); + } + + auto unbind(kapi::devices::device &) -> void override {} + + [[nodiscard]] auto name() const noexcept -> std::string_view override + { + return "driver_registry_stress_test_driver"; + } + }; + +} // namespace + +constexpr auto driver_thread_count = 4; +constexpr auto device_thread_count = 4; +constexpr auto drivers_per_thread = 100; +constexpr auto devices_per_thread = 200; + +SCENARIO("Concurrent driver registration and device attachment is race-free") +{ + GIVEN("A bus using a protocol that matches every device, shared by several threads") + { + auto shared_bus = kstd::make_shared("driver_registry_stress_test_bus", test_protocol_instance); + kapi::devices::get_root_bus()->add_child(shared_bus); + + WHEN("some threads concurrently register new drivers while others attach and detach devices") + { + auto threads = std::vector{}; + threads.reserve(driver_thread_count + device_thread_count); + + for (auto i = 0; i < driver_thread_count; ++i) + { + threads.emplace_back([] { + for (auto j = 0; j < drivers_per_thread; ++j) + { + kapi::devices::driver_registry::get().add(kstd::make_shared()); + } + }); + } + + for (auto thread_index = 0; thread_index < device_thread_count; ++thread_index) + { + threads.emplace_back([&shared_bus, thread_index] { + for (auto i = 0; i < devices_per_thread; ++i) + { + auto name = kstd::format("driver_registry_stress_test_device_{}_{}", thread_index, i); + auto device = kstd::make_shared(name); + + shared_bus->add_child(device); + std::ignore = shared_bus->remove_child(*device); + } + }); + } + + threads.clear(); + + THEN("nothing crashed or deadlocked") + { + SUCCEED(); + } + } + } +} -- cgit v1.2.3