Kernel: Implement SYS_SYNC and add sync executable to userspace

You can (and have to) manually sync disk after writes to it.

kernel/arch/x86_64/IDT.cpp

@@ -160,6 +160,7 @@ namespace IDT
 			regs->rip, regs->rflags,
 			regs->cr0, regs->cr2, regs->cr3, regs->cr4
 		);
+		Debug::dump_stack_trace();
 
 		if (tid)
 		{

kernel/include/kernel/Device/Device.h

@@ -13,6 +13,7 @@ namespace Kernel
 
 		virtual bool is_device() const override { return true; }
 		virtual bool is_partition() const { return false; }
+		virtual bool is_storage_device() const { return false; }
 
 		virtual dev_t rdev() const override = 0;
 

kernel/include/kernel/FS/DevFS/FileSystem.h

@@ -15,6 +15,7 @@ namespace Kernel
 		void initialize_device_updater();
 
 		void add_device(BAN::StringView path, BAN::RefPtr<RamInode>);
+		void for_each_device(const BAN::Function<BAN::Iteration(Device*)>& callback);
 
 		dev_t get_next_dev();
 

kernel/include/kernel/FS/RamFS/FileSystem.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <BAN/HashMap.h>
+#include <BAN/Iteration.h>
 #include <kernel/FS/FileSystem.h>
 #include <kernel/SpinLock.h>
 
@@ -25,7 +26,7 @@ namespace Kernel
 		blksize_t blksize() const { return m_blksize; }
 		ino_t next_ino() { return m_next_ino++; }
 
-		void for_each_inode(void (*callback)(BAN::RefPtr<RamInode>));
+		void for_each_inode(const BAN::Function<BAN::Iteration(BAN::RefPtr<RamInode>)>& callback);
 
 	protected:
 		RamFileSystem(size_t size)

kernel/include/kernel/Process.h

@@ -109,6 +109,8 @@ namespace Kernel
 		BAN::ErrorOr<long> sys_fstatat(int fd, const char* path, struct stat* buf, int flag);
 		BAN::ErrorOr<long> sys_stat(const char* path, struct stat* buf, int flag);
 
+		BAN::ErrorOr<long> sys_sync();
+
 		BAN::ErrorOr<void> mount(BAN::StringView source, BAN::StringView target);
 
 		BAN::ErrorOr<long> sys_read_dir_entries(int fd, DirectoryEntryList* buffer, size_t buffer_size);

kernel/include/kernel/Storage/DiskCache.h

@@ -7,16 +7,18 @@
 namespace Kernel
 {
 
+	class StorageDevice;
+
 	class DiskCache
 	{
 	public:
-		DiskCache(size_t sector_size);
+		DiskCache(size_t sector_size, StorageDevice&);
 		~DiskCache();
 
 		bool read_from_cache(uint64_t sector, uint8_t* buffer);
 		BAN::ErrorOr<void> write_to_cache(uint64_t sector, const uint8_t* buffer, bool dirty);
 
-		void sync();
+		BAN::ErrorOr<void> sync();
 		size_t release_clean_pages(size_t);
 		size_t release_pages(size_t);
 		void release_all_pages();
@@ -32,6 +34,7 @@ namespace Kernel
 
 	private:
 		const size_t m_sector_size;
+		StorageDevice& m_device;
 		BAN::Vector<PageCache> m_cache;
 	};
 

kernel/include/kernel/Storage/StorageDevice.h

@@ -76,12 +76,16 @@ namespace Kernel
 		BAN::Vector<Partition*>& partitions() { return m_partitions; }
 		const BAN::Vector<Partition*>& partitions() const { return m_partitions; }
 
+		BAN::ErrorOr<void> sync_disk_cache();
+		virtual bool is_storage_device() const override { return true; }
+
 	protected:
 		virtual BAN::ErrorOr<void> read_sectors_impl(uint64_t lba, uint8_t sector_count, uint8_t* buffer) = 0;
 		virtual BAN::ErrorOr<void> write_sectors_impl(uint64_t lba, uint8_t sector_count, const uint8_t* buffer) = 0;
 		void add_disk_cache();
 
 	private:
+		SpinLock					m_lock;
 		BAN::Optional<DiskCache>	m_disk_cache;
 		BAN::Vector<Partition*>		m_partitions;
 

kernel/kernel/FS/DevFS/FileSystem.cpp

@@ -44,6 +44,7 @@ namespace Kernel
 						{
 							if (inode->is_device())
 								((Device*)inode.ptr())->update();
+							return BAN::Iteration::Continue;
 						}
 					);
 					s_instance->m_device_lock.unlock();
@@ -60,8 +61,22 @@ namespace Kernel
 		MUST(reinterpret_cast<RamDirectoryInode*>(root_inode().ptr())->add_inode(path, device));
 	}
 
