ptfs: refine the way to support O_DIRECT

We have special support of the O_DIRECT flag, but the implementation
has race conditions due to `Time To Check & Time To Use` issues.
So refine the implementation by using a RwLock.

Signed-off-by: Jiang Liu <[email protected]>

Author: jiangliu

src/passthrough/mod.rs

@@ -22,8 +22,8 @@ use std::os::fd::{AsFd, BorrowedFd};
 use std::os::unix::ffi::OsStringExt;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::path::PathBuf;
-use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering};
-use std::sync::{Arc, Mutex, MutexGuard, RwLock, RwLockWriteGuard};
+use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
+use std::sync::{Arc, RwLock, RwLockWriteGuard};
 use std::time::Duration;
 
 use vm_memory::{bitmap::BitmapSlice, ByteValued};
@@ -35,7 +35,7 @@ use self::mount_fd::MountFds;
 use self::statx::{statx, StatExt};
 use self::util::{
     ebadf, einval, enosys, eperm, is_dir, is_safe_inode, openat, reopen_fd_through_proc, stat_fd,
-    UniqueInodeGenerator,
+    FileFlagGuard, UniqueInodeGenerator,
 };
 use crate::abi::fuse_abi as fuse;
 use crate::abi::fuse_abi::Opcode;
@@ -256,39 +256,30 @@ impl InodeMap {
 struct HandleData {
     inode: Inode,
     file: File,
-    lock: Mutex<()>,
-    open_flags: AtomicU32,
+    open_flags: RwLock<u32>,
 }
 
 impl HandleData {
     fn new(inode: Inode, file: File, flags: u32) -> Self {
         HandleData {
             inode,
             file,
-            lock: Mutex::new(()),
-            open_flags: AtomicU32::new(flags),
+            open_flags: RwLock::new(flags),
         }
     }
 
     fn get_file(&self) -> &File {
         &self.file
     }
 
-    fn get_file_mut(&self) -> (MutexGuard<()>, &File) {
-        (self.lock.lock().unwrap(), &self.file)
+    fn get_file_mut(&self) -> (FileFlagGuard<u32>, &File) {
+        let guard = self.open_flags.write().unwrap();
+        (FileFlagGuard::Writer(guard), &self.file)
     }
 
     fn borrow_fd(&self) -> BorrowedFd {
         self.file.as_fd()
     }
-
-    fn get_flags(&self) -> u32 {
-        self.open_flags.load(Ordering::Relaxed)
-    }
-
-    fn set_flags(&self, flags: u32) {
-        self.open_flags.store(flags, Ordering::Relaxed);
-    }
 }
 
 struct HandleMap {

src/passthrough/sync_io.rs

@@ -43,16 +43,34 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
     /// if these do not match update the file descriptor flags and store the new
     /// result in the HandleData entry
     #[inline(always)]
-    fn check_fd_flags(&self, data: Arc<HandleData>, fd: RawFd, flags: u32) -> io::Result<()> {
-        let open_flags = data.get_flags();
-        if open_flags != flags {
-            let ret = unsafe { libc::fcntl(fd, libc::F_SETFL, flags) };
+    fn ensure_file_flags<'a>(
+        &self,
+        data: &'a Arc<HandleData>,
+        file: &File,
+        mut flags: u32,
+    ) -> io::Result<FileFlagGuard<'a, u32>> {
+        let guard = data.open_flags.read().unwrap();
+        if *guard & libc::O_DIRECT as u32 == flags & libc::O_DIRECT as u32 {
+            return Ok(FileFlagGuard::Reader(guard));
+        }
+        drop(guard);
+
+        let mut guard = data.open_flags.write().unwrap();
+        // Update the O_DIRECT flag if needed
+        if *guard & libc::O_DIRECT as u32 != flags & libc::O_DIRECT as u32 {
+            if flags & libc::O_DIRECT as u32 != 0 {
+                flags = *guard | libc::O_DIRECT as u32;
+            } else {
+                flags = *guard & !libc::O_DIRECT as u32;
+            }
+            let ret = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_SETFL, flags) };
             if ret != 0 {
                 return Err(io::Error::last_os_error());
             }
-            data.set_flags(flags);
+            *guard = flags;
         }
-        Ok(())
+
+        Ok(FileFlagGuard::Writer(guard))
     }
 
     fn do_readdir(
@@ -665,7 +683,7 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
         // so data.file won't be closed.
         let f = unsafe { File::from_raw_fd(data.borrow_fd().as_raw_fd()) };
 
-        self.check_fd_flags(data, f.as_raw_fd(), flags)?;
+        self.ensure_file_flags(&data, &f, flags)?;
 
         let mut f = ManuallyDrop::new(f);
 
@@ -692,7 +710,7 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
         // so data.file won't be closed.
         let f = unsafe { File::from_raw_fd(data.borrow_fd().as_raw_fd()) };
 
-        self.check_fd_flags(data, f.as_raw_fd(), flags)?;
+        self.ensure_file_flags(&data, &f, flags)?;
 
         if self.seal_size.load(Ordering::Relaxed) {
             let st = stat_fd(&f, None)?;

src/passthrough/util.rs

@@ -7,9 +7,10 @@ use std::ffi::{CStr, CString};
 use std::fs::File;
 use std::io;
 use std::mem::MaybeUninit;
+use std::ops::Deref;
 use std::os::unix::io::{AsRawFd, FromRawFd};
 use std::sync::atomic::{AtomicU64, AtomicU8, Ordering};
-use std::sync::Mutex;
+use std::sync::{Mutex, RwLockReadGuard, RwLockWriteGuard};
 
 use super::inode_store::InodeId;
 use super::MAX_HOST_INO;
@@ -225,6 +226,23 @@ pub fn eperm() -> io::Error {
     io::Error::from_raw_os_error(libc::EPERM)
 }
 
+/// A helper structure to hold RwLock guard.
+pub enum FileFlagGuard<'a, T> {
+    Reader(RwLockReadGuard<'a, T>),
+    Writer(RwLockWriteGuard<'a, T>),
+}
+
+impl<'a, T> Deref for FileFlagGuard<'a, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        match self {
+            FileFlagGuard::Reader(v) => v.deref(),
+            FileFlagGuard::Writer(v) => v.deref(),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;