From 74120bb681c5a5e51ba0fe4b0835fa97d731be55 Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Fri, 8 May 2026 16:25:05 +0000 Subject: [PATCH] virtiofs: remove unnecessary per-entry lookup from plain readdir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plain readdir path was calling lookup_helper() on every directory entry just to check accessibility, then discarding the result. This caused a full stat round-trip per entry even though the lookup results were not returned to the guest kernel (unlike readdirplus, where they pre-populate the dentry cache). Neither virtiofsd nor CrosVM performs per-entry lookups during plain readdir — they return raw getdents64 results directly. Directory read permission is already verified at opendir time, making the per-entry access check redundant. This should improve performance for directory listing workloads where the kernel uses plain readdir (e.g. via READDIRPLUS_AUTO fallback). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- vm/devices/virtio/virtiofs/src/file.rs | 46 ++++++++++---------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/vm/devices/virtio/virtiofs/src/file.rs b/vm/devices/virtio/virtiofs/src/file.rs index ec9233da9b..071267a1d4 100644 --- a/vm/devices/virtio/virtiofs/src/file.rs +++ b/vm/devices/virtio/virtiofs/src/file.rs @@ -78,22 +78,6 @@ impl VirtioFsFile { let mut file = self.file.write(); file.read_dir(offset as lx::off_t, |entry| { entry_count += 1; - let get_child_fuse_entry = || -> lx::Result> { - match fs.lookup_helper(&self.inode, &entry.name) { - Ok(e) => Ok(Some(e)), - Err(err) => { - // Ignore entries that are inaccessible to the user or deleted. - // ENOENT can occur if a file was deleted between enumeration - // and lookup (e.g., when deleting files in a loop while - // enumerating the directory). - if err.value() == lx::EACCES || err.value() == lx::ENOENT { - Ok(None) - } else { - Err(err) - } - } - } - }; // If readdirplus is being used, do a lookup on all items except the . and .. entries. if plus { let fuse_entry = if entry.name == "." || entry.name == ".." { @@ -106,28 +90,32 @@ impl VirtioFsFile { return Ok(false); } - match get_child_fuse_entry()? { - Some(e) => e, - None => { - // Ignore entries that are inaccessible to the user. - entry_count -= 1; - return Ok(true); + match fs.lookup_helper(&self.inode, &entry.name) { + Ok(e) => e, + Err(err) => { + // Ignore entries that are inaccessible to the + // user or deleted. ENOENT can occur if a file was + // deleted between enumeration and lookup. + if err.value() == lx::EACCES || err.value() == lx::ENOENT { + entry_count -= 1; + return Ok(true); + } + return Err(err); } } }; Ok(buffer.dir_entry_plus(&entry.name, entry.offset as u64, fuse_entry)) } else { - // Windows doesn't report the inode number for . and .., so just use the current file's - // inode number for that. + // For plain readdir, return directory entries directly without + // per-entry lookups. Directory read permission is already + // checked at opendir time, so the per-entry access check + // is unnecessary overhead. let inode_nr = if entry.inode_nr == 0 { + // Windows doesn't report inode numbers for . and .., so + // use the current directory's inode number. self_inode_nr } else { - if get_child_fuse_entry()?.is_none() { - // Ignore entries that are inaccessible to the user. - entry_count -= 1; - return Ok(true); - } entry.inode_nr };