diff options
Diffstat (limited to 'kernel/kapi/devices')
| -rw-r--r-- | kernel/kapi/devices/bus.cpp | 38 | ||||
| -rw-r--r-- | kernel/kapi/devices/bus.stress.cpp | 76 | ||||
| -rw-r--r-- | kernel/kapi/devices/device_registry.cpp | 96 | ||||
| -rw-r--r-- | kernel/kapi/devices/facet_registry.cpp | 130 |
4 files changed, 233 insertions, 107 deletions
diff --git a/kernel/kapi/devices/bus.cpp b/kernel/kapi/devices/bus.cpp index d917eb96..71dd01e6 100644 --- a/kernel/kapi/devices/bus.cpp +++ b/kernel/kapi/devices/bus.cpp @@ -6,6 +6,7 @@ #include <kapi/system.hpp> #include <kstd/memory.hpp> +#include <kstd/mutex.hpp> #include <kstd/string.hpp> #include <kstd/vector.hpp> @@ -42,30 +43,32 @@ namespace kapi::devices kapi::system::panic("[OS:DEV] Failed to register child device {}", child->name()); } - // TODO: lock bus + auto attached = kstd::shared_ptr<device>{}; + { + auto guard = kstd::lock_guard{m_lock}; + attached = m_devices.emplace_back(std::move(child)); + } - auto & attached = m_devices.emplace_back(std::move(child)); attached->set_state(state::present); - // TODO: unlock bus - driver_registry::get().device_attached(attached); } auto bus::remove_child(device & device) -> bool // NOLINT(misc-no-recursion) { - // TODO: lock bus - - auto found = std::ranges::find_if(m_devices, [&](auto const & d) { return d.get() == &device; }); - if (!found) + auto lifeline = kstd::shared_ptr<struct device>{}; { - return false; - } + auto guard = kstd::lock_guard{m_lock}; - auto lifeline = *found; - m_devices.erase(found); + auto found = std::ranges::find_if(m_devices, [&](auto const & d) { return d.get() == &device; }); + if (found == m_devices.end()) + { + return false; + } - // TODO: unlock bus + lifeline = *found; + m_devices.erase(found); + } do_remove_child(*lifeline); return true; @@ -100,10 +103,11 @@ namespace kapi::devices if (auto child_bus = device.facet<bus>()) { auto grandchildren = kstd::vector<kstd::shared_ptr<kapi::devices::device>>{}; - // TODO: lock child bus - // NOTE: this could be more efficient one vector::assign is implemented. - grandchildren = kstd::vector(child_bus->m_devices.begin(), child_bus->m_devices.end()); - // TODO: unlock child bus + { + auto guard = kstd::lock_guard{m_lock}; + // NOTE: this could be more efficient one vector::assign is implemented. + grandchildren = kstd::vector(child_bus->m_devices.begin(), child_bus->m_devices.end()); + } for (auto & grandchild : grandchildren) { diff --git a/kernel/kapi/devices/bus.stress.cpp b/kernel/kapi/devices/bus.stress.cpp new file mode 100644 index 00000000..226b0296 --- /dev/null +++ b/kernel/kapi/devices/bus.stress.cpp @@ -0,0 +1,76 @@ +#include <kapi/devices.hpp> + +#include <kstd/format.hpp> +#include <kstd/memory.hpp> + +#include <catch2/catch_test_macros.hpp> + +#include <atomic> +#include <cstddef> +#include <thread> +#include <tuple> +#include <vector> + +namespace +{ + + struct test_device final : kapi::devices::device + { + using kapi::devices::device::device; + }; + +} // namespace + +constexpr auto thread_count = 32; +constexpr auto devices_per_thread = 200; + +SCENARIO("Concurrent attach/detach/lookup on a bus is race-free") +{ + GIVEN("A bus shared by several threads") + { + auto shared_bus = kstd::make_shared<kapi::devices::bus>("stress_test_bus"); + kapi::devices::get_root_bus()->add_child(shared_bus); + + WHEN("each thread repeatedly attaches, detaches, and looks up devices concurrently with the others") + { + auto threads = std::vector<std::jthread>{}; + threads.reserve(thread_count); + + auto failure_count = std::atomic<std::size_t>{0}; + + for (auto thread_index = 0uz; thread_index < thread_count; ++thread_index) + { + threads.emplace_back([&shared_bus, &failure_count, thread_index] { + for (auto i = 0uz; i < devices_per_thread; ++i) + { + auto name = kstd::format("stress_test_device_{}_{}", thread_index, i); + auto device = kstd::make_shared<test_device>(name); + + shared_bus->add_child(device); + + std::ignore = kapi::devices::device_registry::get().find(name); + std::ignore = kapi::devices::device_registry::get().all(); + std::ignore = shared_bus->children().size(); + + if (!shared_bus->remove_child(*device)) + { + ++failure_count; + }; + } + }); + } + + threads.clear(); + + THEN("every attach was matched by a successful removal") + { + REQUIRE(failure_count == 0); + } + + THEN("the bus has no children") + { + REQUIRE(shared_bus->children().empty()); + } + } + } +}
\ No newline at end of file diff --git a/kernel/kapi/devices/device_registry.cpp b/kernel/kapi/devices/device_registry.cpp index f5c20a6d..80ddde65 100644 --- a/kernel/kapi/devices/device_registry.cpp +++ b/kernel/kapi/devices/device_registry.cpp @@ -4,6 +4,7 @@ #include <kapi/system.hpp> #include <kstd/memory.hpp> +#include <kstd/mutex.hpp> #include <kstd/print.hpp> #include <kstd/string.hpp> #include <kstd/vector.hpp> @@ -27,7 +28,7 @@ namespace kapi::devices system::panic("[OS:DEV] Device registry has already been initialized"); } - instance = device_registry{}; + instance.emplace(); } auto device_registry::get() -> device_registry & @@ -50,33 +51,38 @@ namespace kapi::devices kstd::println("[OS:DEV] Registering device {}", device->name()); auto added = false; - - // TODO: lock the registry - auto found = m_devices.find(device->name()); - if (found != m_devices.end()) { - if (!found->second.expired()) + auto guard = kstd::lock_guard{m_lock}; + + auto found = m_devices.find(device->name()); + if (found != m_devices.end()) { - added = false; - } + if (!found->second.expired()) + { + added = false; + } - found->second = device; - added = true; - } - else - { - added = m_devices.emplace(device->name(), device).second; + found->second = device; + added = true; + } + else + { + added = m_devices.emplace(device->name(), device).second; + } } - // TODO: unlock registry if (!added) { return false; } - // TODO: consider if we need to lock the registry observers here, since somebody may add an observer in the meantime - // once multiple codepaths can run at the same time. - for (auto const & observer : m_observers) + auto observers = kstd::vector<kstd::weak_ptr<device_registry_observer>>{}; + { + auto guard = kstd::lock_guard{m_lock}; + observers = m_observers; + } + + for (auto const & observer : observers) { if (auto locked_observer = observer.lock()) { @@ -84,36 +90,43 @@ namespace kapi::devices } } - // TODO: lock registry - erase_if(m_observers, [](auto const & observer) { return observer.expired(); }); - // TODO: unlock registry + { + auto guard = kstd::lock_guard{m_lock}; + erase_if(m_observers, [](auto const & observer) { return observer.expired(); }); + } return true; } auto device_registry::remove(device & device) -> bool { - // TODO: lock registry - - auto found = m_devices.find(device.name()); - if (found == m_devices.end()) { - return false; - } + auto guard = kstd::lock_guard{m_lock}; - auto locked = found->second.lock(); - if (!locked || locked.get() != &device) - { - return false; - } + auto found = m_devices.find(device.name()); + if (found == m_devices.end()) + { + return false; + } - found->second = kstd::weak_ptr<struct device>{}; + auto locked = found->second.lock(); + if (!locked || locked.get() != &device) + { + return false; + } - // TODO: unlock registry + found->second = kstd::weak_ptr<struct device>{}; + } kstd::println("[OS:DEV] Unregistering device {}", device.name()); - for (auto const & observer : m_observers) + auto observers = kstd::vector<kstd::weak_ptr<device_registry_observer>>{}; + { + auto guard = kstd::lock_guard{m_lock}; + observers = m_observers; + } + + for (auto const & observer : observers) { if (auto locked_observer = observer.lock()) { @@ -121,16 +134,17 @@ namespace kapi::devices } } - // TODO: lock registry - erase_if(m_observers, [](auto const & observer) { return observer.expired(); }); - // TODO: unlock registry + { + auto guard = kstd::lock_guard{m_lock}; + erase_if(m_observers, [](auto const & observer) { return observer.expired(); }); + } return true; } auto device_registry::find(std::string_view name) const -> kstd::shared_ptr<device> { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; auto found = m_devices.find(kstd::string{name}); if (found == m_devices.end()) @@ -143,7 +157,7 @@ namespace kapi::devices auto device_registry::all() const -> kstd::vector<kstd::shared_ptr<device>> { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; auto result = kstd::vector<kstd::shared_ptr<device>>{}; @@ -160,7 +174,7 @@ namespace kapi::devices auto device_registry::subscribe(kstd::weak_ptr<device_registry_observer> observer) -> void { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; erase_if(m_observers, [](auto const & observer) { return observer.expired(); }); m_observers.push_back(observer); diff --git a/kernel/kapi/devices/facet_registry.cpp b/kernel/kapi/devices/facet_registry.cpp index 4aeebfc1..623ecba2 100644 --- a/kernel/kapi/devices/facet_registry.cpp +++ b/kernel/kapi/devices/facet_registry.cpp @@ -5,6 +5,7 @@ #include <kapi/test_support/devices.hpp> #include <kstd/memory.hpp> +#include <kstd/mutex.hpp> #include <kstd/result.hpp> #include <kstd/string.hpp> #include <kstd/system_error.hpp> @@ -46,64 +47,87 @@ namespace kapi::devices auto facet_registry::do_publish(kstd::shared_ptr<device> device, kstd::string name, kapi::capabilities::facet_id id, void * facet) -> kstd::result<void> { - // TODO: lock registry + auto published = std::optional<entry>{}; - erase_if(m_entries, [id](auto e) { return e.id() == id && !e.device(); }); - - if (!device || !facet || name.empty()) { - return kstd::failure(make_error_code(kstd::errc::invalid_argument)); - } + auto guard = kstd::lock_guard{m_lock}; - auto already_published = std::ranges::any_of( - m_entries, [&](auto const & entry) { return entry.id() == id && entry.device().get() == device.get(); }); + erase_if(m_entries, [id](auto e) { return e.id() == id && !e.device(); }); - if (already_published) - { - return kstd::failure(make_error_code(kstd::errc::file_exists)); - } + if (!device || !facet || name.empty()) + { + return kstd::failure(make_error_code(kstd::errc::invalid_argument)); + } - auto & published = m_entries.emplace_back(device, std::move(name), id, facet); + auto already_published = std::ranges::any_of( + m_entries, [&](auto const & entry) { return entry.id() == id && entry.device().get() == device.get(); }); - do_notify_published(id, published); + if (already_published) + { + return kstd::failure(make_error_code(kstd::errc::file_exists)); + } + + published = m_entries.emplace_back(device, std::move(name), id, facet); + } + + do_notify_published(id, *published); return kstd::success(); } auto facet_registry::do_notify_published(kapi::capabilities::facet_id id, entry const & published) -> void { - std::ranges::for_each(m_observers, [&](auto observer) { + auto observers = kstd::vector<kstd::weak_ptr<facet_registry_observer>>{}; + auto static_observers = kstd::vector<facet_registry_observer *>{}; + { + auto guard = kstd::lock_guard{m_lock}; + observers = m_observers; + static_observers = m_static_observers; + } + + std::ranges::for_each(observers, [&](auto observer) { if (auto locked_observer = observer.lock()) { locked_observer->on_facet_published(id, published); } }); - std::ranges::for_each(m_static_observers, [&](auto observer) { observer->on_facet_published(id, published); }); + std::ranges::for_each(static_observers, [&](auto observer) { observer->on_facet_published(id, published); }); } auto facet_registry::do_notify_withdrawn(kapi::capabilities::facet_id id, device const & device) -> void { - std::ranges::for_each(m_observers, [&](auto observer) { + auto observers = kstd::vector<kstd::weak_ptr<facet_registry_observer>>{}; + auto static_observers = kstd::vector<facet_registry_observer *>{}; + { + auto guard = kstd::lock_guard{m_lock}; + observers = m_observers; + static_observers = m_static_observers; + } + + std::ranges::for_each(observers, [&](auto observer) { if (auto locked_observer = observer.lock()) { locked_observer->on_facet_withdrawn(id, const_cast<devices::device &>(device)); } }); - std::ranges::for_each(m_static_observers, [&](auto observer) { + std::ranges::for_each(static_observers, [&](auto observer) { observer->on_facet_withdrawn(id, const_cast<devices::device &>(device)); }); } auto facet_registry::withdraw(device const & device, kapi::capabilities::facet_id id) -> void { - // TODO: lock registry + auto did_remove = false; - auto did_remove = erase_if(m_entries, [&](auto e) { - auto locked_device = e.device(); - return e.id() == id && locked_device && locked_device.get() == &device; - }) != 0; + { + auto guard = kstd::lock_guard{m_lock}; + did_remove = erase_if(m_entries, [&](auto e) { + auto locked_device = e.device(); + return e.id() == id && locked_device && locked_device.get() == &device; + }) != 0; + } if (did_remove) { @@ -113,26 +137,28 @@ namespace kapi::devices auto facet_registry::withdraw_all_for(device const & device) -> void { - // TODO: lock registry - auto withdrawn_facets = kstd::vector<kapi::capabilities::facet_id>{}; - erase_if(m_entries, [&](auto e) { - auto locked_device = e.device(); - auto do_erase = locked_device && locked_device.get() == &device; - if (do_erase) - { - withdrawn_facets.push_back(e.id()); - } - return do_erase; - }); + { + auto guard = kstd::lock_guard{m_lock}; + + erase_if(m_entries, [&](auto e) { + auto locked_device = e.device(); + auto do_erase = locked_device && locked_device.get() == &device; + if (do_erase) + { + withdrawn_facets.push_back(e.id()); + } + return do_erase; + }); + } std::ranges::for_each(withdrawn_facets, [&](auto facet) { do_notify_withdrawn(facet, device); }); } auto facet_registry::all(kapi::capabilities::facet_id id) const -> kstd::vector<entry> { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; auto filtered = m_entries; erase_if(filtered, [&](auto e) { @@ -142,33 +168,39 @@ namespace kapi::devices return filtered; } - auto facet_registry::resolve(kapi::capabilities::facet_id id, std::string_view name) -> void * + auto facet_registry::resolve(kapi::capabilities::facet_id facet, std::string_view name) -> void * { - // TODO: lock registry + auto found_device = kstd::shared_ptr<device>{}; + { + auto guard = kstd::lock_guard{m_lock}; - auto found = std::ranges::find_if(m_entries, [&](auto e) { return e.name() == name && e.device(); }); + auto found = std::ranges::find_if(m_entries, [&](auto e) { return e.name() == name && e.device(); }); - if (found == m_entries.cend()) - { - return nullptr; + if (found == m_entries.cend()) + { + return nullptr; + } + + found_device = found->device(); } - else if (auto device = found->device()) + + if (!found_device) { - return resolve(id, *device); + return nullptr; } - return nullptr; + return resolve(facet, *found_device); } auto facet_registry::resolve(kapi::capabilities::facet_id id, device & device) -> void * { - // TODO: lock registry - if (auto by_device = device.facet(id)) { return by_device; } + auto guard = kstd::lock_guard{m_lock}; + auto found = std::ranges::find_if(m_entries, [&](auto e) { auto locked_device = e.device(); return e.id() == id && locked_device && locked_device.get() == &device; @@ -184,7 +216,7 @@ namespace kapi::devices auto facet_registry::subscribe(kstd::weak_ptr<facet_registry_observer> observer) -> void { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; erase_if(m_observers, [](auto const & observer) { return observer.expired(); }); m_observers.push_back(observer); @@ -192,14 +224,14 @@ namespace kapi::devices auto facet_registry::subscribe(facet_registry_observer & observer) -> void { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; m_static_observers.push_back(&observer); } auto facet_registry::unsubscribe(facet_registry_observer & observer) -> void { - // TODO: lock registry + auto guard = kstd::lock_guard{m_lock}; erase_if(m_observers, [&](auto const & subscribed) { return subscribed.lock().get() == &observer; }); erase(m_static_observers, &observer); |