diff --git a/repos/libports/src/lib/libc/internal/init.h b/repos/libports/src/lib/libc/internal/init.h index 91d5cde8ec..5e85ca8493 100644 --- a/repos/libports/src/lib/libc/internal/init.h +++ b/repos/libports/src/lib/libc/internal/init.h @@ -72,7 +72,7 @@ namespace Libc { /** * Select support */ - void init_select(Suspend &, Resume &, Select &, Signal &); + void init_select(Select &, Signal &, Monitor &); /** * Support for querying available RAM quota in sysctl functions diff --git a/repos/libports/src/lib/libc/internal/kernel.h b/repos/libports/src/lib/libc/internal/kernel.h index e6ad419c52..1bb1db151c 100644 --- a/repos/libports/src/lib/libc/internal/kernel.h +++ b/repos/libports/src/lib/libc/internal/kernel.h @@ -209,7 +209,7 @@ struct Libc::Kernel final : Vfs::Io_response_handler, bool _dispatch_pending_io_signals = false; /* io_progress_handler marker */ - bool _io_ready { false }; + bool _io_progressed { false }; Thread &_myself { *Thread::myself() }; @@ -285,11 +285,15 @@ struct Libc::Kernel final : Vfs::Io_response_handler, Reconstructible> _execute_monitors { _env.ep(), *this, &Kernel::_monitors_handler }; - bool _execute_monitors_pending { false }; + Monitor::Pool::State _execute_monitors_pending = Monitor::Pool::State::ALL_COMPLETE; + + Constructible _main_monitor_job { }; void _monitors_handler() { - /* used to leave I/O-signal dispatcher only - handled afterwards */ + /* mark monitors for execution when running in kernel only */ + _execute_monitors_pending = Monitor::Pool::State::JOBS_PENDING; + _io_progressed = true; } Constructible _clone_connection { }; @@ -440,29 +444,86 @@ struct Libc::Kernel final : Vfs::Io_response_handler, /* the 'kernel_routine' may install another kernel routine */ _kernel_routine = nullptr; routine.execute_in_kernel(); - if (!_kernel_routine) + + if (!_kernel_routine) { _switch_to_user(); - } + } - if (_execute_monitors_pending) { - - _execute_monitors_pending = false; - _monitors.execute_monitors(); - - } else { - - if (_dispatch_pending_io_signals) { - /* dispatch pending signals but don't block */ - while (_env.ep().dispatch_pending_io_signal()) ; - } else { - /* block for signals */ + if (_kernel_routine) { _env.ep().wait_and_dispatch_one_io_signal(); - handle_io_progress(); } } - if (!_kernel_routine && _resume_main_once && !_setjmp(_kernel_context)) + /* + * Dispatch all pending I/O signals at once and execute + * monitors that may now become able to complete. + */ + auto dispatch_all_pending_io_signals = [&] () + { + while (_env.ep().dispatch_pending_io_signal()); + }; + + dispatch_all_pending_io_signals(); + + if (_io_progressed) + Kernel::resume_all(); + + _io_progressed = false; + + /* + * Execute monitors on kernel entry regardless of any I/O + * because the monitor function may be unrelated to I/O. + */ + if (_execute_monitors_pending == Monitor::Pool::State::JOBS_PENDING) + _execute_monitors_pending = _monitors.execute_monitors(); + + /* + * Process I/O signals without returning to the application + * as long as the main thread depends on I/O. + */ + + auto main_blocked_in_monitor = [&] () + { + /* + * In general, 'resume_all()' only flags the main state but + * does not alter the main monitor job. For exmaple in case + * of a sleep timeout, main is resumed by 'resume_main()' + * in 'Main_blockade::wakeup()' but did not yet return from + * 'suspend()'. The expired state in the main job is set + * only after 'suspend()' returned. + */ + if (_resume_main_once) + return false; + + return _main_monitor_job.constructed() + && !_main_monitor_job->completed() + && !_main_monitor_job->expired(); + }; + + auto main_suspended_for_io = [&] { + return _resume_main_once == false; }; + + while (main_blocked_in_monitor() || main_suspended_for_io()) { + + /* + * Block for one I/O signal and process all pending ones + * before executing the monitor functions. This avoids + * superflous executions of the monitor functions when + * receiving bursts of I/O signals. + */ + _env.ep().wait_and_dispatch_one_io_signal(); + + dispatch_all_pending_io_signals(); + + handle_io_progress(); + } + + /* + * Return to the application + */ + if (!_kernel_routine && _resume_main_once && !_setjmp(_kernel_context)) { _switch_to_user(); + } } } @@ -508,11 +569,17 @@ struct Libc::Kernel final : Vfs::Io_response_handler, Monitor::Result _monitor(Mutex &mutex, Function &fn, uint64_t timeout_ms) override { if (_main_context()) { - Main_job job { fn, timeout_ms }; - _monitors.monitor(mutex, job); - return job.completed() ? Monitor::Result::COMPLETE - : Monitor::Result::TIMEOUT; + _main_monitor_job.construct(fn, timeout_ms); + + _monitors.monitor(mutex, *_main_monitor_job); + + Monitor::Result const job_result = _main_monitor_job->completed() + ? Monitor::Result::COMPLETE + : Monitor::Result::TIMEOUT; + _main_monitor_job.destruct(); + + return job_result; } else { Pthread_job job { fn, _timer_accessor, timeout_ms }; @@ -523,13 +590,12 @@ struct Libc::Kernel final : Vfs::Io_response_handler, } } - void _charge_monitors() override + void _trigger_monitor_examination() override { - if (!_execute_monitors_pending) { - _execute_monitors_pending = true; - if (!_main_context()) - Signal_transmitter(*_execute_monitors).submit(); - } + if (_main_context()) + _monitors_handler(); + else + Signal_transmitter(*_execute_monitors).submit(); } /** @@ -600,7 +666,7 @@ struct Libc::Kernel final : Vfs::Io_response_handler, /** * Cwd interface */ - Absolute_path &cwd() { return _cwd; } + Absolute_path &cwd() override { return _cwd; } /********************************* @@ -625,11 +691,8 @@ struct Libc::Kernel final : Vfs::Io_response_handler, ** Vfs::Io_response_handler interface ** ****************************************/ - void read_ready_response() override { - _io_ready = true; } - - void io_progress_response() override { - _io_ready = true; } + void read_ready_response() override { _io_progressed = true; } + void io_progress_response() override { _io_progressed = true; } /********************************************** diff --git a/repos/libports/src/lib/libc/internal/monitor.h b/repos/libports/src/lib/libc/internal/monitor.h index 108387430b..4c8399d7f3 100644 --- a/repos/libports/src/lib/libc/internal/monitor.h +++ b/repos/libports/src/lib/libc/internal/monitor.h @@ -60,7 +60,7 @@ class Libc::Monitor : Interface protected: virtual Result _monitor(Mutex &, Function &, uint64_t) = 0; - virtual void _charge_monitors() = 0; + virtual void _trigger_monitor_examination() = 0; public: @@ -68,7 +68,7 @@ class Libc::Monitor : Interface * Block until monitored execution completed or timeout expires * * The mutex must be locked when calling the monitor. It is released - * during wait for completion and re-aquired before the function + * during wait for completion and re-acquired before the function * returns. This behavior is comparable to condition variables. * * Returns true if execution completed, false on timeout. @@ -83,14 +83,13 @@ class Libc::Monitor : Interface _Function(FN const &fn) : fn(fn) { } } function { fn }; - _charge_monitors(); return _monitor(mutex, function, timeout_ms); } /** - * Charge monitor to execute the monitored function + * Trigger examination of monitored functions */ - void charge_monitors() { _charge_monitors(); } + void trigger_monitor_examination() { _trigger_monitor_examination(); } }; @@ -136,21 +135,35 @@ struct Libc::Monitor::Pool mutex.release(); - _monitor.charge_monitors(); + _monitor.trigger_monitor_examination(); job.wait_for_completion(); mutex.acquire(); } + enum class State { JOBS_PENDING, ALL_COMPLETE }; + /* called by the monitor context itself */ - void execute_monitors() + State execute_monitors() { + State result = State::ALL_COMPLETE; + _jobs.for_each([&] (Job &job) { - if (!job.completed() && !job.expired() && job.execute()) { - job.complete(); + + if (!job.completed() && !job.expired()) { + + bool const completed = job.execute(); + + if (completed) + job.complete(); + + if (!completed) + result = State::JOBS_PENDING; } }); + + return result; } }; diff --git a/repos/libports/src/lib/libc/kernel.cc b/repos/libports/src/lib/libc/kernel.cc index f1886e3e50..a1baad5e56 100644 --- a/repos/libports/src/lib/libc/kernel.cc +++ b/repos/libports/src/lib/libc/kernel.cc @@ -391,28 +391,13 @@ extern void (*libc_select_notify_from_kernel)(); void Libc::Kernel::handle_io_progress() { - if (_execute_monitors_pending) { - _execute_monitors_pending = false; - _monitors.execute_monitors(); - } + if (_io_progressed) { + _io_progressed = false; - /* - * TODO: make VFS I/O completion checks during - * kernel time to avoid flapping between stacks - */ - - if (_io_ready) { - _io_ready = false; - - /* some contexts may have been deblocked from select() */ - select_notify_from_kernel(); - - /* - * resume all as any VFS context may have - * been deblocked from blocking I/O - */ Kernel::resume_all(); - _monitors.execute_monitors(); + + if (_execute_monitors_pending == Monitor::Pool::State::JOBS_PENDING) + _execute_monitors_pending = _monitors.execute_monitors(); } } @@ -490,7 +475,7 @@ Libc::Kernel::Kernel(Genode::Env &env, Genode::Allocator &heap) init_vfs_plugin(*this, _env.rm()); init_file_operations(*this); init_time(*this, *this); - init_select(*this, *this, *this, _signal); + init_select(*this, _signal, *this); init_socket_fs(*this); init_passwd(_passwd_config()); init_signal(_signal); diff --git a/repos/libports/src/lib/libc/poll.cc b/repos/libports/src/lib/libc/poll.cc index aaa8079a0a..127c286e34 100644 --- a/repos/libports/src/lib/libc/poll.cc +++ b/repos/libports/src/lib/libc/poll.cc @@ -22,7 +22,6 @@ #include #include #include -#include using namespace Libc; diff --git a/repos/libports/src/lib/libc/select.cc b/repos/libports/src/lib/libc/select.cc index fa1f084f88..d459164b2f 100644 --- a/repos/libports/src/lib/libc/select.cc +++ b/repos/libports/src/lib/libc/select.cc @@ -38,10 +38,9 @@ #include #include #include -#include -#include #include #include +#include namespace Libc { struct Select_cb; @@ -51,19 +50,16 @@ namespace Libc { using namespace Libc; -static Suspend *_suspend_ptr; -static Resume *_resume_ptr; static Select *_select_ptr; static Libc::Signal *_signal_ptr; +static Monitor *_monitor_ptr; -void Libc::init_select(Suspend &suspend, Resume &resume, Select &select, - Signal &signal) +void Libc::init_select(Select &select, Signal &signal, Monitor &monitor) { - _suspend_ptr = &suspend; - _resume_ptr = &resume; _select_ptr = &select; _signal_ptr = &signal; + _monitor_ptr = &monitor; } @@ -210,7 +206,6 @@ static int selscan(int nfds, /* this function gets called by plugin backends when file descripors become ready */ void Libc::select_notify_from_kernel() { - bool resume_all = false; fd_set tmp_readfds, tmp_writefds, tmp_exceptfds; /* check for each waiting select() function if one of its fds is ready now @@ -224,18 +219,8 @@ void Libc::select_notify_from_kernel() scb.readfds = tmp_readfds; scb.writefds = tmp_writefds; scb.exceptfds = tmp_exceptfds; - - resume_all = true; } }); - - if (resume_all) { - struct Missing_call_of_init_select : Exception { }; - if (!_resume_ptr) - throw Missing_call_of_init_select(); - - _resume_ptr->resume_all(); - } } @@ -288,35 +273,14 @@ int select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, return 0; } - /* suspend as we don't have any immediate events */ - - struct Timeout - { - timeval const *_tv; - bool const valid { _tv != nullptr }; - Genode::uint64_t duration { - valid ? (Genode::uint64_t)_tv->tv_sec*1000 + _tv->tv_usec/1000 : 0UL }; - - bool expired() const { return valid && duration == 0; }; - - Timeout(timeval *tv) : _tv(tv) { } - } timeout { tv }; - - struct Check : Suspend_functor - { - struct Timeout *timeout; - Select_cb *select_cb; - - Check(Timeout *timeout, Select_cb * select_cb) - : timeout(timeout), select_cb(select_cb) { } - - bool suspend() override { - return !timeout->expired() && select_cb->nready == 0; } - } check ( &timeout, &*select_cb ); + using Genode::uint64_t; + uint64_t const timeout_ms = (tv != nullptr) + ? (uint64_t)tv->tv_sec*1000 + tv->tv_usec/1000 + : 0UL; { struct Missing_call_of_init_select : Exception { }; - if (!_suspend_ptr || !_signal_ptr) + if (!_monitor_ptr || !_signal_ptr) throw Missing_call_of_init_select(); } @@ -324,25 +288,31 @@ int select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, auto signal_occurred_during_select = [&] () { - return _signal_ptr->count() != orig_signal_count; + return (_signal_ptr->count() != orig_signal_count); }; - for (;;) { - if (timeout.expired()) - break; + auto monitor_fn = [&] () + { + select_notify_from_kernel(); if (select_cb->nready != 0) - break; + return Monitor::Function_result::COMPLETE; if (signal_occurred_during_select()) - break; + return Monitor::Function_result::COMPLETE; - timeout.duration = _suspend_ptr->suspend(check, timeout.duration); - } + return Monitor::Function_result::INCOMPLETE; + }; + + Mutex mutex { }; + Mutex::Guard guard(mutex); + + Monitor::Result const monitor_result = + _monitor_ptr->monitor(mutex, monitor_fn, timeout_ms); select_cb_list().remove(&(*select_cb)); - if (timeout.expired()) + if (monitor_result == Monitor::Result::TIMEOUT) return 0; if (signal_occurred_during_select()) @@ -357,6 +327,7 @@ int select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, return select_cb->nready; } + extern "C" __attribute__((alias("select"))) int __sys_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *tv);