From 8a3b0ebea931e41829979221c566ad963c5b6706 Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Sun, 18 Nov 2018 15:30:30 +0100 Subject: [PATCH] Use strictly-typed Microseconds for Libc timeout scheduling Fix #3050 --- repos/libports/lib/symbols/libc | 1 + repos/libports/src/lib/libc/nanosleep.cc | 8 +- repos/libports/src/lib/libc/select.cc | 8 +- repos/libports/src/lib/libc/task.cc | 94 ++++++++++++------------ repos/libports/src/lib/libc/task.h | 13 ++-- 5 files changed, 66 insertions(+), 58 deletions(-) diff --git a/repos/libports/lib/symbols/libc b/repos/libports/lib/symbols/libc index b75d1d695..5714c9bd2 100644 --- a/repos/libports/lib/symbols/libc +++ b/repos/libports/lib/symbols/libc @@ -948,6 +948,7 @@ _ZN16Pthread_registry6insertEP7pthread T _ZN16Pthread_registry6removeEP7pthread T _ZN16Pthread_registry8containsEP7pthread T _ZN4Libc14pthread_createEPP7pthreadPFPvS3_ES3_mPKcPN6Genode11Cpu_sessionENS8_8Affinity8LocationE T +_ZN4Libc7suspendERNS_15Suspend_functorEN6Genode12MicrosecondsE T # # Libc plugin interface diff --git a/repos/libports/src/lib/libc/nanosleep.cc b/repos/libports/src/lib/libc/nanosleep.cc index 7068abe8c..824873d2b 100644 --- a/repos/libports/src/lib/libc/nanosleep.cc +++ b/repos/libports/src/lib/libc/nanosleep.cc @@ -19,12 +19,14 @@ extern "C" __attribute__((weak)) int _nanosleep(const struct timespec *req, struct timespec *rem) { - unsigned long sleep_ms = req->tv_sec*1000 + req->tv_nsec/1000000; + using namespace Libc; - if (!sleep_ms) return 0; + Microseconds sleep_us(req->tv_sec*1000*1000 + req->tv_nsec/1000); + + if (!sleep_us.value) return 0; struct Check : Libc::Suspend_functor { bool suspend() override { return true; } } check; - do { sleep_ms = Libc::suspend(check, sleep_ms); } while (sleep_ms); + do { sleep_us = Libc::suspend(check, sleep_us); } while (sleep_us.value); if (rem) { rem->tv_sec = 0; diff --git a/repos/libports/src/lib/libc/select.cc b/repos/libports/src/lib/libc/select.cc index 15cc53f1d..14ba5a216 100644 --- a/repos/libports/src/lib/libc/select.cc +++ b/repos/libports/src/lib/libc/select.cc @@ -220,6 +220,8 @@ __attribute__((weak)) _select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *tv) { + using namespace Libc; + fd_set in_readfds, in_writefds, in_exceptfds; Genode::Constructible select_cb; @@ -262,10 +264,10 @@ _select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, { timeval const *_tv; bool const valid { _tv != nullptr }; - unsigned long duration { - valid ? (unsigned long)_tv->tv_sec*1000 + _tv->tv_usec/1000 : 0UL }; + Microseconds duration { + valid ? (unsigned long)_tv->tv_sec*1000 + _tv->tv_usec : 0UL }; - bool expired() const { return valid && duration == 0; }; + bool expired() const { return valid && duration.value == 0; }; Timeout(timeval *tv) : _tv(tv) { } } timeout { tv }; diff --git a/repos/libports/src/lib/libc/task.cc b/repos/libports/src/lib/libc/task.cc index 4e2012110..f1c0c9e58 100644 --- a/repos/libports/src/lib/libc/task.cc +++ b/repos/libports/src/lib/libc/task.cc @@ -49,7 +49,7 @@ namespace Libc { class Timeout_handler; class Io_response_handler; - using Genode::Microseconds; + using Genode::Duration; } @@ -217,7 +217,7 @@ struct Libc::Timer Timer(Genode::Env &env) : _timer(env) { } - Genode::Duration curr_time() + Duration curr_time() { return _timer.curr_time(); } @@ -227,9 +227,9 @@ struct Libc::Timer return Microseconds(1000*timeout_ms); } - static unsigned long max_timeout() + static Microseconds max_timeout() { - return ~0UL/1000; + return Microseconds(~0UL/(1000*1000)); } }; @@ -262,13 +262,13 @@ struct Libc::Timeout Timeout_handler &_handler; ::Timer::One_shot_timeout _timeout; - bool _expired = true; - unsigned long _absolute_timeout_ms = 0; + bool _expired = true; + Duration _absolute_timeout { Microseconds(0) }; void _handle(Duration now) { - _expired = true; - _absolute_timeout_ms = 0; + _expired = true; + _absolute_timeout = Duration(Microseconds(0)); _handler.handle_timeout(); } @@ -279,24 +279,23 @@ struct Libc::Timeout _timeout(_timer_accessor.timer()._timer, *this, &Timeout::_handle) { } - void start(unsigned long timeout_ms) + void start(Microseconds timeout_us) { - Milliseconds const now = _timer_accessor.timer().curr_time().trunc_to_plain_ms(); + _absolute_timeout = _timer_accessor.timer().curr_time(); + _absolute_timeout.add(timeout_us); - _expired = false; - _absolute_timeout_ms = now.value + timeout_ms; - - _timeout.schedule(_timer_accessor.timer().microseconds(timeout_ms)); + _timeout.schedule(timeout_us); } - unsigned long duration_left() const + Microseconds duration_left() const { - Milliseconds const now = _timer_accessor.timer().curr_time().trunc_to_plain_ms(); + Duration const now = _timer_accessor.timer().curr_time(); - if (_expired || _absolute_timeout_ms < now.value) - return 0; + if (_expired || _absolute_timeout.less_than(now)) + return Microseconds(0); - return _absolute_timeout_ms - now.value; + return Microseconds(_absolute_timeout.trunc_to_plain_us().value + - now.trunc_to_plain_us().value); } }; @@ -317,16 +316,16 @@ struct Libc::Pthreads _timeout.construct(_timer_accessor, *this); } - Pthread(Timer_accessor &timer_accessor, unsigned long timeout_ms) + Pthread(Timer_accessor &timer_accessor, Microseconds timeout) : _timer_accessor(timer_accessor) { - if (timeout_ms > 0) { + if (timeout.value > 0) { _construct_timeout_once(); - _timeout->start(timeout_ms); + _timeout->start(timeout); } } - unsigned long duration_left() + Microseconds duration_left() { _construct_timeout_once(); return _timeout->duration_left(); @@ -354,9 +353,9 @@ struct Libc::Pthreads p->lock.unlock(); } - unsigned long suspend_myself(Suspend_functor & check, unsigned long timeout_ms) + Microseconds suspend_myself(Suspend_functor & check, Microseconds timeout_us) { - Pthread myself { timer_accessor, timeout_ms }; + Pthread myself { timer_accessor, timeout_us }; { Genode::Lock::Guard g(mutex); @@ -379,7 +378,7 @@ struct Libc::Pthreads } } - return timeout_ms > 0 ? myself.duration_left() : 0; + return timeout_us.value > 0 ? myself.duration_left() : Microseconds(0); } }; @@ -503,13 +502,13 @@ struct Libc::Kernel : _timer_accessor(timer_accessor), _kernel(kernel) { } - void timeout(unsigned long timeout_ms) + void timeout(Microseconds timeout) { _construct_timeout_once(); - _timeout->start(timeout_ms); + _timeout->start(timeout); } - unsigned long duration_left() + Microseconds duration_left() { _construct_timeout_once(); return _timeout->duration_left(); @@ -553,7 +552,7 @@ struct Libc::Kernel kernel->_app_code->execute(); kernel->_app_returned = true; - kernel->_suspend_main(check, 0); + kernel->_suspend_main(check, Microseconds(0)); } bool _main_context() const { return &_myself == Genode::Thread::myself(); } @@ -586,8 +585,8 @@ struct Libc::Kernel _longjmp(_user_context, 1); } - unsigned long _suspend_main(Suspend_functor &check, - unsigned long timeout_ms) + Microseconds _suspend_main(Suspend_functor &check, + Microseconds timeout) { /* check that we're not running on libc kernel context */ if (Thread::mystack().top == _kernel_stack) { @@ -597,10 +596,10 @@ struct Libc::Kernel } if (!check.suspend()) - return 0; + return Microseconds(0); - if (timeout_ms > 0) - _main_timeout.timeout(timeout_ms); + if (timeout.value > 0) + _main_timeout.timeout(timeout); if (!_setjmp(_user_context)) { _valid_user_context = true; @@ -628,7 +627,8 @@ struct Libc::Kernel _longjmp(_kernel_context, 1); } - return timeout_ms > 0 ? _main_timeout.duration_left() : 0; + return timeout.value > 0 + ? _main_timeout.duration_left() : Microseconds(0); } public: @@ -721,19 +721,19 @@ struct Libc::Kernel /** * Suspend this context (main or pthread) */ - unsigned long suspend(Suspend_functor &check, unsigned long timeout_ms) + Microseconds suspend(Suspend_functor &check, Microseconds timeout_us) { - if (timeout_ms > 0 - && timeout_ms > _timer_accessor.timer().max_timeout()) { + if (timeout_us.value > 0 + && timeout_us.value > _timer_accessor.timer().max_timeout().value) { Genode::warning("libc: limiting exceeding timeout of ", - timeout_ms, " ms to maximum of ", - _timer_accessor.timer().max_timeout(), " ms"); + timeout_us, " us to maximum of ", + _timer_accessor.timer().max_timeout(), " us"); - timeout_ms = min(timeout_ms, _timer_accessor.timer().max_timeout()); + timeout_us = min(timeout_us, _timer_accessor.timer().max_timeout()); } - return _main_context() ? _suspend_main(check, timeout_ms) - : _pthreads.suspend_myself(check, timeout_ms); + return _main_context() ? _suspend_main(check, timeout_us) + : _pthreads.suspend_myself(check, timeout_us); } void dispatch_pending_io_signals() @@ -751,7 +751,7 @@ struct Libc::Kernel } } - Genode::Duration current_time() + Duration current_time() { return _timer_accessor.timer().curr_time(); } @@ -873,13 +873,13 @@ static void resumed_callback() { kernel->entrypoint_resumed(); } void Libc::resume_all() { kernel->resume_all(); } -unsigned long Libc::suspend(Suspend_functor &s, unsigned long timeout_ms) +Libc::Microseconds Libc::suspend(Suspend_functor &s, Microseconds timeout_us) { if (!kernel) { error("libc kernel not initialized, needed for suspend()"); exit(1); } - return kernel->suspend(s, timeout_ms); + return kernel->suspend(s, timeout_us); } diff --git a/repos/libports/src/lib/libc/task.h b/repos/libports/src/lib/libc/task.h index cf39bf2aa..35e3ba134 100644 --- a/repos/libports/src/lib/libc/task.h +++ b/repos/libports/src/lib/libc/task.h @@ -26,6 +26,8 @@ namespace Libc { + using Genode::Microseconds; + /** * Resume all user contexts * @@ -36,18 +38,19 @@ namespace Libc { /** * Suspend the execution of the calling user context * - * \param timeout_ms maximum time to stay suspended in milliseconds, - * 0 for infinite suspend + * \param timeout maximum time to stay suspended in microseconds, + * 0 for infinite suspend * - * \return remaining duration until timeout, - * 0 if the timeout expired + * \return remaining duration until timeout, + * 0 if the timeout expired * * The context could be running on the component entrypoint as main context * or as separate pthread. This function returns after the libc kernel * resumed the user context execution. */ struct Suspend_functor { virtual bool suspend() = 0; }; - unsigned long suspend(Suspend_functor &, unsigned long timeout_ms = 0UL); + Microseconds suspend(Suspend_functor &, + Microseconds timeout = Microseconds(0UL)); void dispatch_pending_io_signals();