* pages_io() now handles it gracefully in case the fileVec array is too

small to hold the information for the requested I/O size.
* get_file_map() returned B_BUFFER_OVERFLOW already in case the array
  was exactly as large as needed.
* read_chunk_into_cache() and write_chunk_to_cache() will no longer
  override their local "size" variable.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20509 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/system/kernel/cache/file_cache.cpp

@@ -288,14 +288,14 @@ get_file_map(file_cache_ref *ref, off_t offset, size_t size,
 		vecs[index] = fileExtent->disk;
 		index++;
 
+		if (size <= fileExtent->disk.length)
+			break;
+
 		if (index >= maxVecs) {
 			*_count = index;
 			return B_BUFFER_OVERFLOW;
 		}
 
-		if (size <= fileExtent->disk.length)
-			break;
-
 		size -= fileExtent->disk.length;
 	}
 
@@ -312,8 +312,8 @@ static status_t
 pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 	size_t *_numBytes, bool doWrite)
 {
-	TRACE(("pages_io: ref = %p, offset = %Ld, size = %lu, vecCount = %lu, %s\n", ref, offset,
-		*_numBytes, count, doWrite ? "write" : "read"));
+	TRACE(("pages_io: ref = %p, offset = %Ld, size = %lu, vecCount = %lu, %s\n",
+		ref, offset, *_numBytes, count, doWrite ? "write" : "read"));
 
 	// translate the iovecs into direct device accesses
 	file_io_vec fileVecs[MAX_FILE_IO_VECS];
@@ -322,16 +322,17 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 
 	status_t status = get_file_map(ref, offset, numBytes, fileVecs,
 		&fileVecCount);
-	if (status < B_OK) {
-		TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed\n", offset,
-			numBytes));
+	if (status < B_OK && status != B_BUFFER_OVERFLOW) {
+		TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed: %s\n",
+			offset, numBytes, strerror(status)));
 		return status;
 	}
 
-	// TODO: handle array overflow gracefully!
+	bool bufferOverflow = status == B_BUFFER_OVERFLOW;
 
 #ifdef TRACE_FILE_CACHE
