From c1990557b889feddc35cf926cf62cd2277297027 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Thu, 22 May 2008 16:40:33 +0000 Subject: [PATCH] * Use a lazily initialized notification sem for all SendRequest() calls. * Also use member variables to transport the status information from the callback instead of allocating a dedicated structure each time. * Remove the now unused transfer_result_data struct. This should bring down some overhead otherwise involved with sending requests over control pipes. As this is heavily used in updating for example the hub status (also on roothubs), this change should make the polling loop a lot cheaper. Note that this makes SendRequest() non-thread safe, but as it is a synchronous function (as opposed to QueueRequest) and since the underlying architecture is not thread safe either this shouldn't be a problem (drivers doing a SendRequest from two different threads would have failed previously anyway). A benaphore or mutex could be employed to fix that if the need really arises. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25613 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/add-ons/kernel/bus_managers/usb/Pipe.cpp | 71 ++++++++++---------- src/add-ons/kernel/bus_managers/usb/usb_p.h | 6 ++ 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/add-ons/kernel/bus_managers/usb/Pipe.cpp b/src/add-ons/kernel/bus_managers/usb/Pipe.cpp index 53c3b0f6c1..78ca5a4206 100644 --- a/src/add-ons/kernel/bus_managers/usb/Pipe.cpp +++ b/src/add-ons/kernel/bus_managers/usb/Pipe.cpp @@ -289,58 +289,58 @@ IsochronousPipe::GetPipePolicy(uint8 *maxQueuedPackets, // -typedef struct transfer_result_data_s { - sem_id notify_sem; - status_t status; - size_t actual_length; -} transfer_result_data; - - ControlPipe::ControlPipe(Object *parent) - : Pipe(parent) + : Pipe(parent), + fNotifySem(-1) { } +ControlPipe::~ControlPipe() +{ + if (fNotifySem >= 0) + delete_sem(fNotifySem); +} + + status_t ControlPipe::SendRequest(uint8 requestType, uint8 request, uint16 value, uint16 index, uint16 length, void *data, size_t dataLength, size_t *actualLength) { - transfer_result_data *transferResult - = new(std::nothrow) transfer_result_data; - - transferResult->notify_sem = create_sem(0, "usb send request notify"); - if (transferResult->notify_sem < B_OK) - return B_NO_MORE_SEMS; - - status_t result = QueueRequest(requestType, request, value, index, length, - data, dataLength, SendRequestCallback, transferResult); - if (result < B_OK) { - delete_sem(transferResult->notify_sem); - return result; + if (fNotifySem < 0) { + fNotifySem = create_sem(0, "usb send request notify"); + if (fNotifySem < 0) + return B_NO_MORE_SEMS; } - // the sem will be released in the callback after the result data was - // filled into the provided struct. use a 1 second timeout to avoid - // hanging applications. - if (acquire_sem_etc(transferResult->notify_sem, 1, B_RELATIVE_TIMEOUT, 1000000) < B_OK) { + status_t result = QueueRequest(requestType, request, value, index, length, + data, dataLength, SendRequestCallback, this); + if (result < B_OK) + return result; + + // The sem will be released unconditionally in the callback after the + // result data was filled in. Use a 1 second timeout for control transfers. + if (acquire_sem_etc(fNotifySem, 1, B_RELATIVE_TIMEOUT, 1000000) < B_OK) { TRACE_ERROR(("USB ControlPipe: timeout waiting for queued request to complete\n")); - delete_sem(transferResult->notify_sem); + CancelQueuedTransfers(false); + + // After the above cancel returns it is guaranteed that the callback + // has been invoked. Therefore we can simply grab that released + // semaphore again to clean up. + acquire_sem_etc(fNotifySem, 1, B_RELATIVE_TIMEOUT, 0); + if (actualLength) *actualLength = 0; - CancelQueuedTransfers(false); return B_TIMED_OUT; } - delete_sem(transferResult->notify_sem); if (actualLength) - *actualLength = transferResult->actual_length; + *actualLength = fActualLength; - result = transferResult->status; - delete transferResult; + result = fTransferStatus; return result; } @@ -349,13 +349,10 @@ void ControlPipe::SendRequestCallback(void *cookie, status_t status, void *data, size_t actualLength) { - transfer_result_data *transferResult = (transfer_result_data *)cookie; - transferResult->status = status; - transferResult->actual_length = actualLength; - if (release_sem(transferResult->notify_sem) < B_OK) { - // the request has timed out already - cleanup after us - delete transferResult; - } + ControlPipe *pipe = (ControlPipe *)cookie; + pipe->fTransferStatus = status; + pipe->fActualLength = actualLength; + release_sem(pipe->fNotifySem); } diff --git a/src/add-ons/kernel/bus_managers/usb/usb_p.h b/src/add-ons/kernel/bus_managers/usb/usb_p.h index b80d436c82..719d5f8ab5 100644 --- a/src/add-ons/kernel/bus_managers/usb/usb_p.h +++ b/src/add-ons/kernel/bus_managers/usb/usb_p.h @@ -305,6 +305,7 @@ private: class ControlPipe : public Pipe { public: ControlPipe(Object *parent); +virtual ~ControlPipe(); virtual uint32 Type() { return USB_OBJECT_PIPE | USB_OBJECT_CONTROL_PIPE; }; @@ -331,6 +332,11 @@ static void SendRequestCallback(void *cookie, void *data, size_t dataLength, usb_callback_func callback, void *callbackCookie); + +private: + sem_id fNotifySem; + status_t fTransferStatus; + size_t fActualLength; };