+	void DevFileSystem::for_each_device(const BAN::Function<BAN::Iteration(Device*)>& callback)
+	{
+		LockGuard _(m_device_lock);
+		for_each_inode(
+			[&](BAN::RefPtr<Kernel::RamInode> inode)
+			{
+				if (!inode->is_device())
+					return BAN::Iteration::Continue;
+				return callback((Device*)inode.ptr());
+			}
+		);
+	}
+
 	dev_t DevFileSystem::get_next_dev()
 	{
+		LockGuard _(m_device_lock);
 		static dev_t next_dev = 1;
 		return next_dev++;
 	}

kernel/kernel/FS/Ext2/Inode.cpp

@@ -227,7 +227,7 @@ namespace Kernel
 
 		const uint8_t* u8buffer = (const uint8_t*)buffer;
 
-		size_t written = 0;
+		size_t to_write = count;
 
 		// Write partial block
 		if (offset % block_size)
@@ -236,7 +236,7 @@ namespace Kernel
 			uint32_t block_offset = offset % block_size;
 
 			uint32_t data_block_index = TRY(this->data_block_index(block_index));
-			uint32_t to_copy = BAN::Math::min<uint32_t>(block_size - block_offset, written);
+			uint32_t to_copy = BAN::Math::min<uint32_t>(block_size - block_offset, to_write);
 
 			m_fs.read_block(data_block_index, block_buffer.span());
 			memcpy(block_buffer.data() + block_offset, buffer, to_copy);
@@ -244,10 +244,10 @@ namespace Kernel
 
 			u8buffer += to_copy;
 			offset += to_copy;
-			written -= to_copy;
+			to_write -= to_copy;
 		}
 
-		while (written >= block_size)
+		while (to_write >= block_size)
 		{
 			uint32_t data_block_index = TRY(this->data_block_index(offset / block_size));
 
@@ -255,15 +255,15 @@ namespace Kernel
 
 			u8buffer += block_size;
 			offset += block_size;
-			written -= block_size;
+			to_write -= block_size;
 		}
 
-		if (written > 0)
+		if (to_write > 0)
 		{
 			uint32_t data_block_index = TRY(this->data_block_index(offset / block_size));
 
 			m_fs.read_block(data_block_index, block_buffer.span());
-			memcpy(block_buffer.data(), u8buffer, written);
+			memcpy(block_buffer.data(), u8buffer, to_write);
 			m_fs.write_block(data_block_index, block_buffer.span());
 		}
 

kernel/kernel/FS/RamFS/FileSystem.cpp

@@ -48,11 +48,18 @@ namespace Kernel
 		return m_inodes[ino];
 	}
 
-	void RamFileSystem::for_each_inode(void (*callback)(BAN::RefPtr<RamInode>))
+	void RamFileSystem::for_each_inode(const BAN::Function<BAN::Iteration(BAN::RefPtr<RamInode>)>& callback)
 	{
 		LockGuard _(m_lock);
 		for (auto& [_, inode] : m_inodes)
-			callback(inode);
+		{
+			auto decision = callback(inode);
+			if (decision == BAN::Iteration::Break)
+				break;
+			if (decision == BAN::Iteration::Continue)
+				continue;
+			ASSERT_NOT_REACHED();
+		}
 	}
 
 }

kernel/kernel/Process.cpp

@@ -1,6 +1,7 @@
 #include <BAN/ScopeGuard.h>
 #include <BAN/StringView.h>
 #include <kernel/CriticalScope.h>
+#include <kernel/FS/DevFS/FileSystem.h>
 #include <kernel/FS/VirtualFileSystem.h>
 #include <kernel/InterruptController.h>
 #include <kernel/LockGuard.h>
@@ -8,6 +9,7 @@
 #include <kernel/Memory/PageTableScope.h>
 #include <kernel/Process.h>
 #include <kernel/Scheduler.h>
+#include <kernel/Storage/StorageDevice.h>
 #include <kernel/Timer/Timer.h>
 #include <LibELF/ELF.h>
 #include <LibELF/Values.h>
@@ -752,6 +754,24 @@ namespace Kernel
 		return 0;
 	}
 