-	dprintf("got %lu file vecs for %Ld:%lu:\n", fileVecCount, offset, numBytes);
+	dprintf("got %lu file vecs for %Ld:%lu%s:\n", fileVecCount, offset,
+		numBytes, bufferOverflow ? " (array too small)" : "");
 	for (size_t i = 0; i < fileVecCount; i++) {
 		dprintf("  [%lu] offset = %Ld, size = %Ld\n",
 			i, fileVecs[i].offset, fileVecs[i].length);
@@ -357,8 +358,8 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 		if (size > numBytes)
 			size = numBytes;
 
-		status = vfs_read_pages(ref->device, ref->cookie, fileVecs[0].offset, vecs,
-			count, &size, false);
+		status = vfs_read_pages(ref->device, ref->cookie, fileVecs[0].offset,
+			vecs, count, &size, false);
 		if (status < B_OK)
 			return status;
 
@@ -406,75 +407,100 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 	size_t vecOffset = size;
 	size_t bytesLeft = numBytes - size;
 
-	for (; fileVecIndex < fileVecCount; fileVecIndex++) {
-		file_io_vec &fileVec = fileVecs[fileVecIndex];
-		off_t fileOffset = fileVec.offset;
-		off_t fileLeft = fileVec.length;
+	while (true) {
+		for (; fileVecIndex < fileVecCount; fileVecIndex++) {
+			file_io_vec &fileVec = fileVecs[fileVecIndex];
+			off_t fileOffset = fileVec.offset;
+			off_t fileLeft = min_c(fileVec.length, bytesLeft);
 
-		fileLeft = min_c(fileVec.length, bytesLeft);
+			TRACE(("FILE VEC [%lu] length %Ld\n", fileVecIndex, fileLeft));
 
-		TRACE(("FILE VEC [%lu] length %Ld\n", fileVecIndex, fileLeft));
+			// process the complete fileVec
+			while (fileLeft > 0) {
+				iovec tempVecs[MAX_TEMP_IO_VECS];
+				uint32 tempCount = 0;
 
-		// process the complete fileVec
-		while (fileLeft > 0) {
-			iovec tempVecs[MAX_TEMP_IO_VECS];
-			uint32 tempCount = 0;
+				// size tracks how much of what is left of the current fileVec
+				// (fileLeft) has been assigned to tempVecs 
+				size = 0;
 
-			// size tracks how much of what is left of the current fileVec
-			// (fileLeft) has been assigned to tempVecs 
-			size = 0;
+				// assign what is left of the current fileVec to the tempVecs
+				for (size = 0; size < fileLeft && i < count
+						&& tempCount < MAX_TEMP_IO_VECS;) {
+					// try to satisfy one iovec per iteration (or as much as
+					// possible)
 
-			// assign what is left of the current fileVec to the tempVecs
-			while (size < fileLeft && i < count
-					&& tempCount < MAX_TEMP_IO_VECS) {
-				// try to satisfy one iovec per iteration (or as much as
-				// possible)
-				TRACE(("fill vec %ld, offset = %lu, size = %lu\n",
-					i, vecOffset, size));
+					// bytes left of the current iovec
+					size_t vecLeft = vecs[i].iov_len - vecOffset;
+					if (vecLeft == 0) {
+						vecOffset = 0;
+						i++;
+						continue;
+					}
 
-				// bytes left of the current iovec
-				size_t vecLeft = vecs[i].iov_len - vecOffset;
-				if (vecLeft == 0) {
-					i++;
-					vecOffset = 0;
-					continue;
+					TRACE(("fill vec %ld, offset = %lu, size = %lu\n",
+						i, vecOffset, size));
+
+					// actually available bytes
+					size_t tempVecSize = min_c(vecLeft, fileLeft - size);
+
+					tempVecs[tempCount].iov_base
+						= (void *)((addr_t)vecs[i].iov_base + vecOffset);
+					tempVecs[tempCount].iov_len = tempVecSize;
+					tempCount++;
+
+					size += tempVecSize;
+					vecOffset += tempVecSize;
 				}
 
-				// actually available bytes
-				size_t tempVecSize = min_c(vecLeft, fileLeft - size);
+				size_t bytes = size;
+				if (doWrite) {
+					status = vfs_write_pages(ref->device, ref->cookie,
+						fileOffset, tempVecs, tempCount, &bytes, false);
+				} else {
+					status = vfs_read_pages(ref->device, ref->cookie,
+						fileOffset, tempVecs, tempCount, &bytes, false);
+				}
+				if (status < B_OK)
+					return status;
 
-				tempVecs[tempCount].iov_base
-					= (void *)((addr_t)vecs[i].iov_base + vecOffset);
-				tempVecs[tempCount].iov_len = tempVecSize;
-				tempCount++;
+				totalSize += bytes;
+				bytesLeft -= size;
+				fileOffset += size;
+				fileLeft -= size;
+				//dprintf("-> file left = %Lu\n", fileLeft);
 
-				size += tempVecSize;
-				vecOffset += tempVecSize;
-			}
-
-			size_t bytes = size;
-			if (doWrite) {
-				status = vfs_write_pages(ref->device, ref->cookie, fileOffset,
-					tempVecs, tempCount, &bytes, false);
-			} else {
-				status = vfs_read_pages(ref->device, ref->cookie, fileOffset,
-					tempVecs, tempCount, &bytes, false);
-			}
-			if (status < B_OK)
-				return status;
-
-			totalSize += bytes;
-			bytesLeft -= size;
-			fileOffset += size;
-			fileLeft -= size;
-			//dprintf("-> file left = %Lu\n", fileLeft);
-
-			if (size != bytes || i >= count) {
-				// there are no more bytes or iovecs, let's bail out
-				*_numBytes = totalSize;
-				return B_OK;
+				if (size != bytes || i >= count) {
+					// there are no more bytes or iovecs, let's bail out
+					*_numBytes = totalSize;
+					return B_OK;
+				}
 			}
 		}
+
+		if (bufferOverflow) {
+			status = get_file_map(ref, offset + totalSize, bytesLeft, fileVecs,
+				&fileVecCount);
+			if (status < B_OK && status != B_BUFFER_OVERFLOW) {
+				TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed: %s\n",
+					offset, numBytes, strerror(status)));
+				return status;
+			}
+
+			bufferOverflow = status == B_BUFFER_OVERFLOW;
+			fileVecIndex = 0;
+
+#ifdef TRACE_FILE_CACHE
+			dprintf("got %lu file vecs for %Ld:%lu%s:\n", fileVecCount,
+				offset + totalSize, numBytes,
+				bufferOverflow ? " (array too small)" : "");
+			for (size_t i = 0; i < fileVecCount; i++) {
+				dprintf("  [%lu] offset = %Ld, size = %Ld\n",
+					i, fileVecs[i].offset, fileVecs[i].length);
+			}
+#endif
+		} else
+			break;
 	}
 
 	*_numBytes = totalSize;
@@ -488,7 +514,7 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 	sure that it matches that criterion.
 */
 static inline status_t
-read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
+read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t numBytes,
 	int32 pageOffset, addr_t buffer, size_t bufferSize)
 {
 	TRACE(("read_chunk(offset = %Ld, size = %lu, pageOffset = %ld, buffer = %#lx, bufferSize = %lu\n",
@@ -503,7 +529,7 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
 	int32 pageIndex = 0;
 
 	// allocate pages for the cache and mark them busy
-	for (size_t pos = 0; pos < size; pos += B_PAGE_SIZE) {
+	for (size_t pos = 0; pos < numBytes; pos += B_PAGE_SIZE) {
 		vm_page *page = pages[pageIndex++] = vm_page_allocate_page(PAGE_STATE_FREE);
 		if (page == NULL)
 			panic("no more pages!");
@@ -524,7 +550,7 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
 	mutex_unlock(&cache->lock);
 
 	// read file into reserved pages
-	status_t status = pages_io(ref, offset, vecs, vecCount, &size, false);
+	status_t status = pages_io(ref, offset, vecs, vecCount, &numBytes, false);
 	if (status < B_OK) {
 		// reading failed, free allocated pages
 
@@ -635,7 +661,7 @@ read_into_cache(file_cache_ref *ref, off_t offset, size_t size, addr_t buffer, s
 /**	Like read_chunk_into_cache() but writes data into the cache */
 
 static inline status_t
-write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t size,
+write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t numBytes,
 	int32 pageOffset, addr_t buffer, size_t bufferSize)
 {
 	iovec vecs[MAX_IO_VECS];
@@ -648,7 +674,7 @@ write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t size,
 	bool writeThrough = false;
 
 	// allocate pages for the cache and mark them busy
-	for (size_t pos = 0; pos < size; pos += B_PAGE_SIZE) {
+	for (size_t pos = 0; pos < numBytes; pos += B_PAGE_SIZE) {
 		// ToDo: if space is becoming tight, and this cache is already grown
 		//	big - shouldn't we better steal the pages directly in that case?
 		//	(a working set like approach for the file cache)
@@ -696,7 +722,7 @@ write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t size,
 			iovec readVec = { (void *)last, B_PAGE_SIZE };
 			size_t bytesRead = B_PAGE_SIZE;
 
-			status = pages_io(ref, offset + size - B_PAGE_SIZE, &readVec, 1,
+			status = pages_io(ref, offset + numBytes - B_PAGE_SIZE, &readVec, 1,
 				&bytesRead, false);
 			// ToDo: handle errors for real!
 			if (status < B_OK)
@@ -721,7 +747,7 @@ write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t size,
 
 	if (writeThrough) {
 		// write cached pages back to the file if we were asked to do that
-		status_t status = pages_io(ref, offset, vecs, vecCount, &size, true);
+		status_t status = pages_io(ref, offset, vecs, vecCount, &numBytes, true);
 		if (status < B_OK) {
 			// ToDo: remove allocated pages, ...?
 			panic("file_cache: remove allocated pages! write pages failed: %s\n",

src/tests/system/kernel/cache/pages_io_test.cpp

@@ -10,6 +10,7 @@
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/uio.h>
 
 #define TRACE_FILE_CACHE
@@ -22,7 +23,7 @@
 
 // maximum number of iovecs per request
 #define MAX_IO_VECS			64	// 256 kB
-#define MAX_FILE_IO_VECS	32
+#define MAX_FILE_IO_VECS	4
 #define MAX_TEMP_IO_VECS	8
 
 #define CACHED_FILE_EXTENTS	2
@@ -61,7 +62,9 @@ struct file_cache_ref {
 };
 
 
-file_io_vec gFileVecs[MAX_FILE_IO_VECS];
+const uint32 kMaxFileVecs = 1024;
+
+file_io_vec gFileVecs[kMaxFileVecs];
 size_t gFileVecCount;
 off_t gFileSize;
 
@@ -202,7 +205,7 @@ set_file_map(int32 base, int32 length, ...)
 	va_list args;
 	va_start(args, length);
 
-	while (gFileVecCount < MAX_FILE_IO_VECS) {
+	while (gFileVecCount < kMaxFileVecs) {
 		off_t offset = va_arg(args, int32);
 		if (offset < 0)
 			break;
@@ -252,6 +255,9 @@ vfs_get_file_map(void *vnode, off_t offset, size_t size, file_io_vec *vecs,
 	size_t max = *_count;
 	uint32 index = 0;
 
+	printf("vfs_get_file_map(offset = %Ld, size = %lu, count = %lu)\n",
+		offset, size, *_count);
+
 	while (true) {
 		status_t status = find_map_base(offset, diskOffset, diskLength, fileOffset);
 		//status_t status = inode->FindBlockRun(offset, run, fileOffset);
@@ -415,14 +421,14 @@ get_file_map(file_cache_ref *ref, off_t offset, size_t size,
 		vecs[index] = fileExtent->disk;
 		index++;
 
+		if (size <= fileExtent->disk.length)
+			break;
+
 		if (index >= maxVecs) {
 			*_count = index;
 			return B_BUFFER_OVERFLOW;
 		}
 
-		if (size <= fileExtent->disk.length)
-			break;
-
 		size -= fileExtent->disk.length;
 	}
 
@@ -449,16 +455,17 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 
 	status_t status = get_file_map(ref, offset, numBytes, fileVecs,
 		&fileVecCount);
-	if (status < B_OK) {
-		TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed\n", offset,
-			numBytes));
+	if (status < B_OK && status != B_BUFFER_OVERFLOW) {
+		TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed: %s\n", offset,
+			numBytes, strerror(status)));
 		return status;
 	}
 
-	// TODO: handle array overflow gracefully!
+	bool bufferOverflow = status == B_BUFFER_OVERFLOW;
 
 #ifdef TRACE_FILE_CACHE
-	dprintf("got %lu file vecs for %Ld:%lu:\n", fileVecCount, offset, numBytes);
+	dprintf("got %lu file vecs for %Ld:%lu%s:\n", fileVecCount, offset, numBytes,
+		bufferOverflow ? " (array too small)" : "");
 	for (size_t i = 0; i < fileVecCount; i++) {
 		dprintf("  [%lu] offset = %Ld, size = %Ld\n",
 			i, fileVecs[i].offset, fileVecs[i].length);
@@ -533,75 +540,100 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
 	size_t vecOffset = size;
 	size_t bytesLeft = numBytes - size;
 
-	for (; fileVecIndex < fileVecCount; fileVecIndex++) {
-		file_io_vec &fileVec = fileVecs[fileVecIndex];
-		off_t fileOffset = fileVec.offset;
-		off_t fileLeft = fileVec.length;
+	while (true) {
+		for (; fileVecIndex < fileVecCount; fileVecIndex++) {
+			file_io_vec &fileVec = fileVecs[fileVecIndex];
+			off_t fileOffset = fileVec.offset;
+			off_t fileLeft = min_c(fileVec.length, bytesLeft);
 
-		fileLeft = min_c(fileVec.length, bytesLeft);
+			TRACE(("FILE VEC [%lu] length %Ld\n", fileVecIndex, fileLeft));
 
-		TRACE(("FILE VEC [%lu] length %Ld\n", fileVecIndex, fileLeft));
+			// process the complete fileVec
+			while (fileLeft > 0) {
+				iovec tempVecs[MAX_TEMP_IO_VECS];
+				uint32 tempCount = 0;
 
-		// process the complete fileVec
-		while (fileLeft > 0) {
-			iovec tempVecs[MAX_TEMP_IO_VECS];
-			uint32 tempCount = 0;
+				// size tracks how much of what is left of the current fileVec
+				// (fileLeft) has been assigned to tempVecs 
+				size = 0;
 
-			// size tracks how much of what is left of the current fileVec
-			// (fileLeft) has been assigned to tempVecs 
-			size = 0;
+				// assign what is left of the current fileVec to the tempVecs
+				for (size = 0; size < fileLeft && i < count
+						&& tempCount < MAX_TEMP_IO_VECS;) {
+					// try to satisfy one iovec per iteration (or as much as
+					// possible)
 
-			// assign what is left of the current fileVec to the tempVecs
-			for (size = 0; size < fileLeft && i < count
-					&& tempCount < MAX_TEMP_IO_VECS;) {
-				// try to satisfy one iovec per iteration (or as much as
-				// possible)
-				TRACE(("fill vec %ld, offset = %lu, size = %lu\n",
-					i, vecOffset, size));
+					// bytes left of the current iovec
+					size_t vecLeft = vecs[i].iov_len - vecOffset;
+					if (vecLeft == 0) {
+						vecOffset = 0;
+						i++;
+						continue;
+					}
 
-				// bytes left of the current iovec
-				size_t vecLeft = vecs[i].iov_len - vecOffset;
-				if (vecLeft == 0) {
-					vecOffset = 0;
-					i++;
-					continue;
+					TRACE(("fill vec %ld, offset = %lu, size = %lu\n",
+						i, vecOffset, size));
+
+					// actually available bytes
+					size_t tempVecSize = min_c(vecLeft, fileLeft - size);
+
+					tempVecs[tempCount].iov_base
+						= (void *)((addr_t)vecs[i].iov_base + vecOffset);
+					tempVecs[tempCount].iov_len = tempVecSize;
+					tempCount++;
+
+					size += tempVecSize;
+					vecOffset += tempVecSize;
 				}
 
-				// actually available bytes
-				size_t tempVecSize = min_c(vecLeft, fileLeft - size);
+				size_t bytes = size;
+				if (doWrite) {
+					status = vfs_write_pages(ref->device, ref->cookie,
+						fileOffset, tempVecs, tempCount, &bytes, false);
+				} else {
+					status = vfs_read_pages(ref->device, ref->cookie,
+						fileOffset, tempVecs, tempCount, &bytes, false);
+				}
+				if (status < B_OK)
+					return status;
 
-				tempVecs[tempCount].iov_base
-					= (void *)((addr_t)vecs[i].iov_base + vecOffset);
-				tempVecs[tempCount].iov_len = tempVecSize;
-				tempCount++;
+				totalSize += bytes;
+				bytesLeft -= size;
+				fileOffset += size;
+				fileLeft -= size;
+				//dprintf("-> file left = %Lu\n", fileLeft);
 
-				size += tempVecSize;
-				vecOffset += tempVecSize;
-			}
-
-			size_t bytes = size;
-			if (doWrite) {
-				status = vfs_write_pages(ref->device, ref->cookie, fileOffset,
-					tempVecs, tempCount, &bytes, false);
-			} else {
-				status = vfs_read_pages(ref->device, ref->cookie, fileOffset,
-					tempVecs, tempCount, &bytes, false);
-			}
-			if (status < B_OK)
-				return status;
-
-			totalSize += bytes;
-			bytesLeft -= size;
-			fileOffset += size;
-			fileLeft -= size;
-			//dprintf("-> file left = %Lu\n", fileLeft);
-
-			if (size != bytes || i >= count) {
-				// there are no more bytes or iovecs, let's bail out
-				*_numBytes = totalSize;
-				return B_OK;
+				if (size != bytes || i >= count) {
+					// there are no more bytes or iovecs, let's bail out
+					*_numBytes = totalSize;
+					return B_OK;
+				}
 			}
 		}
+
+		if (bufferOverflow) {
+			status = get_file_map(ref, offset + totalSize, bytesLeft, fileVecs,
+				&fileVecCount);
+			if (status < B_OK && status != B_BUFFER_OVERFLOW) {
+				TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed: %s\n",
+					offset, numBytes, strerror(status)));
+				return status;
+			}
+
+			bufferOverflow = status == B_BUFFER_OVERFLOW;
+			fileVecIndex = 0;
+
+#ifdef TRACE_FILE_CACHE
+			dprintf("got %lu file vecs for %Ld:%lu%s:\n", fileVecCount,
+				offset + totalSize, numBytes,
+				bufferOverflow ? " (array too small)" : "");
+			for (size_t i = 0; i < fileVecCount; i++) {
+				dprintf("  [%lu] offset = %Ld, size = %Ld\n",
+					i, fileVecs[i].offset, fileVecs[i].length);
+			}
+#endif
+		} else
+			break;
 	}
 
 	*_numBytes = totalSize;
@@ -621,10 +653,14 @@ main(int argc, char **argv)
 	size_t numBytes = 10000;
 	off_t offset = 4999;
 
-	set_vecs(vecs, &count, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 4096, 8192, 16384, 4096, 4096, -1);
-	set_file_map(0, 2000, 5000, 3000, 10000, 800, 11000, 20, 12000, 30, 13000, 70, 14000, 100, 15000, 30000, -1);
+	set_vecs(vecs, &count, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+		16, 4096, 8192, 16384, 4096, 4096, -1);
+	set_file_map(0, 2000, 5000, 3000, 10000, 800, 11000, 20, 12000, 30,
+		13000, 70, 14000, 100, 15000, 900, 20000, 30000, -1);
 
-	pages_io(&ref, offset, vecs, count, &numBytes, false);
+	status_t status = pages_io(&ref, offset, vecs, count, &numBytes, false);
+	if (status < B_OK)
+		fprintf(stderr, "pages_io() returned: %s\n", strerror(status));
 
 	return 0;
 }