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/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 */ 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)