From de0a0420a88cd4ac66ae9748d88ff8ea9caedfc6 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 14 Apr 2026 15:21:29 -0400 Subject: [PATCH] kernel/vm: Implement modified page quotas and wait for them in the file cache. Otherwise, if we're marking pages modified faster than we can write them out, the number of modified pages will just grow unboundedly. This can lead (e.g.) to `sync` taking multiple minutes after copying a lot of data to a slow disk. So, instead, we now have a quota of no more than 3 seconds for all pages to be written back. Also drop a TODO comment from the page_writer thread. Since we only start writing pages if there's at least 256 to be written, or if someone wakes us up deliberately (which the page daemon does, if it schedules pages to be written out), we shouldn't need to wait shorter. This should fix #5777 and related tickets. Change-Id: I4d419d149ea780677b462f5fa46cfe4d65044b2c Reviewed-on: https://review.haiku-os.org/c/haiku/+/10811 Tested-by: Commit checker robot Reviewed-by: waddlesplash --- src/system/kernel/cache/file_cache.cpp | 38 +++++++++++-- src/system/kernel/vm/ModifiedPageQueue.h | 10 ++++ src/system/kernel/vm/vm_page.cpp | 7 +++ src/system/kernel/vm/vm_page_writer.cpp | 68 ++++++++++++++++++++---- 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/src/system/kernel/cache/file_cache.cpp b/src/system/kernel/cache/file_cache.cpp index d846ffedd8..8b3544194a 100644 --- a/src/system/kernel/cache/file_cache.cpp +++ b/src/system/kernel/cache/file_cache.cpp @@ -27,6 +27,7 @@ #include #include "IORequest.h" +#include "../vm/ModifiedPageQueue.h" //#define TRACE_FILE_CACHE @@ -792,6 +793,10 @@ do_cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer, AutoLocker locker(cache); + ModifiedPageQueue* modifiedQueue = NULL; + if (doWrite) + modifiedQueue = vm_page_get_modified_queue(); + size_t bytesLeft = size, lastLeft = size; int32 lastPageOffset = pageOffset; addr_t lastBuffer = buffer; @@ -801,9 +806,10 @@ do_cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer, cache_func function = NULL; while (bytesLeft > 0) { - // Periodically reevaluate the low memory situation and select the - // read/write hook accordingly - if (pagesProcessed % 32 == 0) { + // periodic rechecks + if ((pagesProcessed % MAX_IO_VECS) == 0) { + // Re-evaluate the low memory situation and select the + // read/write hook accordingly if (size >= BYPASS_IO_SIZE && low_resource_state(B_KERNEL_RESOURCE_PAGES) != B_NO_LOW_RESOURCE) { @@ -812,6 +818,32 @@ do_cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer, function = doWrite ? write_to_file : read_from_file; } else function = doWrite ? write_to_cache : read_into_cache; + + if (doWrite) { + // Make sure there's enough space in the modified queue for the + // next set of pages. The situation can change while we have locks + // released, but since the modified quota is "best effort" anyway + // as mapped pages may be modified at any time, that's acceptable. + page_num_t toModified = 0; + for (size_t i = 0; i < bytesLeft && i < (B_PAGE_SIZE * MAX_IO_VECS); + i += B_PAGE_SIZE) { + vm_page* page = cache->LookupPage(offset + i); + if (page == NULL || page->State() != PAGE_STATE_MODIFIED) + toModified++; + } + + locker.Unlock(); + status_t status = modifiedQueue->WaitIfOverQuota(toModified, 0, B_CAN_INTERRUPT); + locker.Lock(); + if (status != B_OK) { + if (bytesLeft == size) + return status; + + // don't return the error, but treat this as a partial write + *_size = size - bytesLeft; + return B_OK; + } + } } // check if this page is already in memory diff --git a/src/system/kernel/vm/ModifiedPageQueue.h b/src/system/kernel/vm/ModifiedPageQueue.h index 019f47343d..518958d768 100644 --- a/src/system/kernel/vm/ModifiedPageQueue.h +++ b/src/system/kernel/vm/ModifiedPageQueue.h @@ -16,6 +16,10 @@ public: status_t StartWriter(); void NotifyWriter() { fPageWriterCondition.WakeUp(); } + bool IsOverQuota(page_num_t additionalPages = 0); + status_t WaitIfOverQuota(page_num_t additionalPages, + bigtime_t timeout, uint32 flags); + private: static status_t _WriterThreadEntry(void* _this); status_t _PageWriter(); @@ -23,7 +27,13 @@ private: private: thread_id fWriterThread; BinarySemaphore fPageWriterCondition; + ConditionVariable fUnderQuotaCondition; + + bigtime_t fLastAveragePageWriteDuration; }; +ModifiedPageQueue* vm_page_get_modified_queue(); + + #endif // MODIFIED_PAGE_QUEUE_H diff --git a/src/system/kernel/vm/vm_page.cpp b/src/system/kernel/vm/vm_page.cpp index b4e3dfe63a..34bdf6727a 100644 --- a/src/system/kernel/vm/vm_page.cpp +++ b/src/system/kernel/vm/vm_page.cpp @@ -1460,6 +1460,13 @@ unreserve_pages(uint32 count) } +ModifiedPageQueue* +vm_page_get_modified_queue() +{ + return &sModifiedPageQueue; +} + + static VMPageQueue* page_queue_for(vm_page* page, uint8 state) { diff --git a/src/system/kernel/vm/vm_page_writer.cpp b/src/system/kernel/vm/vm_page_writer.cpp index 732b84e213..b7c8e8d375 100644 --- a/src/system/kernel/vm/vm_page_writer.cpp +++ b/src/system/kernel/vm/vm_page_writer.cpp @@ -46,6 +46,9 @@ // the maximum I/O priority shall be reached when this many pages need to // be written +#define PAGES_FLUSH_DURATION_QUOTA (3 * 1000 * 1000) + // target maximum time needed to write out all modified pages + #if PAGE_WRITER_TRACING @@ -537,7 +540,6 @@ ModifiedPageQueue::_PageWriter() uint32 writtenPages = 0; bigtime_t lastWrittenTime = 0; bigtime_t pageCollectionTime = 0; - bigtime_t pageWritingTime = 0; #endif PageWriterRun run; @@ -549,10 +551,9 @@ ModifiedPageQueue::_PageWriter() page_num_t pagesSinceLastSuccessfulWrite = 0; while (true) { -// TODO: Maybe wait shorter when memory is low! if (queue.Count() < kNumPages) { - fPageWriterCondition.Wait(3000000, true); - // all 3 seconds when no one triggers us + fPageWriterCondition.Wait(PAGES_FLUSH_DURATION_QUOTA, true); + // wait the full amount when no one triggers us } page_num_t modifiedPages = queue.Count(); @@ -696,17 +697,15 @@ ModifiedPageQueue::_PageWriter() continue; // write pages to disk and do all the cleanup -#ifdef TRACE_VM_PAGE_WRITER - pageWritingTime -= system_time(); -#endif + bigtime_t runStart = system_time(); uint32 failedPages = run.Go(); -#ifdef TRACE_VM_PAGE_WRITER - pageWritingTime += system_time(); +#ifdef TRACE_VM_PAGE_WRITER // debug output only... writtenPages += numPages; if (writtenPages >= 1024) { bigtime_t now = system_time(); + bigtime pageWritingTime = now - runStart; TRACE(("page writer: wrote 1024 pages (total: %" B_PRIu64 " ms, " "collect: %" B_PRIu64 " ms, write: %" B_PRIu64 " ms)\n", (now - lastWrittenTime) / 1000, @@ -723,6 +722,12 @@ ModifiedPageQueue::_PageWriter() pagesSinceLastSuccessfulWrite += modifiedPages - maxPagesToSee; else pagesSinceLastSuccessfulWrite = 0; + + if (failedPages == 0 && numPages >= 8) + fLastAveragePageWriteDuration = (system_time() - runStart) / numPages; + + if (!IsOverQuota()) + fUnderQuotaCondition.NotifyAll(); } return B_OK; @@ -736,7 +741,6 @@ ModifiedPageQueue::_WriterThreadEntry(void* _this) } - // #pragma mark - private kernel API @@ -913,6 +917,7 @@ status_t ModifiedPageQueue::StartWriter() { fPageWriterCondition.Init("page writer"); + fUnderQuotaCondition.Init(this, "ModifiedPageQueue"); fWriterThread = spawn_kernel_thread(&_WriterThreadEntry, "page writer", B_NORMAL_PRIORITY + 1, this); @@ -921,3 +926,46 @@ ModifiedPageQueue::StartWriter() return resume_thread(fWriterThread); } + + +bool +ModifiedPageQueue::IsOverQuota(page_num_t additionalPages) +{ + InterruptsSpinLocker _(fLock); + + if (fLastAveragePageWriteDuration == 0) + return false; + + const page_num_t quota = PAGES_FLUSH_DURATION_QUOTA / fLastAveragePageWriteDuration; + return (quota < (fCount + additionalPages)); +} + + +status_t +ModifiedPageQueue::WaitIfOverQuota(page_num_t additionalPages, + bigtime_t timeout, uint32 flags) +{ + if ((flags & B_RELATIVE_TIMEOUT) != 0) { + // Convert to an absolute timeout, so that we can wait multiple times if needed. + timeout += system_time(); + flags = (flags & ~B_RELATIVE_TIMEOUT) | B_ABSOLUTE_TIMEOUT; + } + + while (IsOverQuota(additionalPages)) { + ConditionVariableEntry waitEntry; + fUnderQuotaCondition.Add(&waitEntry); + + if (!IsOverQuota(additionalPages)) + return B_OK; + + fPageWriterCondition.WakeUp(); + status_t status = waitEntry.Wait(flags, timeout); + if (status != B_OK) + return status; + + // The queue itself is now under-quota, but it may still be over + // when considering additionalPages. So, we loop again. + } + + return B_OK; +}