From 752f9c28703e447ab5b127cbac94493a7539eb36 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Mon, 9 Mar 2020 18:46:55 +0900 Subject: devices: Ignore O_DIRECT in 9p and fs devices Specifying O_DIRECT in the 9p device doesn't actually work correctly and leads to an error. O_DIRECT handling in the fs device works correctly but also makes it look much worse in disk I/O benchmarks because the block device gets the benefit of the host cache while the fs device depends on the performance of the actual storage device. BUG=none TEST=`tast run vm.Fio.*` Change-Id: I738e4032081e331ef956c9d4c33616607e403d86 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2093967 Commit-Queue: Chirantan Ekbote Tested-by: Chirantan Ekbote Tested-by: kokoro Reviewed-by: Dylan Reid Reviewed-by: Daniel Verkamp --- devices/src/virtio/fs/passthrough.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'devices/src/virtio/fs/passthrough.rs') diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index bdc1c6f..d23009b 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -395,7 +395,7 @@ impl PassthroughFs { libc::openat( self.proc.as_raw_fd(), pathname.as_ptr(), - (flags | libc::O_CLOEXEC) & (!libc::O_NOFOLLOW), + (flags | libc::O_CLOEXEC) & !(libc::O_NOFOLLOW | libc::O_DIRECT), ) }; if fd < 0 { @@ -965,7 +965,8 @@ impl FileSystem for PassthroughFs { libc::openat( data.file.as_raw_fd(), name.as_ptr(), - flags as i32 | libc::O_CREAT | libc::O_CLOEXEC | libc::O_NOFOLLOW, + (flags as i32 | libc::O_CREAT | libc::O_CLOEXEC | libc::O_NOFOLLOW) + & !libc::O_DIRECT, mode & !(umask & 0o777), ) }; -- cgit 1.4.1 From 93a987705782a882c9f1d05b8b472276c20ae50f Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Tue, 17 Mar 2020 17:52:02 +0900 Subject: devices: fs: Support FOPEN_CACHE_DIR Add support for FOPEN_CACHE_DIR so that the guest can cache directory entries for longer. BUG=b:150264964 TEST=vm.Virtiofs Change-Id: Iade67b54084ed72378afa70af9e9e0f7f0bc03e8 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2105821 Reviewed-by: Daniel Verkamp Tested-by: kokoro Commit-Queue: Chirantan Ekbote --- devices/src/virtio/fs/passthrough.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'devices/src/virtio/fs/passthrough.rs') diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index d23009b..aab1403 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -592,7 +592,13 @@ impl PassthroughFs { OpenOptions::DIRECT_IO, flags & (libc::O_DIRECTORY as u32) == 0, ), - CachePolicy::Always => opts |= OpenOptions::KEEP_CACHE, + CachePolicy::Always => { + opts |= if flags & (libc::O_DIRECTORY as u32) == 0 { + OpenOptions::KEEP_CACHE + } else { + OpenOptions::CACHE_DIR + } + } _ => {} }; -- cgit 1.4.1 From 6dfa1e4ce5c260eabbda21c12d7bd9769f1cc995 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Tue, 17 Mar 2020 18:20:10 +0900 Subject: devices: fs: Implement copy_file_range BUG=none TEST=vm.Virtiofs Change-Id: I2ed7137a901e6e506e6b1562b77fdb042bdc58ab Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2105822 Tested-by: Chirantan Ekbote Tested-by: kokoro Reviewed-by: Daniel Verkamp Commit-Queue: Chirantan Ekbote --- devices/src/virtio/fs/passthrough.rs | 54 ++++++++++++++++++++++++++++++++++++ seccomp/aarch64/fs_device.policy | 1 + seccomp/arm/fs_device.policy | 1 + seccomp/x86_64/fs_device.policy | 1 + 4 files changed, 57 insertions(+) (limited to 'devices/src/virtio/fs/passthrough.rs') diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index aab1403..419d466 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -1696,4 +1696,58 @@ impl FileSystem for PassthroughFs { Err(io::Error::from_raw_os_error(libc::ENOTTY)) } } + + fn copy_file_range( + &self, + ctx: Context, + inode_src: Inode, + handle_src: Handle, + offset_src: u64, + inode_dst: Inode, + handle_dst: Handle, + offset_dst: u64, + length: u64, + flags: u64, + ) -> io::Result { + // We need to change credentials during a write so that the kernel will remove setuid or + // setgid bits from the file if it was written to by someone other than the owner. + let (_uid, _gid) = set_creds(ctx.uid, ctx.gid)?; + let src_data = self + .handles + .read() + .unwrap() + .get(&handle_src) + .filter(|hd| hd.inode == inode_src) + .map(Arc::clone) + .ok_or_else(ebadf)?; + let dst_data = self + .handles + .read() + .unwrap() + .get(&handle_dst) + .filter(|hd| hd.inode == inode_dst) + .map(Arc::clone) + .ok_or_else(ebadf)?; + + let src = src_data.file.lock().as_raw_fd(); + let dst = dst_data.file.lock().as_raw_fd(); + + let res = unsafe { + libc::syscall( + libc::SYS_copy_file_range, + src, + &offset_src, + dst, + offset_dst, + length, + flags, + ) + }; + + if res >= 0 { + Ok(res as usize) + } else { + Err(io::Error::last_os_error()) + } + } } diff --git a/seccomp/aarch64/fs_device.policy b/seccomp/aarch64/fs_device.policy index 9fd4c8b..ec9d155 100644 --- a/seccomp/aarch64/fs_device.policy +++ b/seccomp/aarch64/fs_device.policy @@ -4,6 +4,7 @@ @include /usr/share/policy/crosvm/common_device.policy +copy_file_range: 1 fallocate: 1 fchmodat: 1 fchownat: 1 diff --git a/seccomp/arm/fs_device.policy b/seccomp/arm/fs_device.policy index eb9df16..4078f41 100644 --- a/seccomp/arm/fs_device.policy +++ b/seccomp/arm/fs_device.policy @@ -4,6 +4,7 @@ @include /usr/share/policy/crosvm/common_device.policy +copy_file_range: 1 fallocate: 1 fchmodat: 1 fchownat: 1 diff --git a/seccomp/x86_64/fs_device.policy b/seccomp/x86_64/fs_device.policy index ddb2a51..eb5a1c4 100644 --- a/seccomp/x86_64/fs_device.policy +++ b/seccomp/x86_64/fs_device.policy @@ -4,6 +4,7 @@ @include /usr/share/policy/crosvm/common_device.policy +copy_file_range: 1 fallocate: 1 fchmodat: 1 fchownat: 1 -- cgit 1.4.1