From ccba43574f8695249109a440bfce4a33bb41588e Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Wed, 21 May 2014 15:52:05 +0200 Subject: [PATCH] hw: fix bug in scheduler timing By now the scheduling timer was only refreshed for a new scheduling timeout when the choosen scheduling context has changed. But we want it to be refreshed also when the scheduled context yields without an effect to the schedulers choice (this is the case e.g. when the idle thread gets a scheduling timeout or a thread yields without any competitor in its priority band). ref #1151 --- repos/base-hw/src/core/kernel/kernel.cc | 55 +++------------- repos/base-hw/src/core/kernel/processor.cc | 24 ++++--- repos/base-hw/src/core/kernel/processor.h | 65 ++++++++++++++++++- .../base-hw/src/core/kernel/processor_pool.h | 7 +- repos/base-hw/src/core/kernel/scheduler.h | 25 +++++-- 5 files changed, 108 insertions(+), 68 deletions(-) diff --git a/repos/base-hw/src/core/kernel/kernel.cc b/repos/base-hw/src/core/kernel/kernel.cc index 1941e31343..a44d004f20 100644 --- a/repos/base-hw/src/core/kernel/kernel.cc +++ b/repos/base-hw/src/core/kernel/kernel.cc @@ -74,25 +74,6 @@ namespace Kernel Signal_context_pool * signal_context_pool() { return unmanaged_singleton(); } Signal_receiver_pool * signal_receiver_pool() { return unmanaged_singleton(); } - /** - * Return singleton kernel-timer - */ - Timer * timer() - { - static Timer _object; - return &_object; - } - - /** - * Start a new scheduling lap - */ - void reset_scheduling_time(unsigned const processor_id) - { - unsigned const tics = timer()->ms_to_tics(USER_LAP_TIME_MS); - timer()->start_one_shot(tics, processor_id); - } - - /** * Static kernel PD that describes core */ @@ -287,9 +268,13 @@ extern "C" void init_kernel_multiprocessor() */ perf_counter()->enable(); - /* initialize interrupt controller */ - pic()->init_processor_local(); + /* locally initialize processor */ unsigned const processor_id = Processor::executing_id(); + Processor * const processor = processor_pool()->processor(processor_id); + processor->init_processor_local(); + + /* locally initialize interrupt controller */ + pic()->init_processor_local(); pic()->unmask(Timer::interrupt_id(processor_id), processor_id); /* as primary processor create the core main thread */ @@ -315,7 +300,7 @@ extern "C" void init_kernel_multiprocessor() _main_thread_utcb->start_info()->init(t.id(), Genode::Native_capability()); t.ip = (addr_t)CORE_MAIN;; t.sp = (addr_t)s + STACK_SIZE; - t.init(processor_pool()->processor(processor_id), core_pd(), &utcb, 1); + t.init(processor, core_pd(), &utcb, 1); /* initialize interrupt objects */ static Genode::uint8_t _irqs[Pic::MAX_INTERRUPT_ID * sizeof(Irq)]; @@ -326,7 +311,6 @@ extern "C" void init_kernel_multiprocessor() /* kernel initialization finished */ Genode::printf("kernel initialized\n"); } - reset_scheduling_time(processor_id); } @@ -338,31 +322,10 @@ extern "C" void kernel() /* ensure that no other processor accesses kernel data while we do */ data_lock().lock(); - /* determine local processor scheduler */ + /* determine local processor object and let it handle its exception */ unsigned const processor_id = Processor::executing_id(); Processor * const processor = processor_pool()->processor(processor_id); - Scheduler * const scheduler = processor->scheduler(); - - /* - * Request the current processor occupant without any update. While this - * processor was outside the kernel, another processor may have changed the - * scheduling of the local activities in a way that an update would return - * an occupant other than that whose exception caused the kernel entry. - */ - Processor_client * const old_client = scheduler->occupant(); - Processor_lazy_state * const old_state = old_client->lazy_state(); - old_client->exception(processor_id); - - /* - * The processor local as well as remote exception-handling may have - * changed the scheduling of the local activities. Hence we must update the - * processor occupant. - */ - Processor_client * const new_client = scheduler->update_occupant(); - Processor_lazy_state * const new_state = new_client->lazy_state(); - if (old_client != new_client) { reset_scheduling_time(processor_id); } - processor->prepare_proceeding(old_state, new_state); - new_client->proceed(processor_id); + processor->exception(); } diff --git a/repos/base-hw/src/core/kernel/processor.cc b/repos/base-hw/src/core/kernel/processor.cc index 1328a126a0..4de36eec9a 100644 --- a/repos/base-hw/src/core/kernel/processor.cc +++ b/repos/base-hw/src/core/kernel/processor.cc @@ -44,24 +44,22 @@ void Kernel::Processor_client::_interrupt(unsigned const processor_id) /* determine handling for specific interrupt */ unsigned irq_id; Pic * const ic = pic(); - if (ic->take_request(irq_id)) - { + if (ic->take_request(irq_id)) { + /* check wether the interrupt is a processor-scheduling timeout */ - if (timer()->interrupt_id(processor_id) == irq_id) { + if (!_processor->check_timer_interrupt(irq_id)) { - _processor->scheduler()->yield_occupation(); - timer()->clear_interrupt(processor_id); + /* check wether the interrupt is our inter-processor interrupt */ + if (ic->is_ip_interrupt(irq_id, processor_id)) { - /* check wether the interrupt is our inter-processor interrupt */ - } else if (ic->is_ip_interrupt(irq_id, processor_id)) { + _processor->ip_interrupt(); - _processor->ip_interrupt(); + /* after all it must be a user interrupt */ + } else { - /* after all it must be a user interrupt */ - } else { - - /* try to inform the user interrupt-handler */ - Irq::occurred(irq_id); + /* try to inform the user interrupt-handler */ + Irq::occurred(irq_id); + } } } /* end interrupt request at controller */ diff --git a/repos/base-hw/src/core/kernel/processor.h b/repos/base-hw/src/core/kernel/processor.h index 3fd427f2b4..c78c1ace38 100644 --- a/repos/base-hw/src/core/kernel/processor.h +++ b/repos/base-hw/src/core/kernel/processor.h @@ -16,6 +16,7 @@ #define _KERNEL__PROCESSOR_H_ /* core includes */ +#include #include #include @@ -142,6 +143,16 @@ class Kernel::Processor : public Processor_driver unsigned const _id; Processor_scheduler _scheduler; bool _ip_interrupt_pending; + Timer * const _timer; + + /** + * Reset the scheduling timer for a new scheduling interval + */ + void _reset_timer() + { + unsigned const tics = _timer->ms_to_tics(USER_LAP_TIME_MS); + _timer->start_one_shot(tics, _id); + } public: @@ -150,12 +161,35 @@ class Kernel::Processor : public Processor_driver * * \param id kernel name of the processor object * \param idle_client client that gets scheduled on idle + * \param timer timer that is used for scheduling the processor */ - Processor(unsigned const id, Processor_client * const idle_client) + Processor(unsigned const id, Processor_client * const idle_client, + Timer * const timer) : - _id(id), _scheduler(idle_client), _ip_interrupt_pending(false) + _id(id), _scheduler(idle_client), _ip_interrupt_pending(false), + _timer(timer) { } + /** + * Initializate on the processor that this object corresponds to + */ + void init_processor_local() { _reset_timer(); } + + /** + * Check for a scheduling timeout and handle it in case + * + * \param interrupt_id kernel name of interrupt that caused this call + * + * \return wether it was a timeout and therefore has been handled + */ + bool check_timer_interrupt(unsigned const interrupt_id) + { + if (_timer->interrupt_id(_id) != interrupt_id) { return false; } + _scheduler.yield_occupation(); + _timer->clear_interrupt(_id); + return true; + } + /** * Perform outstanding TLB maintainance work */ @@ -183,6 +217,33 @@ class Kernel::Processor : public Processor_driver */ void schedule(Processor_client * const client); + /** + * Handle exception of the processor and proceed its user execution + */ + void exception() + { + /* + * Request the current occupant without any update. While the + * processor was outside the kernel, another processor may have changed the + * scheduling of the local activities in a way that an update would return + * an occupant other than that whose exception caused the kernel entry. + */ + Processor_client * const old_client = _scheduler.occupant(); + Processor_lazy_state * const old_state = old_client->lazy_state(); + old_client->exception(_id); + + /* + * The processor local as well as remote exception-handling may have + * changed the scheduling of the local activities. Hence we must update the + * occupant. + */ + bool update; + Processor_client * const new_client = _scheduler.update_occupant(update); + if (update) { _reset_timer(); } + Processor_lazy_state * const new_state = new_client->lazy_state(); + prepare_proceeding(old_state, new_state); + new_client->proceed(_id); + } /*************** ** Accessors ** diff --git a/repos/base-hw/src/core/kernel/processor_pool.h b/repos/base-hw/src/core/kernel/processor_pool.h index 87c9728c69..16198a18f9 100644 --- a/repos/base-hw/src/core/kernel/processor_pool.h +++ b/repos/base-hw/src/core/kernel/processor_pool.h @@ -80,8 +80,9 @@ class Kernel::Processor_pool { private: - char _processors[PROCESSORS][sizeof(Processor)]; - char _idle_threads[PROCESSORS][sizeof(Idle_thread)]; + Timer _timer; + char _processors[PROCESSORS][sizeof(Processor)]; + char _idle_threads[PROCESSORS][sizeof(Idle_thread)]; /** * Return idle thread of a specific processor @@ -103,7 +104,7 @@ class Kernel::Processor_pool { for (unsigned i = 0; i < PROCESSORS; i++) { new (_idle_threads[i]) Idle_thread(processor(i)); - new (_processors[i]) Processor(i, _idle_thread(i)); + new (_processors[i]) Processor(i, _idle_thread(i), &_timer); } } diff --git a/repos/base-hw/src/core/kernel/scheduler.h b/repos/base-hw/src/core/kernel/scheduler.h index 97b6caec01..9acaacc459 100644 --- a/repos/base-hw/src/core/kernel/scheduler.h +++ b/repos/base-hw/src/core/kernel/scheduler.h @@ -232,11 +232,22 @@ class Kernel::Scheduler_item : public Double_list::Item template class Kernel::Scheduler { - protected: + private: T * const _idle; T * _occupant; Double_list _items[Priority::MAX + 1]; + bool _yield; + + bool _check_update(T * const occupant) + { + if (_yield) { + _yield = false; + return true; + } + if (_occupant != occupant) { return true; } + return false; + } public: @@ -252,12 +263,17 @@ class Kernel::Scheduler * * \return updated occupant reference */ - T * update_occupant() + T * update_occupant(bool & update) { for (int i = Priority::MAX; i >= 0 ; i--) { - _occupant = _items[i].head(); - if (_occupant) { return _occupant; } + T * const head = _items[i].head(); + if (!head) { continue; } + update = _check_update(head); + _occupant = head; + return head; } + update = _check_update(_idle); + _occupant = 0; return _idle; } @@ -266,6 +282,7 @@ class Kernel::Scheduler */ void yield_occupation() { + _yield = true; if (!_occupant) { return; } _items[_occupant->priority()].head_to_tail(); }