From 5554f0b829d0622e0608963b9bc7340fcd064ed6 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 30 Mar 2026 18:01:45 -0400 Subject: [PATCH] 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 --- src/system/kernel/cache/file_cache.cpp | 19 +++++++++---------- src/system/kernel/vm/vm.cpp | 2 ++ src/system/kernel/vm/vm_page.cpp | 8 +------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/system/kernel/cache/file_cache.cpp b/src/system/kernel/cache/file_cache.cpp index 2999c7895f..c5ec475944 100644 --- a/src/system/kernel/cache/file_cache.cpp +++ b/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; diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 7a526e2e89..d8a4e995b7 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/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 diff --git a/src/system/kernel/vm/vm_page.cpp b/src/system/kernel/vm/vm_page.cpp index 42b99e58ab..91c9b744a4 100644 --- a/src/system/kernel/vm/vm_page.cpp +++ b/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;