diff --git a/repos/base/include/base/entrypoint.h b/repos/base/include/base/entrypoint.h index 492633bd9e..71b4849ade 100644 --- a/repos/base/include/base/entrypoint.h +++ b/repos/base/include/base/entrypoint.h @@ -5,7 +5,7 @@ */ /* - * Copyright (C) 2015-2017 Genode Labs GmbH + * Copyright (C) 2015-2019 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. @@ -31,8 +31,28 @@ class Genode::Entrypoint : Noncopyable { public: + /** + * Functor for post I/O signal progress handling + * + * This mechanism is for processing I/O events + * deferred during signal dispatch. This is the + * case when the application is blocked by I/O + * but should not be resumed during signal + * dispatch. + */ + struct Io_progress_handler : Interface + { + virtual ~Io_progress_handler() { + error("Io_progress_handler subclass cannot be safely destroyed!"); } + + virtual void handle_io_progress() = 0; + }; + /** * Functor for post signal-handler hook + * + * \deprecated + * \noapi */ struct Post_signal_hook : Interface { virtual void function() = 0; }; @@ -96,7 +116,15 @@ class Genode::Entrypoint : Noncopyable int _signal_recipient { NONE }; Genode::Lock _signal_pending_lock { }; Genode::Lock _signal_pending_ack_lock { }; - Post_signal_hook *_post_signal_hook = nullptr; + + Io_progress_handler *_io_progress_handler { nullptr }; + Post_signal_hook *_post_signal_hook { nullptr }; + + void _handle_io_progress() + { + if (_io_progress_handler != nullptr) + _io_progress_handler->handle_io_progress(); + } void _execute_post_signal_hook() { @@ -220,8 +248,23 @@ class Genode::Entrypoint : Noncopyable */ void schedule_suspend(void (*suspended)(), void (*resumed)()); + /** + * Register hook functor to be called after I/O signals are dispatched + */ + void register_io_progress_handler(Io_progress_handler &handler) + { + if (_io_progress_handler != nullptr) { + error("cannot call ", __func__, " twice!"); + throw Exception(); + } + _io_progress_handler = &handler; + } + /** * Register hook functor to be called after signal was handled + * + * \deprecated + * \noapi */ void schedule_post_signal_hook(Post_signal_hook *hook) { diff --git a/repos/base/src/lib/base/entrypoint.cc b/repos/base/src/lib/base/entrypoint.cc index 09167c3330..a0cebfa58d 100644 --- a/repos/base/src/lib/base/entrypoint.cc +++ b/repos/base/src/lib/base/entrypoint.cc @@ -6,7 +6,7 @@ */ /* - * Copyright (C) 2015-2017 Genode Labs GmbH + * Copyright (C) 2015-2019 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. @@ -43,14 +43,30 @@ static char const *initial_ep_name() { return "ep"; } void Entrypoint::Signal_proxy_component::signal() { - /* XXX introduce while-pending loop */ + ep._process_deferred_signals(); + + bool io_progress = false; + + /* + * Try to dispatch one pending signal picked-up by the signal-proxy thread. + * Note, we handle only one signal here to ensure fairness between RPCs and + * signals. + */ try { Signal sig = ep._sig_rec->pending_signal(); ep._dispatch_signal(sig); + + if (sig.context()->level() == Signal_context::Level::Io) { + /* trigger the progress handler */ + io_progress = true; + + /* execute deprecated per-signal hook */ + ep._execute_post_signal_hook(); + } } catch (Signal_receiver::Signal_not_pending) { } - ep._execute_post_signal_hook(); - ep._process_deferred_signals(); + if (io_progress) + ep._handle_io_progress(); } @@ -198,6 +214,7 @@ bool Entrypoint::_wait_and_dispatch_one_io_signal(bool const dont_block) } _dispatch_signal(sig); + _execute_post_signal_hook(); break; } catch (Signal_receiver::Signal_not_pending) { @@ -211,7 +228,7 @@ bool Entrypoint::_wait_and_dispatch_one_io_signal(bool const dont_block) } } - _execute_post_signal_hook(); + _handle_io_progress(); /* initiate potential deferred-signal handling in entrypoint */ if (_deferred_signals.first()) { diff --git a/repos/libports/src/lib/libc/task.cc b/repos/libports/src/lib/libc/task.cc index 771c550e9f..9495bab24d 100644 --- a/repos/libports/src/lib/libc/task.cc +++ b/repos/libports/src/lib/libc/task.cc @@ -6,7 +6,7 @@ */ /* - * Copyright (C) 2016-2018 Genode Labs GmbH + * Copyright (C) 2016-2019 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. @@ -20,8 +20,7 @@ #include #include #include -#include -#include +#include #include /* libc includes */ @@ -40,60 +39,17 @@ extern char **environ; namespace Libc { class Env_implementation; - class Vfs_env; class Kernel; class Pthreads; class Timer; class Timer_accessor; class Timeout; class Timeout_handler; - class Io_response_handler; using Genode::Microseconds; } -class Libc::Vfs_env : public Vfs::Env -{ - private: - - Genode::Env &_env; - Genode::Allocator &_alloc; - - Vfs::Io_response_handler &_io_handler; - - struct Watch_response_stub : Vfs::Watch_response_handler { - void handle_watch_response(Vfs::Vfs_watch_handle::Context*) override { }; - } _watch_stub { }; - - Vfs::Global_file_system_factory _global_file_system_factory { _alloc }; - - Vfs::Dir_file_system _root_dir; - - public: - - Vfs_env(Genode::Env &env, - Genode::Allocator &alloc, - Genode::Xml_node config, - Vfs::Io_response_handler &io_handler) - : _env(env), _alloc(alloc), _io_handler(io_handler), - _root_dir(*this, config, _global_file_system_factory) - { } - - Genode::Env &env() override { return _env; } - - Genode::Allocator &alloc() override { return _alloc; } - - Vfs::File_system &root_dir() override { return _root_dir; } - - Vfs::Io_response_handler &io_handler() override { - return _io_handler; } - - Vfs::Watch_response_handler &watch_handler() override { - return _watch_stub; } -}; - - class Libc::Env_implementation : public Libc::Env { private: @@ -126,20 +82,15 @@ class Libc::Env_implementation : public Libc::Env return Genode::Xml_node(""); } - Vfs::Global_file_system_factory _file_system_factory; - Vfs_env _vfs_env; + Vfs::Simple_env _vfs_env; Genode::Xml_node _config_xml() const override { return _config.xml(); }; public: - Env_implementation(Genode::Env &env, Genode::Allocator &alloc, - Vfs::Io_response_handler &io_response_handler) - : - _env(env), _file_system_factory(alloc), - _vfs_env(_env, alloc, _vfs_config(), io_response_handler) - { } + Env_implementation(Genode::Env &env, Genode::Allocator &alloc) + : _env(env), _vfs_env(_env, alloc, _vfs_config()) { } /************************* @@ -371,19 +322,6 @@ struct Libc::Pthreads extern void (*libc_select_notify)(); -struct Libc::Io_response_handler : Vfs::Io_response_handler -{ - void handle_io_response(Vfs::Vfs_handle::Context *) override - { - /* some contexts may have been deblocked from select() */ - if (libc_select_notify) - libc_select_notify(); - - /* resume all as any context may have been deblocked from blocking I/O */ - Libc::resume_all(); - } -}; - /* internal utility */ static void resumed_callback(); @@ -400,14 +338,14 @@ static void suspended_callback(); * secondary stack for the application task. Context switching uses * setjmp/longjmp. */ -struct Libc::Kernel +struct Libc::Kernel final : Genode::Entrypoint::Io_progress_handler { private: Genode::Env &_env; Genode::Allocator &_heap; - Io_response_handler _io_response_handler; - Env_implementation _libc_env { _env, _heap, _io_response_handler }; + + Env_implementation _libc_env { _env, _heap }; Vfs_plugin _vfs { _libc_env, _heap }; Genode::Reconstructible> _resume_main_handler { @@ -619,7 +557,10 @@ struct Libc::Kernel public: Kernel(Genode::Env &env, Genode::Allocator &heap) - : _env(env), _heap(heap) { } + : _env(env), _heap(heap) + { + _env.ep().register_io_progress_handler(*this); + } ~Kernel() { Genode::error(__PRETTY_FUNCTION__, " should not be executed!"); } @@ -821,6 +762,22 @@ struct Libc::Kernel _switch_to_user(); } } + + /** + * Entrypoint::Io_progress_handler interface + */ + void handle_io_progress() override + { + /* some contexts may have been deblocked from select() */ + if (libc_select_notify) + libc_select_notify(); + + /* + * resume all as any VFS context may have + * been deblocked from blocking I/O + */ + Kernel::resume_all(); + } }; diff --git a/repos/os/src/lib/vfs/fs_file_system.h b/repos/os/src/lib/vfs/fs_file_system.h index ce54df06b7..78586f850c 100644 --- a/repos/os/src/lib/vfs/fs_file_system.h +++ b/repos/os/src/lib/vfs/fs_file_system.h @@ -370,8 +370,7 @@ class Vfs::Fs_file_system : public File_system Io_response_handler &_io_handler; Watch_response_handler &_watch_handler; List _context_list { }; - List - _watch_context_list { }; + Lock _list_lock { }; bool _notify_all { false }; @@ -406,27 +405,6 @@ class Vfs::Fs_file_system : public File_system _ep.schedule_post_signal_hook(this); } - void arm_watch_event(Vfs_watch_handle::Context &context) - { - { - Lock::Guard list_guard(_list_lock); - - for (Vfs_watch_handle::Context *list_context = _watch_context_list.first(); - list_context; - list_context = list_context->next()) - { - if (list_context == &context) { - /* already in list */ - return; - } - } - - _watch_context_list.insert(&context); - } - - _ep.schedule_post_signal_hook(this); - } - void function() override { Vfs_handle::Context *context = nullptr; @@ -451,18 +429,6 @@ class Vfs::Fs_file_system : public File_system /* done if no contexts and all notified */ } while (context); - - for (;;) { - Vfs_watch_handle::Context *context = nullptr; - { - Lock::Guard list_guard(_list_lock); - - context = _watch_context_list.first(); - if (!context) break; - _watch_context_list.remove(context); - _watch_handler.handle_watch_response(context); - } - } } }; @@ -609,9 +575,14 @@ class Vfs::Fs_file_system : public File_system try { if (packet.operation() == Packet_descriptor::CONTENT_CHANGED) { + /* + * Trigger the watch response during signal dispatch. + * This is incompatible with the Libc I/O handling + * but the Libc does not open watch handles and shall + * not use them before Post_signal_hook is removed. + */ _watch_handle_space.apply(id, [&] (Fs_vfs_watch_handle &handle) { - if (auto *ctx = handle.context()) - _post_signal_hook.arm_watch_event(*ctx); }); + _env.watch_handler().handle_watch_response(handle.context()); }); } else { _handle_space.apply(id, handle_read); } diff --git a/repos/os/src/server/vfs/main.cc b/repos/os/src/server/vfs/main.cc index a574be13ae..15df90aa6e 100644 --- a/repos/os/src/server/vfs/main.cc +++ b/repos/os/src/server/vfs/main.cc @@ -712,8 +712,9 @@ class Vfs_server::Session_component : private Session_resources, void control(Node_handle, Control) override { } }; + /** - * Global I/O event handler + * Global VFS event handler */ struct Vfs_server::Io_response_handler : Vfs::Io_response_handler { @@ -751,6 +752,7 @@ struct Vfs_server::Io_response_handler : Vfs::Io_response_handler } }; + /** * Global VFS watch handler */ @@ -822,6 +824,34 @@ class Vfs_server::Root : public Genode::Root_component Vfs_env _vfs_env { _env, vfs_config(), _session_registry }; + /** + * Global I/O event handler + * + * This is a safe and slow intermediate implementation + * to be replaced with one that only processes handles + * and sessions that await progress. + */ + struct Io_progress_handler : Genode::Entrypoint::Io_progress_handler + { + Session_registry &_session_registry; + + Io_progress_handler(Genode::Entrypoint &ep, + Session_registry &session_registry) + : _session_registry(session_registry) + { + ep.register_io_progress_handler(*this); + } + + /** + * Entrypoint::Io_progress_handler interface + */ + void handle_io_progress() override + { + _session_registry.for_each([ ] (Registered_session &r) { + r.handle_general_io(); }); + } + } _io_progress_handler { _env.ep(), _session_registry }; + Genode::Signal_handler _config_handler { _env.ep(), *this, &Root::_config_update };