+	BAN::ErrorOr<long> Process::sys_sync()
+	{
+		BAN::ErrorOr<long> ret = 0;
+		DevFileSystem::get().for_each_device(
+			[&](Device* device)
+			{
+				if (device->is_storage_device())
+				{
+					auto success = ((StorageDevice*)device)->sync_disk_cache();
+					if (success.is_error())
+						ret = success.release_error();
+				}
+				return BAN::Iteration::Continue;
+			}
+		);
+		return ret;
+	}
+
 	BAN::ErrorOr<long> Process::sys_read_dir_entries(int fd, DirectoryEntryList* list, size_t list_size)
 	{
 		LockGuard _(m_lock);

kernel/kernel/Storage/DiskCache.cpp

@@ -8,8 +8,9 @@
 namespace Kernel
 {
 
-	DiskCache::DiskCache(size_t sector_size)
+	DiskCache::DiskCache(size_t sector_size, StorageDevice& device)
 		: m_sector_size(sector_size)
+		, m_device(device)
 	{
 		ASSERT(PAGE_SIZE % m_sector_size == 0);
 		ASSERT(PAGE_SIZE / m_sector_size <= sizeof(PageCache::sector_mask) * 8);
@@ -31,7 +32,6 @@ namespace Kernel
 		LockGuard page_table_locker(page_table);
 		ASSERT(page_table.is_page_free(0));
 
-		CriticalScope _;
 		for (auto& cache : m_cache)
 		{
 			if (cache.first_sector < page_cache_start)
@@ -42,6 +42,7 @@ namespace Kernel
 			if (!(cache.sector_mask & (1 << page_cache_offset)))
 				continue;
 
+			CriticalScope _;
 			page_table.map_page_at(cache.paddr, 0, PageTable::Flags::Present);
 			memcpy(buffer, (void*)(page_cache_offset * m_sector_size), m_sector_size);
 			page_table.unmap_page(0);
@@ -62,8 +63,6 @@ namespace Kernel
 		LockGuard page_table_locker(page_table);
 		ASSERT(page_table.is_page_free(0));
 
-		CriticalScope _;
-
 		size_t index = 0;
 
 		// Search the cache if the have this sector in memory
@@ -76,9 +75,12 @@ namespace Kernel
 			if (cache.first_sector > page_cache_start)
 				break;
 			
-			page_table.map_page_at(cache.paddr, 0, PageTable::Flags::ReadWrite | PageTable::Flags::Present);
-			memcpy((void*)(page_cache_offset * m_sector_size), buffer, m_sector_size);
-			page_table.unmap_page(0);
+			{
+				CriticalScope _;
+				page_table.map_page_at(cache.paddr, 0, PageTable::Flags::ReadWrite | PageTable::Flags::Present);
+				memcpy((void*)(page_cache_offset * m_sector_size), buffer, m_sector_size);
+				page_table.unmap_page(0);
+			}
 
 			cache.sector_mask |= 1 << page_cache_offset;
 			if (dirty)
@@ -104,25 +106,51 @@ namespace Kernel
 			return ret.error();
 		}
 
-		page_table.map_page_at(cache.paddr, 0, PageTable::Flags::Present);
-		memcpy((void*)(page_cache_offset * m_sector_size), buffer, m_sector_size);
-		page_table.unmap_page(0);
+		{
+			CriticalScope _;
+			page_table.map_page_at(cache.paddr, 0, PageTable::Flags::Present);
+			memcpy((void*)(page_cache_offset * m_sector_size), buffer, m_sector_size);
+			page_table.unmap_page(0);
+		}
 
 		return {};
 	}
 
-	void DiskCache::sync()
+	BAN::ErrorOr<void> DiskCache::sync()
 	{
-		CriticalScope _;
+		BAN::Vector<uint8_t> sector_buffer;
+		TRY(sector_buffer.resize(m_sector_size));
+
+		PageTable& page_table = PageTable::current();
+		LockGuard page_table_locker(page_table);
+		ASSERT(page_table.is_page_free(0));
+
 		for (auto& cache : m_cache)
-			ASSERT(cache.dirty_mask == 0);
+		{
+			for (int i = 0; cache.dirty_mask; i++)
+			{
+				if (!(cache.dirty_mask & (1 << i)))
+					continue;
+
+				{
+					CriticalScope _;
+					page_table.map_page_at(cache.paddr, 0, PageTable::Flags::Present);
+					memcpy(sector_buffer.data(), (void*)(i * m_sector_size), m_sector_size);
+					page_table.unmap_page(0);
+				}
+
+				TRY(m_device.write_sectors_impl(cache.first_sector + i, 1, sector_buffer.data()));
+				cache.dirty_mask &= ~(1 << i);
+			}
+		}
+
+		return {};
 	}
 
 	size_t DiskCache::release_clean_pages(size_t page_count)
 	{
 		// NOTE: There might not actually be page_count pages after this
 		//       function returns. The synchronization must be done elsewhere.
-		CriticalScope _;
 
 		size_t released = 0;
 		for (size_t i = 0; i < m_cache.size() && released < page_count;)
@@ -147,8 +175,9 @@ namespace Kernel
 		size_t released = release_clean_pages(page_count);
 		if (released >= page_count)
 			return released;
-
-		ASSERT_NOT_REACHED();
+		if (!sync().is_error())
+			released += release_clean_pages(page_count - released);
+		return released;
 	}
 
 	void DiskCache::release_all_pages()

kernel/kernel/Storage/StorageDevice.cpp

@@ -3,6 +3,7 @@
 #include <BAN/StringView.h>
 #include <BAN/UTF8.h>
 #include <kernel/FS/VirtualFileSystem.h>
+#include <kernel/LockGuard.h>
 #include <kernel/PCI.h>
 #include <kernel/Storage/StorageDevice.h>
 #include <kernel/Thread.h>
@@ -257,15 +258,26 @@ namespace Kernel
 
 	void StorageDevice::add_disk_cache()
 	{
+		LockGuard _(m_lock);
 		ASSERT(!m_disk_cache.has_value());
-		m_disk_cache = DiskCache(sector_size());
+		m_disk_cache = DiskCache(sector_size(), *this);
+	}
+
+	BAN::ErrorOr<void> StorageDevice::sync_disk_cache()
+	{
+		LockGuard _(m_lock);
+		if (m_disk_cache.has_value())
+			TRY(m_disk_cache->sync());
+		return {};
 	}
 
 	BAN::ErrorOr<void> StorageDevice::read_sectors(uint64_t lba, uint8_t sector_count, uint8_t* buffer)
 	{
 		for (uint8_t offset = 0; offset < sector_count; offset++)
 		{
-			Thread::TerminateBlocker _(Thread::current());
+			LockGuard _(m_lock);
+			Thread::TerminateBlocker blocker(Thread::current());
+
 			uint8_t* buffer_ptr = buffer + offset * sector_size();
 			if (m_disk_cache.has_value())
 				if (m_disk_cache->read_from_cache(lba + offset, buffer_ptr))
@@ -283,11 +295,12 @@ namespace Kernel
 		// TODO: use disk cache for dirty pages. I don't wanna think about how to do it safely now
 		for (uint8_t offset = 0; offset < sector_count; offset++)
 		{
-			Thread::TerminateBlocker _(Thread::current());
+			LockGuard _(m_lock);
+			Thread::TerminateBlocker blocker(Thread::current());
+
 			const uint8_t* buffer_ptr = buffer + offset * sector_size();
-			TRY(write_sectors_impl(lba + offset, 1, buffer_ptr));
-			if (m_disk_cache.has_value())
-				(void)m_disk_cache->write_to_cache(lba + offset, buffer_ptr, false);
+			if (!m_disk_cache.has_value() || m_disk_cache->write_to_cache(lba + offset, buffer_ptr, true).is_error())
+				TRY(write_sectors_impl(lba + offset, 1, buffer_ptr));
 		}
 
 		return {};

kernel/kernel/Syscall.cpp

@@ -191,6 +191,9 @@ namespace Kernel
 		case SYS_STAT:
 			ret = Process::current().sys_stat((const char*)arg1, (struct stat*)arg2, (int)arg3);
 			break;
+		case SYS_SYNC:
+			ret = Process::current().sys_sync();
+			break;
 		default:
 			dwarnln("Unknown syscall {}", syscall);
 			break;

libc/include/sys/syscall.h

@@ -54,6 +54,7 @@ __BEGIN_DECLS
 #define SYS_NANOSLEEP 47
 #define SYS_FSTATAT 48
 #define SYS_STAT 49 // stat/lstat
+#define SYS_SYNC 50
 
 __END_DECLS
 

libc/unistd.cpp

@@ -183,6 +183,11 @@ int chdir(const char* path)
 	return syscall(SYS_SET_PWD, path);
 }
 
+void sync(void)
+{
+	syscall(SYS_SYNC);
+}
+
 pid_t getpid(void)
 {
 	return syscall(SYS_GET_PID);

userspace/CMakeLists.txt

@@ -12,6 +12,7 @@ set(USERSPACE_PROJECTS
 	Shell
 	snake
 	stat
+	sync
 	tee
 	test
 	touch

userspace/sync/CMakeLists.txt

@@ -0,0 +1,17 @@
+cmake_minimum_required(VERSION 3.26)
+
+project(sync CXX)
+
+set(SOURCES
+	main.cpp
+)
+
+add_executable(sync ${SOURCES})
+target_compile_options(sync PUBLIC -O2 -g)
+target_link_libraries(sync PUBLIC libc)
+
+add_custom_target(sync-install
+	COMMAND sudo cp ${CMAKE_CURRENT_BINARY_DIR}/sync ${BANAN_BIN}/
+	DEPENDS sync
+	USES_TERMINAL
+)

userspace/sync/main.cpp

@@ -0,0 +1,6 @@
+#include <unistd.h>
+
+int main()
+{
+	sync();
+}