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 <[email protected]>
Reviewed-by: waddlesplash <[email protected]>

src/system/kernel/cache/file_cache.cpp

@@ -27,6 +27,7 @@
 #include <vm/VMCache.h>
 
 #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<VMCache> 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

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

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)
 {

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;
+}