kernel/vm: Fix a TODO about page I/O leaving pages in ACCESS state.

It turns out PAGE_ACCESS assertions could be tripped in other cases,
e.g. when resizing a cache while pages were being read in. So,
properly unset ACCESS (and just leave 'busy' set) in the places
where pages are read in, and re-enable the check.

Change-Id: I22ac5c8cb432c87613bb85e82703a0c1aeb55d86
Reviewed-on: https://review.haiku-os.org/c/haiku/+/10627
Reviewed-by: waddlesplash <[email protected]>

src/system/kernel/cache/file_cache.cpp

@@ -92,9 +92,6 @@ private:
 			off_t				fOffset;
 			uint32				fVecCount;
 			generic_size_t		fSize;
-#if DEBUG_PAGE_ACCESS
-			thread_id			fAllocatingThread;
-#endif
 };
 
 typedef status_t (*cache_func)(file_cache_ref* ref, void* cookie, off_t offset,
@@ -164,16 +161,13 @@ PrecacheIO::Prepare(vm_page_reservation* reservation)
 		page->busy_io = true;
 
 		fCache->InsertPage(page, fOffset + pos);
+		DEBUG_PAGE_ACCESS_END(page);
 
 		add_to_iovec(fVecs, fVecCount, fPageCount,
 			page->physical_page_number * B_PAGE_SIZE, B_PAGE_SIZE);
 		fPages[i++] = page;
 	}
 
-#if DEBUG_PAGE_ACCESS
-	fAllocatingThread = find_thread(NULL);
-#endif
-
 	return B_OK;
 }
 
@@ -203,6 +197,8 @@ PrecacheIO::IOFinished(status_t status, bool partialTransfer,
 		bytesTransferred = fCache->virtual_end - fOffset;
 
 	for (uint32 i = 0; i < pagesTransferred; i++) {
+		DEBUG_PAGE_ACCESS_START(fPages[i]);
+
 		if (i == pagesTransferred - 1
 			&& (bytesTransferred % B_PAGE_SIZE) != 0) {
 			// clear partial page
@@ -213,8 +209,6 @@ PrecacheIO::IOFinished(status_t status, bool partialTransfer,
 				0, B_PAGE_SIZE - bytesTouched);
 		}
 
-		DEBUG_PAGE_ACCESS_TRANSFER(fPages[i], fAllocatingThread);
-
 		if (!fPages[i]->busy_io) {
 			// The busy_io flag was cleared. Let the cache handle the rest.
 			fCache->FreeRemovedPage(fPages[i]);
@@ -228,7 +222,7 @@ PrecacheIO::IOFinished(status_t status, bool partialTransfer,
 
 	// Free pages after failed I/O
 	for (uint32 i = pagesTransferred; i < fPageCount; i++) {
-		DEBUG_PAGE_ACCESS_TRANSFER(fPages[i], fAllocatingThread);
+		DEBUG_PAGE_ACCESS_START(fPages[i]);
 		if (!fPages[i]->busy_io) {
 			fCache->FreeRemovedPage(fPages[i]);
 			continue;
@@ -413,6 +407,7 @@ read_into_cache(file_cache_ref* ref, void* cookie, off_t offset,
 		page->busy_io = true;
 
 		cache->InsertPage(page, offset + pos);
+		DEBUG_PAGE_ACCESS_END(page);
 
 		add_to_iovec(vecs, vecCount, MAX_IO_VECS,
 			page->physical_page_number * B_PAGE_SIZE, B_PAGE_SIZE);
@@ -434,6 +429,7 @@ read_into_cache(file_cache_ref* ref, void* cookie, off_t offset,
 		cache->Lock();
 
 		for (int32 i = 0; i < pageIndex; i++) {
+			DEBUG_PAGE_ACCESS_START(pages[i]);
 			if (!pages[i]->busy_io) {
 				cache->FreeRemovedPage(pages[i]);
 				continue;
@@ -468,6 +464,7 @@ read_into_cache(file_cache_ref* ref, void* cookie, off_t offset,
 
 	// make the pages accessible in the cache
 	for (int32 i = pageIndex; i-- > 0;) {
+		DEBUG_PAGE_ACCESS_START(pages[i]);
 		if (!pages[i]->busy_io) {
 			cache->FreeRemovedPage(pages[i]);
 			continue;
@@ -552,6 +549,7 @@ write_to_cache(file_cache_ref* ref, void* cookie, off_t offset,
 		page->modified = !writeThrough;
 
 		ref->cache->InsertPage(page, offset + pos);
+		DEBUG_PAGE_ACCESS_END(page);
 
 		add_to_iovec(vecs, vecCount, MAX_IO_VECS,
 			page->physical_page_number * B_PAGE_SIZE, B_PAGE_SIZE);
@@ -647,6 +645,7 @@ write_to_cache(file_cache_ref* ref, void* cookie, off_t offset,
 
 	// make the pages accessible in the cache
 	for (int32 i = pageIndex; i-- > 0;) {
+		DEBUG_PAGE_ACCESS_START(pages[i]);
 		if (!pages[i]->busy_io) {
 			ref->cache->FreeRemovedPage(pages[i]);
 			continue;

src/system/kernel/vm/vm.cpp

@@ -4361,6 +4361,7 @@ fault_get_page(PageFaultContext& context)
 			page = vm_page_allocate_page(&context.reservation,
 				PAGE_STATE_ACTIVE | VM_PAGE_ALLOC_BUSY);
 			cache->InsertPage(page, context.cacheOffset);
+			DEBUG_PAGE_ACCESS_END(page);
 
 			// We need to unlock all caches and the address space while reading
 			// the page in. Keep a reference to the cache around.
@@ -4376,6 +4377,7 @@ fault_get_page(PageFaultContext& context)
 				B_PHYSICAL_IO_REQUEST, &bytesRead);
 
 			cache->Lock();
+			DEBUG_PAGE_ACCESS_START(page);
 
 			if (status < B_OK) {
 				// on error remove and free the page

src/system/kernel/vm/vm_page.cpp

@@ -4123,13 +4123,7 @@ vm_page_requeue(struct vm_page *page, bool tail)
 {
 	PAGE_ASSERT(page, page->Cache() != NULL);
 	page->Cache()->AssertLocked();
-	// DEBUG_PAGE_ACCESS_CHECK(page);
-		// TODO: This assertion cannot be satisfied by idle_scan_active_pages()
-		// when it requeues busy pages. The reason is that vm_soft_fault()
-		// (respectively fault_get_page()) and the file cache keep newly
-		// allocated pages accessed while they are reading them from disk. It
-		// would probably be better to change that code and reenable this
-		// check.
+	DEBUG_PAGE_ACCESS_CHECK(page);
 
 	VMPageQueue *queue = NULL;