IORequest: Correct major oversight in finished callback API.

The IORequest internally likes to deal with transferEndOffset
not transferredBytes because of sub-requests potentially being
prepared all at once (in some paths in the I/O scheduler),
thus fTransferSize can get incremented in Advance() before we have
actually executed that transfer.

But external consumers much prefer just knowing transferredBytes
not transferEndOffset. And many of them actually named their
variables that (or "bytesTransferred") and just passed the
transferEndOffset through to variables with that name! That's
obviously wrong, and it's surprising it wasn't discovered before now.

The problem was uncovered by repeated KDLs in PrecacheIO.
That method used the "bytesTransferred" value as a count of
pages transferred, which would then run past the end of the array
if the transfer start offset was not 0 (which the majority
of the time it would be, since this method gets called on
the first mmap() of a file, probably before any pages are read in.)

Most other consumers of this API did not check the value, it seems,
or otherwise had some mitigating factor that prevented it from
causing more problems. An exception is the page code, which
may have spuriously considered writes as successful when they
really weren't.

May fix some of the "invalid concurrent access to page" KDLs.

headers/private/kernel/vfs.h

@@ -302,10 +302,10 @@ public:
 									bool partialTransfer,
 									generic_size_t bytesTransferred) = 0;
 
-	static	status_t 			IORequestCallback(void* data,
+	static	void	 			IORequestCallback(void* data,
 									io_request* request, status_t status,
 									bool partialTransfer,
-									generic_size_t transferEndOffset);
+									generic_size_t bytesTransferred);
 };
 
 

src/system/kernel/cache/file_cache.cpp

@@ -195,7 +195,7 @@ PrecacheIO::IOFinished(status_t status, bool partialTransfer,
 	phys_size_t pagesTransferred
 		= (bytesTransferred + B_PAGE_SIZE - 1) / B_PAGE_SIZE;
 
-	if (fOffset + (off_t)bytesTransferred > fCache->virtual_end)
+	if ((fOffset + (off_t)bytesTransferred) > fCache->virtual_end)
 		bytesTransferred = fCache->virtual_end - fOffset;
 
 	for (uint32 i = 0; i < pagesTransferred; i++) {

src/system/kernel/device_manager/IORequest.cpp

@@ -965,6 +965,7 @@ IORequest::NotifyFinished()
 	ASSERT(fPendingChildren == 0);
 	ASSERT(fChildren.IsEmpty()
 		|| dynamic_cast<IOOperation*>(fChildren.Head()) == NULL);
+	ASSERT(fTransferSize <= fLength);
 
 	// unlock the memory
 	if (fBuffer->IsMemoryLocked())
@@ -977,8 +978,9 @@ IORequest::NotifyFinished()
 	io_request_finished_callback finishedCallback = fFinishedCallback;
 	void* finishedCookie = fFinishedCookie;
 	status_t status = fStatus;
+	generic_size_t transferredBytes = fTransferSize;
 	generic_size_t lastTransferredOffset
-		= fRelativeParentOffset + fTransferSize;
+		= fRelativeParentOffset + transferredBytes;
 	bool partialTransfer = status != B_OK || fPartialTransfer;
 	bool deleteRequest = (fFlags & B_DELETE_IO_REQUEST) != 0;
 
@@ -991,7 +993,7 @@ IORequest::NotifyFinished()
 	// notify callback
 	if (finishedCallback != NULL) {
 		finishedCallback(finishedCookie, this, status, partialTransfer,
-			lastTransferredOffset);
+			transferredBytes);
 	}
 
 	// notify parent
@@ -1076,8 +1078,8 @@ void
 IORequest::SubRequestFinished(IORequest* request, status_t status,
 	bool partialTransfer, generic_size_t transferEndOffset)
 {
-	TRACE("IORequest::SubrequestFinished(%p, %#" B_PRIx32 ", %d, %"
+	TRACE("IORequest::SubrequestFinished(%p, %#" B_PRIx32 ", %d, %" B_PRIuGENADDR
-		B_PRIuGENADDR "): request: %p\n", request, status, partialTransfer, transferEndOffset, this);
+		"): request: %p\n", request, status, partialTransfer, transferEndOffset, this);
 
 	MutexLocker locker(fLock);
 
@@ -1122,8 +1124,6 @@ IORequest::SetTransferredBytes(bool partialTransfer,
 
 	MutexLocker _(fLock);
 
-	ASSERT(transferredBytes <= fLength);
-
 	fPartialTransfer = partialTransfer;
 	fTransferSize = transferredBytes;
 }

src/system/kernel/device_manager/IORequest.h

@@ -199,10 +199,9 @@ typedef IOOperation io_operation;
 typedef DoublyLinkedList<IOOperation> IOOperationList;
 
 typedef struct IORequest io_request;
-typedef status_t (*io_request_finished_callback)(void* data,
+typedef void (*io_request_finished_callback)(void* data,
 			io_request* request, status_t status, bool partialTransfer,
-			generic_size_t transferEndOffset);
+			generic_size_t transferredBytes);
-			// TODO: Return type: status_t -> void
 typedef status_t (*io_request_iterate_callback)(void* data,
 			io_request* request, bool* _partialTransfer);
 

src/system/kernel/fs/vfs_request_io.cpp

@@ -26,13 +26,12 @@ AsyncIOCallback::~AsyncIOCallback()
 }
 
 
-/* static */ status_t
+/* static */ void
 AsyncIOCallback::IORequestCallback(void* data, io_request* request,
-	status_t status, bool partialTransfer, generic_size_t transferEndOffset)
+	status_t status, bool partialTransfer, generic_size_t bytesTransferred)
 {
 	((AsyncIOCallback*)data)->IOFinished(status, partialTransfer,
-		transferEndOffset);
+		bytesTransferred);
-	return B_OK;
 }
 
 
@@ -257,27 +256,25 @@ do_iterative_fd_io_iterate(void* _cookie, io_request* request,
 }
 
 
-static status_t
+static void
 do_iterative_fd_io_finish(void* _cookie, io_request* request, status_t status,
-	bool partialTransfer, generic_size_t transferEndOffset)
+	bool partialTransfer, generic_size_t bytesTransferred)
 {
 	iterative_io_cookie* cookie = (iterative_io_cookie*)_cookie;
 
 	if (cookie->finished != NULL) {
 		cookie->finished(cookie->cookie, request, status, partialTransfer,
-			transferEndOffset);
+			bytesTransferred);
 	}
 
 	put_fd(cookie->descriptor);
 
 	if (cookie->next_finished_callback != NULL) {
 		cookie->next_finished_callback(cookie->next_finished_cookie, request,
-			status, partialTransfer, transferEndOffset);
+			status, partialTransfer, bytesTransferred);
 	}
 
 	delete cookie;
-
-	return B_OK;
 }
 
 

src/system/kernel/vm/VMAnonymousCache.cpp

@@ -429,7 +429,6 @@ public:
 		}
 
 		fNextCallback->IOFinished(status, partialTransfer, bytesTransferred);
-
 		delete this;
 	}
 

src/system/kernel/vm/vm_page.cpp

@@ -1966,6 +1966,7 @@ public:
 
 	virtual void IOFinished(status_t status, bool partialTransfer,
 		generic_size_t bytesTransferred);
+
 private:
 	PageWriterRun*		fRun;
 	struct VMCache*		fCache;