From c0035251016d3b2614aeb69579dd524fb01774da Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 15 Jul 2024 14:08:15 -0400 Subject: [PATCH] USB: Release the device manager lock before exploring. Exploring can take a while, and if we hold the device manager lock the whole time, we'll block a lot of other things on the system (including open() calls to anything in devfs.) So, instead of holding the lock continuously, we now lock it only temporarily, and instead add protection against deadlocks when called by other threads besides the explore thread. --- src/add-ons/kernel/bus_managers/usb/Stack.cpp | 26 +++++++++++++++---- .../kernel/bus_managers/usb/usb_private.h | 2 +- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/add-ons/kernel/bus_managers/usb/Stack.cpp b/src/add-ons/kernel/bus_managers/usb/Stack.cpp index 31d7633118..9dba7e9a61 100644 --- a/src/add-ons/kernel/bus_managers/usb/Stack.cpp +++ b/src/add-ons/kernel/bus_managers/usb/Stack.cpp @@ -217,11 +217,26 @@ Stack::ExploreThread(void *data) void Stack::Explore() { - // Acquire the device manager lock before the explore lock, to prevent lock-order inversion. - RecursiveLocker dmLocker(device_manager_get_lock()); + recursive_lock* dmLock = device_manager_get_lock(); + if (find_thread(NULL) != fExploreThread + && RECURSIVE_LOCK_HOLDER(dmLock) == find_thread(NULL)) { + // This should only happen during the initial device scan, during which + // we should be able to acquire the explore lock immediately (since the + // explore thread will be waiting on the device manager lock as below), + // but in case we aren't, use a timeout to avoid lock-order-inversion deadlocks. + if (mutex_lock_with_timeout(&fExploreLock, B_RELATIVE_TIMEOUT, 1000) != B_OK) { + release_sem(fExploreSem); + return; + } + } else { + // Temporarily acquire the device manager lock, to ensure it isn't scanning. + RecursiveLocker dmLocker(dmLock); - if (mutex_lock(&fExploreLock) != B_OK) - return; + if (mutex_lock(&fExploreLock) != B_OK) + return; + + dmLocker.Unlock(); + } int32 semCount = 0; get_sem_count(fExploreSem, &semCount); @@ -256,12 +271,13 @@ Stack::Explore() void Stack::AddBusManager(BusManager *busManager) { + MutexLocker _(fExploreLock); fBusManagers.PushBack(busManager); } int32 -Stack::IndexOfBusManager(BusManager *busManager) +Stack::IndexOfBusManager(BusManager *busManager) const { return fBusManagers.IndexOf(busManager); } diff --git a/src/add-ons/kernel/bus_managers/usb/usb_private.h b/src/add-ons/kernel/bus_managers/usb/usb_private.h index 659bd92889..dbad91b16c 100644 --- a/src/add-ons/kernel/bus_managers/usb/usb_private.h +++ b/src/add-ons/kernel/bus_managers/usb/usb_private.h @@ -143,7 +143,7 @@ public: Object * GetObjectNoLock(usb_id id) const; void AddBusManager(BusManager *bus); - int32 IndexOfBusManager(BusManager *bus); + int32 IndexOfBusManager(BusManager *bus) const; BusManager * BusManagerAt(int32 index) const; status_t AllocateChunk(void **logicalAddress,