From c3f882094753fe6b610c42cddd5e68ec029e3e7c Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Thu, 27 Aug 2026 04:35:01 +0000 Subject: [PATCH 1/2] pagecache: report page-writeback errors from sync() by return value pagecache::sync() threw a C++ error when a page writeback failed. Its only caller is the munmap / msync path (vfs_file::sync, invoked from mmu::file_vma::sync), which already converts the outcome to a return value and surfaces any error through its own sys_fsync() tail. No caller depended on the exception. Throwing from that path is both unnecessary and fragile: raising an exception during munmap can abort when the C++ unwinder cannot complete in the calling context, turning a recoverable writeback error into a fatal abort() that takes down the whole instance. Return the writeback errno from sync() instead; vfs_file::sync() consumes it and file_vma::sync() continues to report the error via sys_fsync(). Behavior is unchanged on success. On writeback error, sync() now returns the errno rather than throwing. --- core/pagecache.cc | 17 ++++++++++++++--- fs/vfs/vfs_fops.cc | 5 ++++- include/osv/pagecache.hh | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/core/pagecache.cc b/core/pagecache.cc index e9259dba7..0731436d2 100644 --- a/core/pagecache.cc +++ b/core/pagecache.cc @@ -957,7 +957,15 @@ bool release(vfs_file* fp, void *addr, off_t offset, mmu::hw_ptep<0> ptep) return addr != zero_page; } -void sync(vfs_file* fp, off_t start, off_t end) +// Flush dirty MAP_SHARED pages for [start,end) back to the filesystem. +// Returns 0 on success or the writeback errno. Historically this threw a C++ +// error on writeback failure, but it is reached from munmap() (via +// mmu::file_vma::sync) as well as from an explicit fsync path, and throwing on +// the munmap path is both unnecessary (the caller converts the result to a +// return value) and unsafe: an exception raised there can abort when the C++ +// unwinder cannot run in that context. Report the error by return value +// instead and let each caller decide how to surface it. +int sync(vfs_file* fp, off_t start, off_t end) { struct stat st; fp->stat(&st); @@ -989,20 +997,23 @@ void sync(vfs_file* fp, off_t start, off_t end) } if (to_flush.empty()) - return; + return 0; mmu::flush_tlb_all(); /* Phase 3: write each page back to the filesystem. */ + int ret = 0; for (auto cp : to_flush) { auto err = cp->writeback(); if (err) { // Re-mark dirty: phase 2 cleared the flag, but the data never // reached the filesystem, so it must not be treated as clean. cp->mark_dirty(); - throw make_error(err); + if (!ret) + ret = err; } } + return ret; } /* diff --git a/fs/vfs/vfs_fops.cc b/fs/vfs/vfs_fops.cc index 603af8337..989c52c47 100644 --- a/fs/vfs/vfs_fops.cc +++ b/fs/vfs/vfs_fops.cc @@ -177,7 +177,10 @@ bool vfs_file::put_page(void *addr, uintptr_t off, mmu::hw_ptep<0> ptep) void vfs_file::sync(off_t start, off_t end) { - pagecache::sync(this, start, end); + // pagecache::sync() now reports writeback errors by return value rather + // than throwing; the munmap/msync path (mmu::file_vma::sync) surfaces any + // error via its own sys_fsync() tail, so no caller depends on a throw here. + (void) pagecache::sync(this, start, end); } // Locking: VOP_CACHE will call into the filesystem, and that can trigger an diff --git a/include/osv/pagecache.hh b/include/osv/pagecache.hh index c91b9c953..3c5746be6 100644 --- a/include/osv/pagecache.hh +++ b/include/osv/pagecache.hh @@ -39,7 +39,7 @@ bool release(vfs_file* fp, void *addr, off_t offset, mmu::hw_ptep<0> ptep); * sync() — flush all dirty pages in [start, end) for the file described by fp. * Throws on I/O error. Used by VOP_FSYNC implementations. */ -void sync(vfs_file* fp, off_t start, off_t end); +int sync(vfs_file* fp, off_t start, off_t end); /* * writeback_inode() — flush dirty write-cache pages for a specific (dev, ino) From 9f06b5f9da889b2e2237aeec54cc3947d35c863d Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Thu, 27 Aug 2026 07:50:00 +0000 Subject: [PATCH 2/2] fs: default file::sync() is a no-op instead of throwing ENOSYS The base file::sync(off_t,off_t) threw make_error(ENOSYS). It is called from the munmap / msync page-writeback path (mmu::file_vma::sync -> _file->sync()), so unmapping a file-backed mapping whose file type does not override sync() raised a C++ exception during munmap(). That exception cannot be unwound from this path (it reaches __cxa_throw / _Unwind_RaiseException and bottoms out in abort()), taking down the instance. A file type that does not implement sync() has no cached writable pages to flush, so the correct default is to do nothing. vfs_file, the only type that caches writable file pages, overrides sync() with the real page-cache writeback (which reports errors by return value, per the previous commit). Make the default a no-op so munmap of any other file-backed mapping cannot raise an exception. Companion to the pagecache::sync() change: together they remove all C++ throws from the munmap page-writeback path. --- include/osv/file.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/osv/file.h b/include/osv/file.h index a39960f83..0c7f713f7 100755 --- a/include/osv/file.h +++ b/include/osv/file.h @@ -168,7 +168,7 @@ struct file { virtual bool map_page(uintptr_t offset, mmu::hw_ptep<1> ptep, mmu::pt_element<1> pte, bool write, bool shared) { throw make_error(ENOSYS); } virtual bool put_page(void *addr, uintptr_t offset, mmu::hw_ptep<0> ptep) { throw make_error(ENOSYS); } virtual bool put_page(void *addr, uintptr_t offset, mmu::hw_ptep<1> ptep) { throw make_error(ENOSYS); } - virtual void sync(off_t start, off_t end) { throw make_error(ENOSYS); } + virtual void sync(off_t start, off_t end) { /* default: nothing to sync */ } int f_flags; /* open flags */ int f_count; /* reference count, see below */