signal: fix starvation by fast signal contexts

In the past, a signal context, that was chosen for handling by
'Signal_receiver::pending_signal and always triggered again before
the next call of 'pending_signal', caused all other contexts behind
in the list to starve. This was the case because 'pending_signal'
always took the first pending context in its context list.

We avoid this problem now by handling pending signals in a round-robin
fashion instead.

Ref #2532
This commit is contained in:
Martin Stein 2017-09-17 12:30:20 +02:00 committed by Christian Helmuth
parent cd21074201
commit b811ef4331
4 changed files with 75 additions and 16 deletions

View File

@ -99,7 +99,7 @@ Signal_context_capability Signal_receiver::manage(Signal_context * const c)
/* use signal context as imprint */ /* use signal context as imprint */
c->_cap = pd().alloc_context(_cap, (unsigned long)c); c->_cap = pd().alloc_context(_cap, (unsigned long)c);
c->_receiver = this; c->_receiver = this;
_contexts.insert(&c->_receiver_le); _contexts.insert(c);
return c->_cap; return c->_cap;
} }
catch (Out_of_ram) { ram_upgrade = Ram_quota { 1024*sizeof(long) }; } catch (Out_of_ram) { ram_upgrade = Ram_quota { 1024*sizeof(long) }; }

View File

@ -174,6 +174,26 @@ class Genode::Signal_receiver : Noncopyable
{ {
private: private:
/**
* A list where the head can be moved
*/
class Context_list
{
private:
Signal_context *_head { nullptr };
public:
Signal_context *head() const { return _head; }
void head(Signal_context *re);
void insert(Signal_context *re);
void remove(Signal_context const *re);
};
/** /**
* Semaphore used to indicate that signal(s) are ready to be picked * Semaphore used to indicate that signal(s) are ready to be picked
* up. This is needed for platforms other than 'base-hw' only. * up. This is needed for platforms other than 'base-hw' only.
@ -189,8 +209,8 @@ class Genode::Signal_receiver : Noncopyable
/** /**
* List of associated contexts * List of associated contexts
*/ */
Lock _contexts_lock; Lock _contexts_lock;
List<List_element<Signal_context> > _contexts; Context_list _contexts;
/** /**
* Helper to dissolve given context * Helper to dissolve given context
@ -324,7 +344,8 @@ class Genode::Signal_context
/** /**
* List element in 'Signal_receiver' * List element in 'Signal_receiver'
*/ */
List_element<Signal_context> _receiver_le; Signal_context mutable *_next { nullptr };
Signal_context mutable *_prev { nullptr };
/** /**
* List element in process-global registry * List element in process-global registry
@ -373,8 +394,8 @@ class Genode::Signal_context
* Constructor * Constructor
*/ */
Signal_context() Signal_context()
: _receiver_le(this), _registry_le(this), _deferred_le(this), : _registry_le(this), _deferred_le(this), _receiver(0), _pending(0),
_receiver(0), _pending(0), _ref_cnt(0) { } _ref_cnt(0) { }
/** /**
* Destructor * Destructor

View File

@ -224,7 +224,7 @@ Signal_context_capability Signal_receiver::manage(Signal_context *context)
Lock::Guard list_lock_guard(_contexts_lock); Lock::Guard list_lock_guard(_contexts_lock);
/* insert context into context list */ /* insert context into context list */
_contexts.insert(&context->_receiver_le); _contexts.insert(context);
/* register context at process-wide registry */ /* register context at process-wide registry */
signal_context_registry()->insert(&context->_registry_le); signal_context_registry()->insert(&context->_registry_le);

View File

@ -163,9 +163,7 @@ Signal Signal_receiver::pending_signal()
Lock::Guard list_lock_guard(_contexts_lock); Lock::Guard list_lock_guard(_contexts_lock);
/* look up the contexts for the pending signal */ /* look up the contexts for the pending signal */
for (List_element<Signal_context> *le = _contexts.first(); le; le = le->next()) { for (Signal_context *context = _contexts.head(); context; context = context->_next) {
Signal_context *context = le->object();
Lock::Guard lock_guard(context->_lock); Lock::Guard lock_guard(context->_lock);
@ -173,6 +171,7 @@ Signal Signal_receiver::pending_signal()
if (!context->_pending) if (!context->_pending)
continue; continue;
_contexts.head(context);
context->_pending = false; context->_pending = false;
Signal::Data result = context->_curr_signal; Signal::Data result = context->_curr_signal;
@ -206,8 +205,8 @@ Signal_receiver::~Signal_receiver()
Lock::Guard list_lock_guard(_contexts_lock); Lock::Guard list_lock_guard(_contexts_lock);
/* disassociate contexts from the receiver */ /* disassociate contexts from the receiver */
for (List_element<Signal_context> *le; (le = _contexts.first()); ) while (_contexts.head())
_unsynchronized_dissolve(le->object()); _unsynchronized_dissolve(_contexts.head());
_platform_destructor(); _platform_destructor();
} }
@ -225,7 +224,7 @@ void Signal_receiver::_unsynchronized_dissolve(Signal_context * const context)
context->_cap = Signal_context_capability(); context->_cap = Signal_context_capability();
/* remove context from context list */ /* remove context from context list */
_contexts.remove(&context->_receiver_le); _contexts.remove(context);
_platform_finish_dissolve(context); _platform_finish_dissolve(context);
} }
@ -250,9 +249,7 @@ bool Signal_receiver::pending()
Lock::Guard list_lock_guard(_contexts_lock); Lock::Guard list_lock_guard(_contexts_lock);
/* look up the contexts for the pending signal */ /* look up the contexts for the pending signal */
for (List_element<Signal_context> *le = _contexts.first(); le; le = le->next()) { for (Signal_context *context = _contexts.head(); context; context = context->_next) {
Signal_context *context = le->object();
Lock::Guard lock_guard(context->_lock); Lock::Guard lock_guard(context->_lock);
@ -261,3 +258,44 @@ bool Signal_receiver::pending()
} }
return false; return false;
} }
void Signal_receiver::Context_list::insert(Signal_context *re)
{
if (_head) {
re->_prev = _head->_prev;
re->_next = nullptr;
_head->_prev->_next = re;
_head->_prev = re;
} else {
re->_prev = re;
re->_next = nullptr;
_head = re;
}
}
void Signal_receiver::Context_list::remove(Signal_context const *re)
{
if (re->_prev == re) {
_head = nullptr;
} else {
if (_head == re) {
_head = re->_next;
}
re->_prev->_next = re->_next;
if (re->_next) {
re->_next->_prev = re->_prev;
} else {
_head->_prev = re->_prev;
}
}
}
void Signal_receiver::Context_list::head(Signal_context *re)
{
_head->_prev->_next = _head;
_head = re;
_head->_prev->_next = nullptr;
}