kernel: Add thread setting to prevent page waits and use it in the file cache.

See inline comment: otherwise we could deadlock waiting for busy pages.

At the same time, make page_faults_allowed just an int16 and drop
atomics and extra checks, they aren't needed.

Fixes #19441.

Change-Id: I1b7cc06f66b44c3520fa36497c076ee5a6320706
Reviewed-on: https://review.haiku-os.org/c/haiku/+/9120
Tested-by: Commit checker robot <[email protected]>
Reviewed-by: waddlesplash <[email protected]>

headers/private/kernel/thread_types.h

@@ -287,8 +287,8 @@ struct Thread : TeamThreadIteratorEntry<thread_id>, KernelReferenceable {
 
 	void			(*fault_handler)(void);
 	jmp_buf			fault_handler_state;
-	int32			page_faults_allowed;
-		/* this field may only stay in debug builds in the future */
+	int16			page_faults_allowed;
+	int16			page_fault_waits_allowed;
 
 	BKernel::Team	*team;	// protected by team lock, thread lock, scheduler
 							// lock, team_lock

headers/private/kernel/vm/vm.h

@@ -68,8 +68,8 @@ void slab_init_post_sem();
 void slab_init_post_thread();
 
 // to protect code regions with interrupts turned on
-void permit_page_faults(void);
-void forbid_page_faults(void);
+void permit_page_faults();
+void forbid_page_faults();
 
 // private kernel only extension (should be moved somewhere else):
 area_id create_area_etc(team_id team, const char *name, size_t size,

src/add-ons/kernel/file_systems/ramfs/DataContainer.cpp

@@ -3,14 +3,16 @@
  * Copyright 2019-2024, Haiku, Inc. All rights reserved.
  * Distributed under the terms of the MIT license.
  */
+
 #include "DataContainer.h"
 
 #include <StackOrHeapArray.h>
 #include <util/AutoLock.h>
 #include <util/BitUtils.h>
 #include <slab/Slab.h>
-
+#include <thread.h>
 #include <vfs.h>
+
 #include <vm/VMCache.h>
 #include <vm/vm_page.h>
 #include "VMAnonymousNoSwapCache.h"
@@ -321,8 +323,8 @@ DataContainer::_SwitchToCacheMode()
 
 
 status_t
-DataContainer::_DoCacheIO(const off_t offset, uint8* buffer, ssize_t length,
-	size_t* bytesProcessed, bool isWrite)
+DataContainer::_DoCacheIO(off_t offset, uint8* buffer, ssize_t length,
+	size_t* bytesProcessed, bool isWrite, bool retriesAllowed)
 {
 	const size_t originalLength = length;
 	const bool user = IS_USER_ADDRESS(buffer);
@@ -335,6 +337,7 @@ DataContainer::_DoCacheIO(const off_t offset, uint8* buffer, ssize_t length,
 		return B_NO_MEMORY;
 
 	cache_get_pages(fCache, rounded_offset, rounded_len, isWrite, pages);
+	thread_get_current_thread()->page_fault_waits_allowed--;
 
 	status_t error = B_OK;
 	size_t index = 0;
@@ -358,23 +361,39 @@ DataContainer::_DoCacheIO(const off_t offset, uint8* buffer, ssize_t length,
 			if (page != NULL) {
 				error = vm_memcpy_from_physical(buffer, at, bytes, user);
 			} else {
-				if (user) {
+				if (user)
 					error = user_memset(buffer, 0, bytes);
-				} else {
+				else
 					memset(buffer, 0, bytes);
-				}
 			}
 		}
 		if (error != B_OK)
 			break;
 
+		offset += bytes;
 		buffer += bytes;
 		length -= bytes;
 		index++;
 	}
 
+	thread_get_current_thread()->page_fault_waits_allowed++;
 	cache_put_pages(fCache, rounded_offset, rounded_len, pages, error == B_OK);
 
+	if (error == B_BUSY && retriesAllowed) {
+		// See comment in the file_cache's cache_io() routine.
+		if (user) {
+			error = user_memset(buffer, 0, length);
+		} else {
+			memset(buffer, 0, length);
+			error = B_OK;
+		}
+		if (error == B_OK) {
+			size_t processed;
+			error = _DoCacheIO(offset, buffer, length, &processed, isWrite, false);
+			length -= processed;
+		}
+	}
+
 	if (bytesProcessed != NULL)
 		*bytesProcessed = length > 0 ? originalLength - length : originalLength;
 

src/add-ons/kernel/file_systems/ramfs/DataContainer.h

@@ -42,8 +42,8 @@ private:
 	inline bool _RequiresCacheMode(size_t size);
 	inline bool _IsCacheMode() const;
 	status_t _SwitchToCacheMode();
-	status_t _DoCacheIO(const off_t offset, uint8* buffer, ssize_t length,
-		size_t* bytesProcessed, bool isWrite);
+	status_t _DoCacheIO(off_t offset, uint8* buffer, ssize_t length,
+		size_t* bytesProcessed, bool isWrite, bool retriesAllowed = true);
 
 	inline int32 _CountBlocks() const;
 

src/system/kernel/cache/file_cache.cpp

@@ -450,7 +450,6 @@ 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_END(pages[i]);
-
 		cache->MarkPageUnbusy(pages[i]);
 	}
 
@@ -729,7 +728,7 @@ satisfy_cache_io(file_cache_ref* ref, void* cookie, cache_func function,
 
 
 static status_t
-cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer,
+do_cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer,
 	size_t* _size, bool doWrite)
 {
 	if (_cacheRef == NULL)
@@ -911,6 +910,47 @@ cache_io(void* _cacheRef, void* cookie, off_t offset, addr_t buffer,
 }
 
 
+static status_t
+cache_io(void* ref, void* cookie, off_t offset, addr_t buffer,
+	size_t* _size, bool doWrite)
+{
+	size_t originalSize = *_size;
+
+	thread_get_current_thread()->page_fault_waits_allowed--;
+	status_t status = do_cache_io(ref, cookie, offset, buffer, _size, doWrite);
+	thread_get_current_thread()->page_fault_waits_allowed++;
+
+	if (status == B_BUSY) {
+		// This likely means that fault handler would've needed to wait for a page,
+		// but we can't allow that here because it could be one of our pages that
+		// it would've waited on, which would cause a deadlock.
+		// Call memset so that all pages are faulted in, and retry.
+		off_t retryOffset = offset;
+		addr_t retryBuffer = buffer;
+		size_t retrySize = originalSize;
+		if (*_size != originalSize) {
+			retryOffset += *_size;
+			retryBuffer += *_size;
+			retrySize -= *_size;
+		}
+		if (IS_USER_ADDRESS(buffer)) {
+			status = user_memset((void*)retryBuffer, 0, retrySize);
+		} else {
+			memset((void*)retryBuffer, 0, retrySize);
+			status = B_OK;
+		}
+		if (status == B_OK) {
+			thread_get_current_thread()->page_fault_waits_allowed--;
+			status = do_cache_io(ref, cookie, retryOffset, retryBuffer, &retrySize, doWrite);
+			*_size += retrySize;
+			thread_get_current_thread()->page_fault_waits_allowed++;
+		}
+	}
+
+	return status;
+}
+
+
 static status_t
 file_cache_control(const char* subsystem, uint32 function, void* buffer,
 	size_t bufferSize)

src/system/kernel/thread.cpp

@@ -281,6 +281,7 @@ Thread::Thread(const char* name, thread_id threadID, struct cpu_ent* cpu)
 	user_thread(NULL),
 	fault_handler(0),
 	page_faults_allowed(1),
+	page_fault_waits_allowed(1),
 	team(NULL),
 	select_infos(NULL),
 	kernel_stack_area(-1),

src/system/kernel/vm/vm.cpp

@@ -4084,20 +4084,16 @@ vm_init_post_modules(kernel_args* args)
 
 
 void
-permit_page_faults(void)
+permit_page_faults()
 {
-	Thread* thread = thread_get_current_thread();
-	if (thread != NULL)
-		atomic_add(&thread->page_faults_allowed, 1);
+	thread_get_current_thread()->page_faults_allowed++;
 }
 
 
 void
-forbid_page_faults(void)
+forbid_page_faults()
 {
-	Thread* thread = thread_get_current_thread();
-	if (thread != NULL)
-		atomic_add(&thread->page_faults_allowed, -1);
+	thread_get_current_thread()->page_faults_allowed--;
 }
 
 
@@ -4289,6 +4285,9 @@ fault_get_page(PageFaultContext& context)
 
 		page = cache->LookupPage(context.cacheOffset);
 		if (page != NULL && page->busy) {
+			if (thread_get_current_thread()->page_fault_waits_allowed < 1)
+				return B_BUSY;
+
 			// page must be busy -- wait for it to become unbusy
 			context.UnlockAll(cache);
 			cache->ReleaseRefLocked();