From cc9368ccb45e7f9c972898d00c89bb7b5daa3b6a Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Mon, 12 Sep 2022 14:38:23 +0200 Subject: [PATCH] os: add File_system_session::num_entries RPC This patch splits the querying of the number of directory entries from the directory's 'status' information. Subsuming the number of directory entries as part of the status makes 'stat' calls too costly for some file systems that need to read a directory for determining the number of entries. So when stat'ing the entries of one directory that contains sub directories, all entries of each sub directory are visited. Thanks to Cedric Degea for pointing out this performance bottleneck! With this change, the 'status' function returns a 'Status::size' value of 0 when called for a directory handle. Fixes #4603 --- repos/os/include/file_system_session/client.h | 5 +++++ .../file_system_session/file_system_session.h | 15 ++++++++++++--- repos/os/src/lib/vfs/fs_file_system.h | 8 +++----- repos/os/src/server/lx_fs/directory.h | 9 +++++++-- repos/os/src/server/lx_fs/main.cc | 13 +++++++++++++ repos/os/src/server/lx_fs/node.h | 2 ++ repos/os/src/server/vfs/main.cc | 8 ++++++-- 7 files changed, 48 insertions(+), 12 deletions(-) diff --git a/repos/os/include/file_system_session/client.h b/repos/os/include/file_system_session/client.h index 01164eee91..f9d94003a8 100644 --- a/repos/os/include/file_system_session/client.h +++ b/repos/os/include/file_system_session/client.h @@ -112,6 +112,11 @@ class File_system::Session_client : public Genode::Rpc_client { call(from_dir, from_name, to_dir, to_name); } + + unsigned num_entries(Dir_handle dir) override + { + return call(dir); + } }; #endif /* _INCLUDE__FILE_SYSTEM_SESSION__CLIENT_H_ */ diff --git a/repos/os/include/file_system_session/file_system_session.h b/repos/os/include/file_system_session/file_system_session.h index c744a53cc8..7e7fca9de5 100644 --- a/repos/os/include/file_system_session/file_system_session.h +++ b/repos/os/include/file_system_session/file_system_session.h @@ -474,6 +474,13 @@ struct File_system::Session : public Genode::Session virtual void move(Dir_handle, Name const &from, Dir_handle, Name const &to) = 0; + /** + * Return number of directory entries + * + * \throw Invalid_handle the directory handle is invalid + */ + virtual unsigned num_entries(Dir_handle) = 0; + /******************* ** RPC interface ** @@ -525,12 +532,14 @@ struct File_system::Session : public Genode::Session GENODE_TYPE_LIST(Invalid_handle, Invalid_name, Lookup_failed, Permission_denied, Unavailable), Dir_handle, Name const &, Dir_handle, Name const &); + GENODE_RPC_THROW(Rpc_num_entries, unsigned, num_entries, + GENODE_TYPE_LIST(Invalid_handle), + Dir_handle); GENODE_RPC_INTERFACE(Rpc_tx_cap, - Rpc_file, Rpc_symlink, Rpc_dir, - Rpc_node, Rpc_watch, + Rpc_file, Rpc_symlink, Rpc_dir, Rpc_node, Rpc_watch, Rpc_close, Rpc_status, Rpc_control, Rpc_unlink, - Rpc_truncate, Rpc_move); + Rpc_truncate, Rpc_move, Rpc_num_entries); }; #endif /* _INCLUDE__FILE_SYSTEM_SESSION__FILE_SYSTEM_SESSION_H_ */ diff --git a/repos/os/src/lib/vfs/fs_file_system.h b/repos/os/src/lib/vfs/fs_file_system.h index 5dfdb687e9..8009ed71ae 100644 --- a/repos/os/src/lib/vfs/fs_file_system.h +++ b/repos/os/src/lib/vfs/fs_file_system.h @@ -754,13 +754,11 @@ class Vfs::Fs_file_system : public File_system path = "/"; try { - ::File_system::Node_handle node = _fs.node(path); - Fs_handle_guard node_guard(*this, _fs, node, + ::File_system::Dir_handle dir = _fs.dir(path, false); + Fs_handle_guard node_guard(*this, _fs, dir, _handle_space, _fs); - ::File_system::Status status = _fs.status(node); - - return status.size / sizeof(::File_system::Directory_entry); + return _fs.num_entries(dir); } catch (...) { } return 0; diff --git a/repos/os/src/server/lx_fs/directory.h b/repos/os/src/server/lx_fs/directory.h index d98bf7d9fc..adc019fbe1 100644 --- a/repos/os/src/server/lx_fs/directory.h +++ b/repos/os/src/server/lx_fs/directory.h @@ -85,7 +85,7 @@ class Lx_fs::Directory : public Node return fd; } - size_t _num_entries() const + unsigned _num_entries() const { unsigned num = 0; @@ -250,7 +250,7 @@ class Lx_fs::Directory : public Node st.st_mtime = 0; return { - .size = _num_entries() * sizeof(File_system::Directory_entry), + .size = 0, .type = Node_type::DIRECTORY, .rwx = { .readable = (st.st_mode & S_IRUSR) != 0, .writeable = (st.st_mode & S_IWUSR) != 0, @@ -260,6 +260,11 @@ class Lx_fs::Directory : public Node }; } + unsigned num_entries() override + { + return _num_entries(); + } + Path path() const override { return _path; diff --git a/repos/os/src/server/lx_fs/main.cc b/repos/os/src/server/lx_fs/main.cc index e93e98550e..2d62bdfa00 100644 --- a/repos/os/src/server/lx_fs/main.cc +++ b/repos/os/src/server/lx_fs/main.cc @@ -441,6 +441,19 @@ class Lx_fs::Session_component : private Session_resources, } } + unsigned num_entries(Dir_handle dir_handle) override + { + auto fn = [&] (Open_node &open_node) { + return open_node.node().num_entries(); + }; + + try { + return _open_node_registry.apply(dir_handle, fn); + } catch (Id_space::Unknown_id const &) { + throw Invalid_handle(); + } + } + void control(Node_handle, Control) override { Genode::error(__func__, " not implemented"); diff --git a/repos/os/src/server/lx_fs/node.h b/repos/os/src/server/lx_fs/node.h index bd8d84d5f0..0322c353c6 100644 --- a/repos/os/src/server/lx_fs/node.h +++ b/repos/os/src/server/lx_fs/node.h @@ -74,6 +74,8 @@ class Lx_fs::Node : public File_system::Node_base virtual Status status() = 0; + virtual unsigned num_entries() { return 0; } + /* * File functionality */ diff --git a/repos/os/src/server/vfs/main.cc b/repos/os/src/server/vfs/main.cc index 15a7d2913a..623bf7a588 100644 --- a/repos/os/src/server/vfs/main.cc +++ b/repos/os/src/server/vfs/main.cc @@ -660,8 +660,6 @@ class Vfs_server::Session_component : private Session_resources, { switch (vfs_stat.type) { case Vfs::Node_type::DIRECTORY: - return _vfs.num_dirent(node.path()) * sizeof(Directory_entry); - case Vfs::Node_type::SYMLINK: return 0ULL; @@ -688,6 +686,12 @@ class Vfs_server::Session_component : private Session_resources, return fs_stat; } + unsigned num_entries(Dir_handle dir_handle) override + { + return _apply(dir_handle, [&] (Directory &dir) { + return (unsigned)_vfs.num_dirent(dir.path()); }); + } + void unlink(Dir_handle dir_handle, Name const &name) override { if (!_writeable) throw Permission_denied();