* 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

@@ -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;

src/system/kernel/device_manager/IOScheduler.cpp

@@ -20,6 +20,14 @@
 #include <util/AutoLock.h>
 
 
+#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 {

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;

src/system/kernel/device_manager/io_requests.cpp

@@ -9,6 +9,7 @@
 #include <string.h>
 
 #include <team.h>
+#include <util/AutoLock.h>
 #include <vm.h>
 
 #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
+	fFinishedCallback = callback;
-	if (fStatus <= 0) {
+	fFinishedCookie = cookie;
-		// we're already done
+}
-		return;
+
+
+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)
+	if (--fPendingChildren > 0)
-		fParent->ChunkFinished(this, Status());
+		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);
 }
 
 

src/system/kernel/device_manager/io_requests.h

@@ -10,6 +10,8 @@
 
 #include <SupportDefs.h>
 
+#include <condition_variable.h>
+#include <lock.h>
 #include <util/DoublyLinkedList.h>
 
 #include "dma_resources.h"
@@ -21,6 +23,7 @@
 struct DMABuffer;
 struct IOOperation;
 
+typedef struct IOOperation io_operation;
 
 class IOBuffer : public DoublyLinkedListLinkImpl<IOBuffer> {
 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<IORequestChunk>*
 									ListLink()	{ return &fListLink; }
@@ -158,6 +163,10 @@ protected:
 typedef IOOperation io_operation;
 typedef DoublyLinkedList<IOOperation> IOOperationList;
 
+typedef struct IORequest io_request;
+typedef status_t (*io_request_finished_callback)(void* data,
+	io_request* request);
+
 
 struct IORequest : IORequestChunk, DoublyLinkedListLinkImpl<IORequest> {
 								IORequest();
@@ -168,8 +177,16 @@ struct IORequest : IORequestChunk, DoublyLinkedListLinkImpl<IORequest> {
 			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;