From 4a3fc21adaea148dccef2e09a8c702b74f6c5166 Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Mon, 12 Feb 2018 13:12:27 +0100 Subject: [PATCH] New watch handle mechanism for File_system session File_system clients may now watch files and directories for changes by opening a 'Watch_handle' rather than submitting a 'CONTENT_CHANGED' packet to the server. When a change happens at a node with an open Watch_handle a CONTENT_CHANGED packet will be sent from the server to the client. This serializes registration with other handle operations and separates I/O handle state from notification handle state. Test at run/fs_rom_update. Ref #1934 --- repos/os/include/file_system_session/client.h | 7 +- .../include/file_system_session/connection.h | 8 +- .../file_system_session/file_system_session.h | 35 +- .../include/file_system_session/rpc_object.h | 7 + repos/os/run/fs_rom_update.run | 8 +- repos/os/src/server/fs_rom/main.cc | 356 +++++++++--------- repos/os/src/server/ram_fs/main.cc | 24 +- 7 files changed, 242 insertions(+), 203 deletions(-) diff --git a/repos/os/include/file_system_session/client.h b/repos/os/include/file_system_session/client.h index 4a15352857..cdb2e0b13d 100644 --- a/repos/os/include/file_system_session/client.h +++ b/repos/os/include/file_system_session/client.h @@ -5,7 +5,7 @@ */ /* - * Copyright (C) 2012-2017 Genode Labs GmbH + * Copyright (C) 2012-2018 Genode Labs GmbH * * This file is part of the Genode OS framework, which is distributed * under the terms of the GNU Affero General Public License version 3. @@ -81,6 +81,11 @@ class File_system::Session_client : public Genode::Rpc_client return call(path); } + Watch_handle watch(Path const &path) override + { + return call(path); + } + void close(Node_handle node) override { call(node); diff --git a/repos/os/include/file_system_session/connection.h b/repos/os/include/file_system_session/connection.h index 5833e4b466..5b7834889d 100644 --- a/repos/os/include/file_system_session/connection.h +++ b/repos/os/include/file_system_session/connection.h @@ -5,7 +5,7 @@ */ /* - * Copyright (C) 2012-2017 Genode Labs GmbH + * Copyright (C) 2012-2018 Genode Labs GmbH * * This file is part of the Genode OS framework, which is distributed * under the terms of the GNU Affero General Public License version 3. @@ -152,6 +152,12 @@ struct File_system::Connection : File_system::Connection_base return _retry([&] () { return Session_client::node(path); }); } + + Watch_handle watch(Path const &path) override + { + return _retry([&] () { + return Session_client::watch(path); }); + } }; #endif /* _INCLUDE__FILE_SYSTEM_SESSION__CONNECTION_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 70d68e9eb3..8e6debf13d 100644 --- a/repos/os/include/file_system_session/file_system_session.h +++ b/repos/os/include/file_system_session/file_system_session.h @@ -7,7 +7,7 @@ */ /* - * Copyright (C) 2012-2017 Genode Labs GmbH + * Copyright (C) 2012-2018 Genode Labs GmbH * * This file is part of the Genode OS framework, which is distributed * under the terms of the GNU Affero General Public License version 3. @@ -52,10 +52,19 @@ namespace File_system { }; }; + struct Watch : Node + { + struct Id : Node::Id + { + explicit Id(unsigned long value) : Node::Id { value } { }; + }; + }; + typedef Node::Id Node_handle; typedef File::Id File_handle; typedef Directory::Id Dir_handle; typedef Symlink::Id Symlink_handle; + typedef Watch::Id Watch_handle; using Genode::size_t; @@ -337,6 +346,23 @@ struct File_system::Session : public Genode::Session */ virtual Node_handle node(Path const &path) = 0; + /** + * Watch a node for for changes. + * + * When changes are made to the node at this path a CONTENT_CHANGED + * packet will be sent from the server to the client. + * + * \throw Lookup_failed path lookup failed because one element + * of 'path' does not exist + * \throw Out_of_ram server cannot allocate metadata + * \throw Out_of_caps + * \throw Unavailable file-system is static or does not support + * notifications + * + * The returned node handle is used to identify notification packets. + */ + virtual Watch_handle watch(Path const &path) = 0; + /** * Close file * @@ -421,6 +447,9 @@ struct File_system::Session : public Genode::Session GENODE_RPC_THROW(Rpc_node, Node_handle, node, GENODE_TYPE_LIST(Lookup_failed, Out_of_ram, Out_of_caps), Path const &); + GENODE_RPC_THROW(Rpc_watch, Watch_handle, watch, + GENODE_TYPE_LIST(Lookup_failed, Out_of_ram, Out_of_caps, Unavailable), + Path const &); GENODE_RPC_THROW(Rpc_close, void, close, GENODE_TYPE_LIST(Invalid_handle), Node_handle); @@ -444,7 +473,9 @@ struct File_system::Session : public Genode::Session Lookup_failed, Permission_denied, Unavailable), Dir_handle, Name const &, Dir_handle, Name const &); - GENODE_RPC_INTERFACE(Rpc_tx_cap, Rpc_file, Rpc_symlink, Rpc_dir, Rpc_node, + GENODE_RPC_INTERFACE(Rpc_tx_cap, + Rpc_file, Rpc_symlink, Rpc_dir, + Rpc_node, Rpc_watch, Rpc_close, Rpc_status, Rpc_control, Rpc_unlink, Rpc_truncate, Rpc_move); }; diff --git a/repos/os/include/file_system_session/rpc_object.h b/repos/os/include/file_system_session/rpc_object.h index a0423fd98e..d397c0ffc3 100644 --- a/repos/os/include/file_system_session/rpc_object.h +++ b/repos/os/include/file_system_session/rpc_object.h @@ -49,6 +49,13 @@ class File_system::Session_rpc_object : public Genode::Rpc_object _tx_cap() { return _tx.cap(); } Tx::Sink *tx_sink() { return _tx.sink(); } + + /** + * Default stub implementation + */ + Watch_handle watch(Path const &) override { + throw Unavailable(); } + }; #endif /* _INCLUDE__FILE_SYSTEM_SESSION__SERVER_H_ */ diff --git a/repos/os/run/fs_rom_update.run b/repos/os/run/fs_rom_update.run index ca018c6185..2ef71a6094 100644 --- a/repos/os/run/fs_rom_update.run +++ b/repos/os/run/fs_rom_update.run @@ -1,7 +1,3 @@ -if {[have_spec arm]} { - assert_spec arm_v7 -} - # # Build # @@ -115,6 +111,6 @@ set boot_modules { build_boot_image $boot_modules -append qemu_args " -nographic " +append qemu_args " -nographic" -run_genode_until forever +run_genode_until {.*.*} 60 diff --git a/repos/os/src/server/fs_rom/main.cc b/repos/os/src/server/fs_rom/main.cc index 7b23170269..1c330497ba 100755 --- a/repos/os/src/server/fs_rom/main.cc +++ b/repos/os/src/server/fs_rom/main.cc @@ -1,11 +1,12 @@ /* * \brief Service that provides files of a file system as ROM sessions * \author Norman Feske + * \author Emery Hemingway * \date 2013-01-11 */ /* - * Copyright (C) 2013-2017 Genode Labs GmbH + * Copyright (C) 2013-2018 Genode Labs GmbH * * This file is part of the Genode OS framework, which is distributed * under the terms of the GNU Affero General Public License version 3. @@ -51,6 +52,7 @@ class Fs_rom::Rom_session_component : public Rpc_object, private: friend class List; + friend class Packet_handler; Env &_env; @@ -65,9 +67,14 @@ class Fs_rom::Rom_session_component : public Rpc_object, Path const _file_path; /** - * Handle of associated file + * Wandering notification handle */ - Constructible _file_handle { }; + Constructible _watch_handle { }; + + /** + * Handle of associated file opened during read loop + */ + File_system::File_handle _file_handle { ~0UL }; /** * Size of current version of the file @@ -79,14 +86,6 @@ class Fs_rom::Rom_session_component : public Rpc_object, */ File_system::seek_off_t _file_seek = 0; - /** - * Handle of currently watched compound directory - * - * The compund directory is watched only if the requested file could - * not be looked up. - */ - Constructible _compound_dir_handle { }; - /** * Dataspace exposed as ROM module to the client */ @@ -97,11 +96,6 @@ class Fs_rom::Rom_session_component : public Rpc_object, */ Signal_context_capability _sigh { }; - /* - * Exception - */ - struct Open_compound_dir_failed { }; - /* * Version number used to track the need for ROM update notifications */ @@ -110,193 +104,126 @@ class Fs_rom::Rom_session_component : public Rpc_object, Version _handed_out_version { ~0U }; /** - * Open compound directory of specified file - * - * \param walk_up If set to true, the function tries to walk up the - * hierarchy towards the root and returns the first - * existing directory on the way. If set to false, the - * function returns the immediate compound directory. + * Track if the session file or a directory is being watched */ - File_system::Dir_handle _open_compound_dir(File_system::Session &fs, - Path const &path, - bool walk_up) - { - using namespace File_system; - - Path dir_path(path.base()); - - while (!path.equals("/")) { - - dir_path.strip_last_element(); - - try { return fs.dir(dir_path.base(), false); } - - catch (Invalid_handle) { error(_file_path, ": invalid_handle"); } - catch (Invalid_name) { error(_file_path, ": invalid_name"); } - catch (Lookup_failed) { error(_file_path, ": lookup_failed"); } - catch (Permission_denied) { error(_file_path, ": permission_denied"); } - catch (Name_too_long) { error(_file_path, ": name_too_long"); } - catch (No_space) { error(_file_path, ": no_space"); } - - /* - * If the directory could not be opened, walk up the hierarchy - * towards the root and try again. - */ - if (!walk_up) break; - } - throw Open_compound_dir_failed(); - } + bool _watching_file = false; /* * Exception */ - struct Open_file_failed { }; + struct Watch_failed { }; /** - * Open file with specified name at the file system + * Watch the session ROM file or some parent directory */ - File_system::File_handle _open_file(File_system::Session &fs, - Path const &path) + void _open_watch_handle() { using namespace File_system; + _close_watch_handle(); + + Path watch_path(_file_path); + + /* track if we can open the file or resort to a parent */ + bool at_the_file = true; + try { + while (true) { + try { + _watch_handle.construct(_fs.watch(watch_path.base())); + _watching_file = at_the_file; + return; + } + catch (File_system::Lookup_failed) { } + catch (File_system::Unavailable) { } - Dir_handle dir = _open_compound_dir(fs, path, false); + if (watch_path == "/") + throw Watch_failed(); + watch_path.strip_last_element(); - try { - - Handle_guard guard(fs, dir); - - /* open file */ - Path file_name(path.base()); - file_name.keep_only_last_element(); - return fs.file(dir, file_name.base() + 1, - File_system::READ_ONLY, false); + /* keep looping, but only directories will be opened */ + at_the_file = false; } - catch (Invalid_handle) { error(_file_path, ": Invalid_handle"); } - catch (Invalid_name) { error(_file_path, ": invalid_name"); } - catch (Lookup_failed) { error(_file_path, ": lookup_failed"); } - catch (Permission_denied) { error(_file_path, ": Permission_denied"); } - catch (...) { error(_file_path, ": unhandled error"); }; - - throw Open_file_failed(); - - } catch (Open_compound_dir_failed) { - throw Open_file_failed(); + } catch (File_system::Out_of_ram) { + error("not enough RAM to watch '", watch_path, "'"); + } catch (File_system::Out_of_caps) { + error("not enough caps to watch '", watch_path, "'"); } + throw Watch_failed(); } - void _register_for_compound_dir_changes() + void _close_watch_handle() { - using namespace File_system; - - /* forget about the previously watched compound directory */ - if (_compound_dir_handle.constructed()) { - _fs.close(*_compound_dir_handle); - _compound_dir_handle.destruct(); + if (_watch_handle.constructed()) { + _fs.close(*_watch_handle); + _watch_handle.destruct(); } - - try { - _compound_dir_handle.construct(_open_compound_dir(_fs, _file_path, true)); - - /* register for changes in compound directory */ - _fs.tx()->submit_packet(File_system::Packet_descriptor( - *_compound_dir_handle, - File_system::Packet_descriptor::CONTENT_CHANGED)); - } - catch (Open_compound_dir_failed) { - warning("could not track compound dir, giving up"); } + _watching_file = false; } + enum { UPDATE_OR_REPLACE = false, UPDATE_ONLY = true }; + /** - * Initialize '_file_ds' dataspace with file content + * Fill dataspace with file content, return true if the + * current dataspace is reused. */ - void _update_dataspace() + bool _read_dataspace(bool update_only) { using namespace File_system; - /* - * On each repeated call of this function, the dataspace is - * replaced with a new one that contains the most current file - * content. The dataspace is re-allocated if the new version - * of the file has become bigger. - */ - try { - File_handle const file_handle = _open_file(_fs, _file_path); - File_system::file_size_t const new_file_size = - _fs.status(file_handle).size; + Genode::Path dir_path(_file_path); + dir_path.strip_last_element(); + Genode::Path file_name(_file_path); + file_name.keep_only_last_element(); - if (_file_ds.size() && (new_file_size > _file_size)) { - /* mark as invalid */ - _file_ds.realloc(&_env.ram(), 0); - _file_size = 0; - _file_seek = 0; - } - _fs.close(file_handle); - } catch (Open_file_failed) { } + Dir_handle parent_handle = _fs.dir(dir_path.base(), false); + Handle_guard parent_guard(_fs, parent_handle); - /* close and then re-open the file */ - if (_file_handle.constructed()) { - _fs.close(*_file_handle); - _file_handle.destruct(); - } + /* the file handle is opened here... */ + _file_handle = _fs.file( + parent_handle, file_name.base() + 1, + File_system::READ_ONLY, false); + Handle_guard file_guard(_fs, _file_handle); + /* ...but only for the lifetime of this procedure */ - try { - _file_handle.construct(_open_file(_fs, _file_path)); - } catch (Open_file_failed) { } + _file_seek = 0; + _file_size = _fs.status(_file_handle).size; - /* - * If we got the file, we can stop paying attention to the - * compound directory. - */ - if (_file_handle.constructed() && _compound_dir_handle.constructed()) { - _fs.close(*_compound_dir_handle); - _compound_dir_handle.destruct(); - } + if (_file_size > _file_ds.size()) { + /* allocate new RAM dataspace according to file size */ + if (update_only) + return false; - /* register for file changes */ - if (_file_handle.constructed()) - _fs.tx()->submit_packet(File_system::Packet_descriptor( - *_file_handle, File_system::Packet_descriptor::CONTENT_CHANGED)); - - size_t const file_size = _file_handle.constructed() - ? _fs.status(*_file_handle).size : 0; - - /* allocate new RAM dataspace according to file size */ - if (file_size > 0) { try { - _file_seek = 0; - _file_ds.realloc(&_env.ram(), file_size); - _file_size = file_size; + _file_ds.realloc(&_env.ram(), _file_size); } catch (...) { - error("couldn't allocate memory for file, empty result"); - return; + error("failed to allocate memory for ", _file_path); + return false; } } else { - _register_for_compound_dir_changes(); - return; + memset(_file_ds.local_addr(), 0x00, _file_ds.size()); } /* omit read if file is empty */ if (_file_size == 0) - return; + return false; /* read content from file */ Tx_source &source = *_fs.tx(); while (_file_seek < _file_size) { /* if we cannot submit then process acknowledgements */ - if (source.ready_to_submit()) { - size_t chunk_size = min(_file_size - _file_seek, - source.bulk_buffer_size() / 2); - File_system::Packet_descriptor - packet(source.alloc_packet(chunk_size), - *_file_handle, - File_system::Packet_descriptor::READ, - chunk_size, - _file_seek); - source.submit_packet(packet); - } + while (!source.ready_to_submit()) + _env.ep().wait_and_dispatch_one_io_signal(); + + size_t chunk_size = min(_file_size - _file_seek, + source.bulk_buffer_size() / 2); + + File_system::Packet_descriptor + packet(source.alloc_packet(chunk_size), _file_handle, + File_system::Packet_descriptor::READ, + chunk_size, _file_seek); + + source.submit_packet(packet); /* * Process the global signal handler until we got a response @@ -307,12 +234,45 @@ class Fs_rom::Rom_session_component : public Rpc_object, while (_file_seek == orig_file_seek) _env.ep().wait_and_dispatch_one_io_signal(); } + + _handed_out_version = _curr_version; + return true; + } + + bool _try_read_dataspace(bool update_only) + { + using namespace File_system; + + try { _open_watch_handle(); } + catch (Watch_failed) { } + + try { return _read_dataspace(update_only); } + catch (Lookup_failed) { log(_file_path, " ROM file is missing"); } + catch (Invalid_handle) { error(_file_path, ": invalid handle"); } + catch (Invalid_name) { error(_file_path, ": invalid name"); } + catch (Permission_denied) { error(_file_path, ": permission denied"); } + catch (...) { error(_file_path, ": unhandled error"); }; + + return false; } void _notify_client_about_new_version() { - if (_sigh.valid() && _curr_version.value != _handed_out_version.value) - Signal_transmitter(_sigh).submit(); + using namespace File_system; + + if (_sigh.valid() && _curr_version.value != _handed_out_version.value) { + + /* notify if the file is not empty */ + try { + Node_handle file = _fs.node(_file_path.base()); + Handle_guard g(_fs, file); + _file_size = _fs.status(file).size; + } + catch (...) { _file_size = 0; } + + if (_file_size > 0) + Signal_transmitter(_sigh).submit(); + } } public: @@ -328,17 +288,15 @@ class Fs_rom::Rom_session_component : public Rpc_object, * creation time) */ Rom_session_component(Env &env, - File_system::Session &fs, const char *file_path) + File_system::Session &fs, + const char *file_path) : _env(env), _fs(fs), _file_path(file_path), _file_ds(env.ram(), env.rm(), 0) /* realloc later */ { - try { - _file_handle.construct(_open_file(_fs, _file_path)); - } catch (Open_file_failed) { } - - _register_for_compound_dir_changes(); + try { _open_watch_handle(); } + catch (Watch_failed) { } } /** @@ -346,33 +304,45 @@ class Fs_rom::Rom_session_component : public Rpc_object, */ ~Rom_session_component() { - /* close re-open the file */ - if (_file_handle.constructed()) - _fs.close(*_file_handle); - - if (_compound_dir_handle.constructed()) - _fs.close(*_compound_dir_handle); + _close_watch_handle(); } - using Sessions::Element::next; - /** * Return dataspace with up-to-date content of file */ Rom_dataspace_capability dataspace() { - _update_dataspace(); + using namespace File_system; + + _try_read_dataspace(UPDATE_OR_REPLACE); + + /* always serve a valid, even empty, dataspace */ + if (_file_ds.size() < 1) { + _file_ds.realloc(&_env.ram(), 1); + } + Dataspace_capability ds = _file_ds.cap(); - _handed_out_version = _curr_version; return static_cap_cast(ds); } void sigh(Signal_context_capability sigh) { _sigh = sigh; + + if (_sigh.valid()) { + try { _open_watch_handle(); } + catch (Watch_failed) { } + } + _notify_client_about_new_version(); } + /** + * Update the current dataspace content + */ + bool update() override { + return _try_read_dataspace(UPDATE_ONLY); } + /** * If packet corresponds to this session then process and return true. * @@ -383,25 +353,30 @@ class Fs_rom::Rom_session_component : public Rpc_object, switch (packet.operation()) { case File_system::Packet_descriptor::CONTENT_CHANGED: + if (!(packet.handle() == *_watch_handle)) + return false; - _curr_version = Version { _curr_version.value + 1 }; - - if ((_file_handle.constructed() && (*_file_handle == packet.handle())) || - (_compound_dir_handle.constructed() && (*_compound_dir_handle == packet.handle()))) - { - _notify_client_about_new_version(); - return true; + if (!_watching_file) { + /* try and get closer to the file */ + _open_watch_handle(); } - return false; + + if (_watching_file) { + /* notify the client of the change */ + _curr_version = Version { _curr_version.value + 1 }; + _notify_client_about_new_version(); + } + return true; case File_system::Packet_descriptor::READ: { - if (!(_file_handle.constructed() && (*_file_handle == packet.handle()))) + if (!(packet.handle() == _file_handle)) return false; if (packet.position() > _file_seek || _file_seek >= _file_size) { error("bad packet seek position"); _file_ds.realloc(&_env.ram(), 0); + _file_seek = 0; return true; } @@ -412,9 +387,14 @@ class Fs_rom::Rom_session_component : public Rpc_object, return true; } - default: - - error("discarding strange packet acknowledgement"); + case File_system::Packet_descriptor::WRITE: + warning("discarding strange WRITE acknowledgement"); + return true; + case File_system::Packet_descriptor::SYNC: + warning("discarding strange SYNC acknowledgement"); + return true; + case File_system::Packet_descriptor::READ_READY: + warning("discarding strange READ_READY acknowledgement"); return true; } return false; diff --git a/repos/os/src/server/ram_fs/main.cc b/repos/os/src/server/ram_fs/main.cc index 8422de99a6..54cf54bf07 100644 --- a/repos/os/src/server/ram_fs/main.cc +++ b/repos/os/src/server/ram_fs/main.cc @@ -97,11 +97,7 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object break; case Packet_descriptor::CONTENT_CHANGED: { - open_node.register_notify(*tx_sink()); - Locked_ptr node { open_node.node() }; - if (!node.valid()) - return; - node->notify_listeners(); + Genode::error("CONTENT_CHANGED packets from clients have no effect"); return; } @@ -365,6 +361,24 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object return open_node->id(); } + Watch_handle watch(Path const &path) override + { + _assert_valid_path(path.string()); + + Node *node = _root.lookup(path.string() + 1); + + Open_node *watcher = new (_alloc) + Open_node(node->weak_ptr(), _open_node_registry); + + /* + * like other open nodes, just the only + * kind registered for notifications + */ + watcher->register_notify(*tx_sink()); + + return Watch_handle { watcher->id().value }; + } + void close(Node_handle handle) { auto close_fn = [&] (Open_node &open_node) {