Skip to content

Commit 7eb699e

Browse files
slpXanClic
authored andcommitted
file: implement relaxed sync for macos
On macOS, fsync() flushes the filesystem cache to the drive, but doesn't request the drive to flush it's internal cache to its persistent storage. To do the latter, there's a specific fcntl: F_FULLFSYNC. The reason why they decided to have fsync() behave this way it's because in Apple Silicon hardware flushing the drive cache has a very significant performance impact. In File::sync() we were calling std::fs::File::sync_all(), which does use F_FULLSYNC, which is safer but comes with a high cost. Let's allow crate users on macOS opt for a more relaxed synchronization using fsync() instead of std::fs::File::sync_all(). Expose the option in StorageOpenOptions as relaxed_sync but gate it behind a build-time conditional for target_os = macos. Signed-off-by: Sergio Lopez <slp@redhat.com> (cherry picked from commit 16db536) Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
1 parent 6acc672 commit 7eb699e

File tree

2 files changed

+54
-3
lines changed

2 files changed

+54
-3
lines changed

src/file.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ pub struct File {
5454

5555
/// Storage helper.
5656
common_storage_helper: CommonStorageHelper,
57+
58+
/// macOS-only: Use fsync() instead of F_FULLFSYNC on `sync()` method.
59+
#[cfg(target_os = "macos")]
60+
relaxed_sync: bool,
5761
}
5862

5963
impl TryFrom<fs::File> for File {
@@ -66,7 +70,13 @@ impl TryFrom<fs::File> for File {
6670
/// When using this, the resulting object will not know its own filename. That makes it
6771
/// impossible to auto-resolve relative paths to it, e.g. qcow2 backing file names.
6872
fn try_from(file: fs::File) -> io::Result<Self> {
69-
Self::new(file, None, false)
73+
Self::new(
74+
file,
75+
None,
76+
false,
77+
#[cfg(target_os = "macos")]
78+
false,
79+
)
7080
}
7181
}
7282

@@ -333,6 +343,12 @@ impl Storage for File {
333343
}
334344

335345
async fn sync(&self) -> io::Result<()> {
346+
#[cfg(target_os = "macos")]
347+
if self.relaxed_sync {
348+
// Safe: File descriptor is valid and there aren't any other arguments.
349+
while_eintr(|| unsafe { libc::fsync(self.file.write().unwrap().as_raw_fd()) })?;
350+
return Ok(());
351+
}
336352
self.file.write().unwrap().sync_all()
337353
}
338354

@@ -346,7 +362,12 @@ impl File {
346362
///
347363
/// `direct_io` should be `true` if direct I/O was requested, and can be `false` if that status
348364
/// is unknown.
349-
fn new(mut file: fs::File, filename: Option<PathBuf>, direct_io: bool) -> io::Result<Self> {
365+
fn new(
366+
mut file: fs::File,
367+
filename: Option<PathBuf>,
368+
direct_io: bool,
369+
#[cfg(target_os = "macos")] relaxed_sync: bool,
370+
) -> io::Result<Self> {
350371
let size = get_file_size(&file).err_context(|| "Failed to determine file size")?;
351372

352373
#[cfg(all(unix, not(target_os = "macos")))]
@@ -385,6 +406,8 @@ impl File {
385406
mem_align,
386407
size: size.into(),
387408
common_storage_helper: Default::default(),
409+
#[cfg(target_os = "macos")]
410+
relaxed_sync,
388411
})
389412
}
390413

@@ -663,7 +686,13 @@ impl File {
663686
.err_context(|| "Failed to disable host cache")?;
664687
}
665688

666-
Self::new(file, Some(filename_owned), opts.direct)
689+
Self::new(
690+
file,
691+
Some(filename_owned),
692+
opts.direct,
693+
#[cfg(target_os = "macos")]
694+
opts.relaxed_sync,
695+
)
667696
}
668697

669698
/// Attempt to discard range by truncating the file.

src/storage/mod.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ pub struct StorageOpenOptions {
2727

2828
/// Whether to bypass the host page cache (if applicable).
2929
pub(crate) direct: bool,
30+
31+
/// macOS-only: Use fsync() instead of F_FULLFSYNC on `sync()` method.
32+
#[cfg(target_os = "macos")]
33+
pub(crate) relaxed_sync: bool,
3034
}
3135

3236
/// Implementation for storage objects.
@@ -533,6 +537,18 @@ impl StorageOpenOptions {
533537
self
534538
}
535539

540+
/// macOS-only: whether to use relaxed synchronization on `File`.
541+
///
542+
/// If relaxed synchronization is enabled, `File::sync()` will use the `fsync()` syscall
543+
/// instead of `fcntl(F_FULLFSYNC)`, which is a lighter synchronization mechanism that flushes
544+
/// the filesystem cache to the drive, but doesn't request the drive to flush its internal
545+
/// buffers to persistent storage.
546+
#[cfg(target_os = "macos")]
547+
pub fn relaxed_sync(mut self, relaxed_sync: bool) -> Self {
548+
self.relaxed_sync = relaxed_sync;
549+
self
550+
}
551+
536552
/// Get the set filename (if any).
537553
pub fn get_filename(&self) -> Option<&Path> {
538554
self.filename.as_deref()
@@ -547,4 +563,10 @@ impl StorageOpenOptions {
547563
pub fn get_direct(&self) -> bool {
548564
self.direct
549565
}
566+
567+
/// macOS-only: return the relaxed synchronization state.
568+
#[cfg(target_os = "macos")]
569+
pub fn get_relaxed_sync(&self) -> bool {
570+
self.relaxed_sync
571+
}
550572
}

0 commit comments

Comments
 (0)