From 902559ce32c16724ad24cfa90582097ba8d6fd43 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Thu, 24 Jul 2008 01:54:41 +0000 Subject: [PATCH] * Added a mutex to IORequest. It doesn't look like we can get around using a lock and I'm not very much in favor of a global one. * Added "finished" callback to IORequest. * IOOperation::Finish() no longer invokes its parent request's ChunkFinished(). The finisher does this instead. ChunkFinished() can optionally remove the chunk from the parent. * Added IORequest::Wait() which waits for the completion of the request. * Introduced IORequestChunk::ResetStatus() to make setting the status to "pending" somewhat more explicit. * Implemented the missing IOScheduler::SetCallback() methods. * The NotifyAll() calls on the IOScheduler's condition variables were missing, so it just waited forever. * Added some more debug output. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26597 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/os/drivers/device_manager.h | 2 +- .../kernel/device_manager/IOScheduler.cpp | 63 +++++++++-- .../kernel/device_manager/IOScheduler.h | 2 + .../kernel/device_manager/io_requests.cpp | 103 +++++++++++++++--- .../kernel/device_manager/io_requests.h | 29 ++++- 5 files changed, 170 insertions(+), 29 deletions(-) diff --git a/headers/os/drivers/device_manager.h b/headers/os/drivers/device_manager.h index 5cf2abf154..2b11af2c73 100644 --- a/headers/os/drivers/device_manager.h +++ b/headers/os/drivers/device_manager.h @@ -153,7 +153,7 @@ struct driver_module_info { /* interface of device */ -typedef struct io_request io_request; +typedef struct IORequest io_request; struct device_module_info { module_info info; diff --git a/src/system/kernel/device_manager/IOScheduler.cpp b/src/system/kernel/device_manager/IOScheduler.cpp index 8daeb1cdd7..ac59aa8de6 100644 --- a/src/system/kernel/device_manager/IOScheduler.cpp +++ b/src/system/kernel/device_manager/IOScheduler.cpp @@ -20,6 +20,14 @@ #include +#define TRACE_IO_SCHEDULER +#ifdef TRACE_IO_SCHEDULER +# define TRACE(x...) dprintf(x) +#else +# define TRACE(x...) ; +#endif + + IOScheduler::IOScheduler(DMAResource* resource) : fDMAResource(resource), @@ -66,9 +74,33 @@ IOScheduler::Init(const char* name) } +void +IOScheduler::SetCallback(IOCallback& callback) +{ + SetCallback(&_IOCallbackWrapper, &callback); +} + + +void +IOScheduler::SetCallback(io_callback callback, void* data) +{ + fIOCallback = callback; + fIOCallbackData = data; +} + + +/*static*/ status_t +IOScheduler::_IOCallbackWrapper(void* data, io_operation* operation) +{ + return ((IOCallback*)data)->DoIO(operation); +} + + status_t IOScheduler::ScheduleRequest(IORequest* request) { + TRACE("IOScheduler::ScheduleRequest(%p)\n", request); + IOBuffer* buffer = request->Buffer(); // TODO: it would be nice to be able to lock the memory later, but we can't @@ -83,6 +115,7 @@ IOScheduler::ScheduleRequest(IORequest* request) MutexLocker _(fLock); fUnscheduledRequests.Add(request); + fNewRequestCondition.NotifyAll(); return B_OK; } @@ -108,6 +141,7 @@ IOScheduler::OperationCompleted(IOOperation* operation, status_t status) operation->SetStatus(status); fCompletedOperations.Add(operation); + fFinishedOperationCondition.NotifyAll(); if (fWaiting) { SpinLocker _2(thread_spinlock); @@ -128,18 +162,26 @@ IOScheduler::_Finisher() locker.Unlock(); - if (!operation->Finish()) { + TRACE("IOScheduler::_Finisher(): operation: %p\n", operation); + + while (!operation->Finish()) { + TRACE(" operation: %p not finished yet\n", operation); // TODO: This must be done differently once the scheduler implements // an actual scheduling policy (other than no-op). fIOCallback(fIOCallbackData, operation); - } else { - MutexLocker _(fLock); - operation->Parent()->RemoveOperation(operation); - if (fDMAResource != NULL) - fDMAResource->RecycleBuffer(operation->Buffer()); - - fUnusedOperations.Add(operation); } + + // notify request and remove operation + IORequest* request = operation->Parent(); + if (request != NULL) + request->ChunkFinished(operation, operation->Status(), true); + + // recycle the operation + MutexLocker _(fLock); + if (fDMAResource != NULL) + fDMAResource->RecycleBuffer(operation->Buffer()); + + fUnusedOperations.Add(operation); } } @@ -229,6 +271,8 @@ IOScheduler::_Scheduler() while (true) { IORequest* request = _GetNextUnscheduledRequest(); + TRACE("IOScheduler::_Scheduler(): request: %p\n", request); + if (fDMAResource != NULL) { while (request->RemainingBytes() > 0) { IOOperation* operation = _GetOperation(); @@ -240,6 +284,9 @@ IOScheduler::_Scheduler() break; } + TRACE("IOScheduler::_Scheduler(): calling callback for " + "operation: %p\n", operation); + fIOCallback(fIOCallbackData, operation); } } else { diff --git a/src/system/kernel/device_manager/IOScheduler.h b/src/system/kernel/device_manager/IOScheduler.h index 9287d2abd4..94de0c8d4d 100644 --- a/src/system/kernel/device_manager/IOScheduler.h +++ b/src/system/kernel/device_manager/IOScheduler.h @@ -52,6 +52,8 @@ private: status_t _Scheduler(); static status_t _SchedulerThread(void* self); + static status_t _IOCallbackWrapper(void* data, + io_operation* operation); private: DMAResource* fDMAResource; diff --git a/src/system/kernel/device_manager/io_requests.cpp b/src/system/kernel/device_manager/io_requests.cpp index f71eb2bbff..b7952f647c 100644 --- a/src/system/kernel/device_manager/io_requests.cpp +++ b/src/system/kernel/device_manager/io_requests.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include "dma_resources.h" @@ -131,7 +132,7 @@ IOOperation::Finish() fPhase = HasPartialEnd() && !skipReadEndPhase ? PHASE_READ_END : PHASE_DO_ALL; _PrepareVecs(); - SetStatus(1); + ResetStatus(); // TODO: Is there a race condition, if the request is // aborted at the same time? return false; @@ -152,7 +153,7 @@ IOOperation::Finish() // We're done with the second phase only (read in end). // Get ready for next phase... fPhase = PHASE_DO_ALL; - SetStatus(1); + ResetStatus(); // TODO: Is there a race condition, if the request is // aborted at the same time? return false; @@ -215,10 +216,6 @@ IOOperation::Finish() SetStatus(error); } - // notify parent request - if (fParent != NULL) - fParent->ChunkFinished(this, fStatus); - return true; } @@ -310,7 +307,7 @@ IOOperation::Prepare(IORequest* request) _PrepareVecs(); } - fStatus = 1; + ResetStatus(); if (fParent != NULL) fParent->AddOperation(this); @@ -487,12 +484,19 @@ IOOperation::_CopyPartialEnd(bool isWrite) IORequest::IORequest() + : + fFinishedCallback(NULL), + fFinishedCookie(NULL) { + mutex_init(&fLock, "I/O request lock"); + fFinishedCondition.Init(this, "I/O request finished"); } IORequest::~IORequest() { + mutex_lock(&fLock); + mutex_destroy(&fLock); } @@ -528,23 +532,86 @@ IORequest::Init(off_t offset, iovec* vecs, size_t count, size_t length, fVecOffset = 0; fRemainingBytes = length; + fPendingChildren = 0; + + fStatus = 1; + return B_OK; } void -IORequest::ChunkFinished(IORequestChunk* chunk, status_t status) +IORequest::SetFinishedCallback(io_request_finished_callback callback, + void* cookie) { - // TODO: we would need to update status atomically - if (fStatus <= 0) { - // we're already done - return; + fFinishedCallback = callback; + fFinishedCookie = cookie; +} + + +status_t +IORequest::Wait(uint32 flags, bigtime_t timeout) +{ + MutexLocker locker(fLock); + + if (IsFinished()) + return Status(); + + ConditionVariableEntry entry; + fFinishedCondition.Add(&entry); + + locker.Unlock(); + + status_t error = entry.Wait(B_CAN_INTERRUPT); + if (error != B_OK) + return error; + + return Status(); +} + + +void +IORequest::ChunkFinished(IORequestChunk* chunk, status_t status, bool remove) +{ + MutexLocker locker(fLock); + + if (remove) { + fChildren.Remove(chunk); + chunk->SetParent(NULL); } - fStatus = status; + if (status != B_OK && fStatus == 1) + fStatus = status; - if (fParent != NULL) - fParent->ChunkFinished(this, Status()); + if (--fPendingChildren > 0) + return; + + // last child finished + + // set status, if not done yet + if (fStatus == 1) + fStatus = B_OK; + + // Cache the callbacks before we unblock waiters and unlock. Any of the + // following could delete this request, so we don't want to touch it once + // we have started telling others that it is done. + IORequest* parent = fParent; + io_request_finished_callback finishedCallback = fFinishedCallback; + void* finishedCookie = fFinishedCookie; + status = fStatus; + + // unblock waiters + fFinishedCondition.NotifyAll(); + + locker.Unlock(); + + // notify callback + if (finishedCallback != NULL) + finishedCallback(finishedCookie, this); + + // notify parent + if (parent != NULL) + parent->ChunkFinished(this, status, false); } @@ -569,16 +636,18 @@ IORequest::Advance(size_t bySize) void IORequest::AddOperation(IOOperation* operation) { - // TODO: locking? + MutexLocker locker(fLock); fChildren.Add(operation); + fPendingChildren++; } void IORequest::RemoveOperation(IOOperation* operation) { - // TODO: locking? + MutexLocker locker(fLock); fChildren.Remove(operation); + operation->SetParent(NULL); } diff --git a/src/system/kernel/device_manager/io_requests.h b/src/system/kernel/device_manager/io_requests.h index ef09a52439..fb46611844 100644 --- a/src/system/kernel/device_manager/io_requests.h +++ b/src/system/kernel/device_manager/io_requests.h @@ -10,6 +10,8 @@ #include +#include +#include #include #include "dma_resources.h" @@ -21,6 +23,7 @@ struct DMABuffer; struct IOOperation; +typedef struct IOOperation io_operation; class IOBuffer : public DoublyLinkedListLinkImpl { public: @@ -71,13 +74,15 @@ public: IORequestChunk(); virtual ~IORequestChunk(); -// virtual status_t Wait(bigtime_t timeout = B_INFINITE_TIMEOUT); - IORequest* Parent() const { return fParent; } + void SetParent(IORequest* parent) + { fParent = parent; } status_t Status() const { return fStatus; } void SetStatus(status_t status) { fStatus = status; } + void ResetStatus() + { fStatus = 1; } DoublyLinkedListLink* ListLink() { return &fListLink; } @@ -158,6 +163,10 @@ protected: typedef IOOperation io_operation; typedef DoublyLinkedList IOOperationList; +typedef struct IORequest io_request; +typedef status_t (*io_request_finished_callback)(void* data, + io_request* request); + struct IORequest : IORequestChunk, DoublyLinkedListLinkImpl { IORequest(); @@ -168,8 +177,16 @@ struct IORequest : IORequestChunk, DoublyLinkedListLinkImpl { status_t Init(off_t offset, iovec* vecs, size_t count, size_t length, bool write, uint32 flags); + void SetFinishedCallback( + io_request_finished_callback callback, + void* cookie); + + status_t Wait(uint32 flags = 0, bigtime_t timeout = 0); + + bool IsFinished() const { return fStatus != 1; } + void ChunkFinished(IORequestChunk* chunk, - status_t status); + status_t status, bool remove); size_t RemainingBytes() const { return fRemainingBytes; } @@ -204,14 +221,20 @@ private: static status_t _CopyUser(void* bounceBuffer, void* external, size_t size, bool copyIn); + mutex fLock; IOBuffer* fBuffer; off_t fOffset; size_t fLength; IORequestChunkList fChildren; + int32 fPendingChildren; uint32 fFlags; team_id fTeam; bool fIsWrite; + io_request_finished_callback fFinishedCallback; + void* fFinishedCookie; + ConditionVariable fFinishedCondition; + // these are for iteration uint32 fVecIndex; size_t fVecOffset;