Skip to content

virtiofs: remove unnecessary per-entry lookup from plain readdir #3440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benhillis wants to merge 1 commit into
base: main
Choose a base branch
from
Open

virtiofs: remove unnecessary per-entry lookup from plain readdir #3440

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 17 additions & 29 deletions vm/devices/virtio/virtiofs/src/file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<fuse_entry_out>> {
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 == ".." {
Expand All @@ -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
};

Expand Down
Loading