sys: single interrupt handler thread for interrupts in modules with blocking functions

[gschorcht]

Contribution description

There are many modules that use shared peripheral interfaces such as SPI and I2C, especially driver modules for sensors or actuators connected via SPI or I2C. This type of modules also use interrupts very often and normally require access to the SPI or the I2C interfaces when an interrupt
occurs, for example, to read status registers.

Accessing SPI or I2C interfaces within an interrupt service routine could interfere with an already existing interface access as described for example in issue #10488, PR #10430, and in http://api.riot-os.org/group__drivers__netdev__api.html. The only solution to this problem is not to call any function that interacts with a device directly from interrupt context. Rather, the interrupt service routine only has to indicate the occurrence of the interrupt. The interrupt is then handled asynchronously in a thread.

The problem is that many modules, such as driver modules, do not use their own thread, but run in the context of the calling thread. However, it can not be up to the application thread to handle the interrupts of such modules. The only solution would be for any module that uses interrupts with SPI or I2C interfaces to create its own interrupt handler thread. However, as the number of such modules increases, many resources (the thread context including the thread stack) are used only for interrupt
handling.

The proposed solution provided with this PR is to have only one thread with high priority for handling the interrupts of such modules. This PR provides a small module which realizes such a single interrupt handler thread for modules that do not use their own thread but run in the context of the calling thread.

Testing procedure

Use tests/sys_irq_handler as following.

make -C tests/sys_irq_handler BOARD=...

Issues/PRs references

Fixes the problem described in issue #10488 and PR #10430

[kaspar030]

Did you take a look at sys/event? It can easily be used for the same purpose.

[gschorcht]

Yes, of course. But the problem is the same. Each driver module would need its own thread to handle the events. The main purpose is of this PR is not to reimplement an event queue but to provide only a sinlge thread for queueing interrupt request of all driver modules that require bus access in their ISR. I started the implementation based on your event module but I realized that I only need add and remove elements from a queue. Furthermore, it should be possible to handle interrupt priorities. Therefore, I used the priority queue.

[kaspar030]

The main purpose is of this PR is not to reimplement an event queue but to provide only a single thread for queueing interrupt request of all driver modules that require bus access in their ISR.

Isn't one a mechanism, the other a use case?

I'm a bit reluctant to add a functionally very similar module at this low level, without trying hard to see whether one might be enough.

Could you maybe elaborate why a shared thread running an event queue didn't cut it?

Was it just the prioritization? That could maybe be added to event queues indirectly by making the queue thread flag configurable. A single thread could then set up and listen to multiple event queues. By using thread flags to prioritize, we'd have the benefit of not using O(n) insertion into a single priority queue.

[kaspar030]

That could maybe be added to event queues indirectly by making the queue thread flag configurable.

Actually that shouldn't even be needed. If use exactly the API as proposed in this PR, and assuming there's always just a single ISR handler thread, irq_event_add() can just, in addition to do an event_post(), set another thread flag indicating the IRQ's priority.

[gschorcht]

@kaspar030 Thank you first for your comments.

I'm a bit reluctant to add a functionally very similar module at this low level, without trying hard to see whether one might be enough.

Yes, that's comprehensible and I completely agree with you.

Could you maybe elaborate why a shared thread running an event queue didn't cut it?
Was it just the prioritization?

Exactly. It is important to have priorities because all interrupts are "serialized" by the single ISR thread and some driver modules trigger interrupts with higher priorities than others. For example, the interrupt of a gas sensor that provides a sensor value only every second has a lower priority than the interrupt of a GPIO expander indicating the change of the level at an external GPIO which is used to trigger an GPIO interrupt. However, if the same gas sensor indicates the exceeding of a threshold by an interrupt, the interrupt may have a high priority. Which interrupt has which priority should be known to the developer of the driver.

With the current implementation in the PR there are three predefined interrupt priorities, but the user might want to use also priorities between these predefined priorities. I'm not sure whether this is sensible or not. Having three predefined interrupt priorities should normally be sufficient: HIGH if the interrupt should to be handled immediately, LOW if the latency is uncritical and MEDIUM if the developer is not sure.

Suppose we only allow these three priorities. In that case, we would need three event queues, one for each priority, right? It is not really clear how to deal with three event queues in a single thread. The blocking function event_wait can only be called for one of the event queues. In addition, even if there is a possibility to wait for an event in any of the three event queues, each time one interrupt event has been handled, you must have to iterate again over all three queues because a new higher priority interrupt may have arrived in the meantime. This is not the case for a single priority queue. The priority queue ensures the order in the queue and I have only to read the first element from the queue until there are no more in the queue.

It is clear that event queue has a complexity of O(1) while the priority queue has a complexity of O(n). The question is how probable it is that more than 1 event is in the queue at the same time. The question is how often does it happen that more than one interrupt event is queued at the same time and whether the higher overhead of reading from multiple event queues is justified. For an empty priority queue the complexity is also O(1). I would guess that probability of having multiple interrupt events in the queue is quite low.

Nevertheless, it is worthwhile to use a single ISR thread to solve the "interrupt context problem" for a number of driver modules with the resource requirement for only one thread. If only one driver module is used at the same time, it makes no difference if the driver uses its own thread or the single ISR thread. But if, for example, five driver modules are using the ISR thread at the same time, a lot of resources can be saved.

[jcarrano]

The problem for me is that we have an interface based mostly on blocking calls (issue 1).

Those other interfaces that accept a callback call it in interrupt context (issue 2), where there is functionality that cannot be used (issue 3).

Issue (3) is caused in great by issue (1). Also, if interrupt could trigger a callback that is run on a user thread contex, by using that thread's stack - issue (2)- then it could be a solution too.

[gschorcht]

Also, if interrupt could trigger a callback that is run on a user thread contex, by using that thread's stack - issue (2)- then it could be a solution too.

But this would require that the user has knowledge about the internals of the driver. For interrupts that are intended for the user, e.g., a data-ready interrupt of a sensor, this approach is clear and possible. Just tell the user to register a callback function to get informed when new data are available. Tell him further, that this callback function must not be used to fetch the data and that he has to fetch the new data the next time its thread is scheduled. That is ok.

But the situation is completly different, if the interrupts are not intended to be forwared to the user thread but only to realize the driver functionality. For example, an GPIO expander get an interrupt when the level at on pin changes and has to update its internal state. In that case we cannot ask the user to register a callback function for doing driver internals.

[jcarrano]

But the situation is completly different, if the interrupts are not intended to be forwared to the user thread but only to realize the driver functionality.

Then you could run the callbacks in the context of a special thread, like you are doing here.

[gschorcht]

@jcarrano Exactly. And if there is not only one such device driver, but several, we could save a lot of resources if all would used the same thread for their interrupt handling. Saving resources is one of the important design rules of RIOT. Therefore, I though it might be worth to have something like a shared thread for interrupt handling.

I was not really clear to me if you are pro or con with having such a shared thread as it is provided with this PR.

I could imagine to enhance my approach further by developing a module that provides each device driver with a separate thread for its interrupt handing if required, and if a special pseudomodule is enabled, they all use the same thread,

[kaspar030]

@gschorcht thanks for your detailed explanation!

I totally agree with the need for shared event threads, be it for interrupts or whatever. I still have hope that we can get away without duplicating functionality.

Suppose we only allow these three priorities. In that case, we would need three event queues, one for each priority, right? It is not really clear how to deal with three event queues in a single thread. The blocking function event_wait can only be called for one of the event queues.

That's true, at the moment. With the addition of another field in event_queue_t (let's call it thread_flag) for per-queue configurable thread flag, a single thread can wait on multiple event queues. If an event gets posted to any queue, bitwise find_first_set would find the number of the highest priority flag, which in turn could be used as an array index for a number of event queues. There would be no need to poll multiple queues.

Another option would be to wrap this PR's API over sys/event, and manage setting extra thread flags (for multiple priorities) at that level, using a similar dispatch as described above. No need for changing sys/event in that case, but the resulting multi-priority event API would be tied to ISRs, at least by naming.

[jcarrano]

I was not really clear to me if you are pro or con with having such a shared thread as it is provided with this PR.

I think it is a good idea. It would be some sort of "kernel idle thread". However:

• I'd like a mechanism implemented in the scheduler, by which a callback can be triggered in any context, just like with signal handlers. That functionality would be key, and the shared thread could be implemented in terms of that.
• If a thread is dedicated solely to running callbacks, then there is almost no context to save after a callback is done running. I don't know if it is currently possible to have a thread's main function exit, while keeping the stack and keeping the thread in the scheduler. If that is possible then there would be no need for a loop, and the whole thing would be even more efficient.
• I suspect that most of the time it should not be necessary to have more than one idle thread. Having more than one thread we risk having functionality that overlaps that of interrupt priorities.

I could imagine to enhance my approach further by developing a module that provides each device driver with a separate thread for its interrupt handing if required

Exactly that is what we should avoid, there is no reason why each driver should need it's own thread.

The issue I see with this (is it really an issue?) is that many of our APIs are not callback-based, so until that changes, it will be hard to take advantage of this feature.

I have been thinking recently on how wasteful it is, for example, that driver initialization is done sequentially, with blocking functions, essentially having the CPU wait on a peripheral while it could be initializing another device.

[maribu]

It would be some sort of "kernel idle thread".

I personally think it will cause more trouble than it can potentially yield benefit to somehow merge the idle thread and the interrupt handler thread.

Pros:

• One thread less, so less resource requirements

Cons:

• The idle thread has the lowest priority, and interrupt handler thread will have a high priority
• The only calls pm_set_lowest() in an infinite loop. This simplistic nature of the idle thread allows to tweak resource requirements considerably. E.g. THREAD_STACKSIZE_IDLE can be as low as 96 bytes on MSP430 based platforms or 128 bytes on ATmega based platforms
  • The interrupt handler thread does potentially complex stuff and requires more stack size
  • Either the resource requirements of the idle thread need to be raised to allow performing the async interrupt handling - thus wasting resources when no interrupt handler thread is actually used - or the complexity of the code base is increased quite a bit

I personally think the cons outweigh the pros considerably and thus I would like to add the interrupt handler thread as a completely new thread.

[maribu]

I have been thinking recently on how wasteful it is, for example, that driver initialization is done sequentially, with blocking functions, essentially having the CPU wait on a peripheral while it could be initializing another device.

Pros:

• Dramatic increase in performance when booting the system

Cons:

• Increased complexity and thus resource requirements
• All sort of dependency bugs and race conditions will be introduced
  • The initialization order of drivers could be different in every boot of the device
  • A dependency tracking mechanism would be required to make sure e.g. periphs are initialized before drivers using them will be initialized
    • This adds hugh complexity
  • Missing dependencies will often remain undiscovered, as e.g. periph's will likely end up being initialized before drivers using them just by chance.

My personal opinion is strongly against async device driver initialization. I never had a practical use case for increasing boot up times, so this solves a non-issue to me. And I personally hate racy code. It is just horrible to debug. In my opinion there has to be a really strong reason for adding "racyness" to software. Also trading in boot up time (that is paid only once during start up) for less ROM/RAM sounds like a really good deal for me.

[jcarrano]

I personally think it will cause more trouble than it can potentially yield benefit to somehow merge the idle thread and the interrupt handler thread.

Ok, now I see that there is already something called "idle thread", which I did not know when I wrote that (and still now I do not know what it does.) I guess my choice of words generated confusion.

The only calls pm_set_lowest() in an infinite loop

Is that necessary? If no threads are runnable, shouldn't the scheduler send the MCU to sleep?

My question is: can our scheduler support a "bare" thread, one with no saved context at all? That, in conjunction with the ability to run a callback in any context would meant we don't need an explicit loop.

Related to this question: what happens when a thread's main function returns?

Increased complexity and thus resource requirements

Unnecessarily blocking consumes more resources. A parallel start would probably consume less power, even if using power-saving features, there is an overhead each time the MCU has to sleep and wake up.

All sort of dependency bugs and race conditions will be introduced

Replace introduced with uncovered. If initializing things in parallel causes something to break, it was already broken to begin with. In any case, in an idle thread or in an event loop only one handler is running at each instant, and handler cannot preempt each other so there are a lot of bug which simply cannot
occur.

A dependency tracking mechanism would be required to make sure e.g. periphs are initialized before drivers using them will be initialized

I believe there is a misdesign in RIOT drivers, in that they don't remember if they have been initialized already, and therefore calling init() two times is an error. The overhead of calling 3, 4 or 10 times a function that does nothing, specially when you do it only once, at boot time, is well worth the safety and reduced complexity, but that's another topic.

Missing dependencies will often remain undiscovered, as e.g. periph's will likely end up being initialized before drivers using them just by chance.

May happen as well with sequential initialization.

My personal opinion is strongly against async device driver initialization. I never had a practical use case for increasing boot up times, so this solves a non-issue to me.

The thing is, if you have proper async-capable interfaces, async initialization comes naturally.

[kaspar030]

Is that necessary? If no threads are runnable, shouldn't the scheduler send the MCU to sleep?

In theory, this is not necessary at all. It actually causes quite a lot of unnecessary context switch overhead and at least the stack memory for the idle thread.

In practice, it simplifies things quite a lot when there's always a runnable thread.

Getting rid of the idle thread is definitely desirable, but it is also not trivial, especially when considering different architectures and their peculiarities regarding sleeping, nested interrupts, long-running interrupts etc. It might boil down to something like while(!(thread=thread_next_runnable() || isr_pending()) pm_set_lowest(); somewhere in sched_run(), but the devil lies in the details.

My question is: can our scheduler support a "bare" thread, one with no saved context at all? That, in conjunction with the ability to run a callback in any context would meant we don't need an explicit loop.

Without saveable context, the "bare thread" would be non-preemptable. At that point we could as well execute the callback in interrupt context.
There are probably methods. All of them would require extra complexity in the scheduler, also extra branches, which will cost performance.

Having contexts executed at random times in suspended thread contexts have the problem that the thread would need to pre-allocate as much stack space as itself needs PLUS all the stack space that the callback needs. In the end, after adding all the code for this "execute callback on any thread context", which is also highly platform dependent, we'd save not much more than a single "thread_t", compared to just adding another thread. Probably that gain would be lost through house keepig.

Related to this question: what happens when a thread's main function returns?

On thread creation, the stack is pre-filled with, among other things, a function to jump to. That's sched_task_exit. It will do some (minimal) cleanup, and the thread is removed from all queues. At that point it'll not be run again, it's thread_t and pid can be re-used.

[kaspar030]

The thing is, if you have proper async-capable interfaces, async initialization comes naturally.

Why don't you prototype something so we have a basis for discussion?

[jcarrano]

Why don't you prototype something so we have a basis for discussion?

Fine, let me dive into the horribly undocumented kernel and try to implement running callbacks on another context. Or maybe I can do what I did, that is ask if it is even possible, or if maybe there is a mechanism already in place.

When I say "The thing is, if you have proper async-capable interfaces, async initialization comes naturally."
I expect (perhaps unrealistically) that people will understand what is async initialization without me having to show them code, but I may have unrealistic expectations.

[maribu]

Replace introduced with uncovered. If initializing things in parallel causes something to break, it was already broken to begin with.

That is a good point. But I do not fully agree: sys/auto_init makes sure that the drivers are initialized sequentially in a specific order. This specific order complies with the dependency requirements of each driver. BUT: As far as I know most drivers do not document its requirements properly, so quite easily a change in sys/auto_init breaks things. (Something like that happen recently #10267.) If any driver initialization has special requirements and that is not documented properly, I do agree that this is a minor bug that easily leads to not-so-minor bugs.

I believe there is a misdesign in RIOT drivers, in that they don't remember if they have been initialized already, and therefore calling init() two times is an error.

I personally disagree with that. With the current sys/auto_init approach this is perfectly fine. Having the drivers tracking their state results in increased RAM requirements. If the sys/auto_init approach is changed, it might become mandatory that they track their state. But currently, it would just be using RAM for no benefit.

[jcarrano]

sys/auto_init makes sure that the drivers are initialized sequentially in a specific order.

Why not have each driver initialize its dependencies, and make init functions tolerant to repeat initializations? Then one only need to initialize the top level functionality the one actually needs.

Having the drivers tracking their state results in increased RAM requirements.ç

One boolean value. Is that a big deal? For some drivers they may even be able to squeeze it into a bit flag.

[maribu]

Let me try to understand what your async-capable interfaces could look like by suggesting some pseudo-code:

Assuming we have the drivers foonet, foodisplay, footemp and foohumidity. foonet and foodisplay would depend on spi, footemp and foohumidity would depend on i2c.

Pseudo code API for i2c and spi initialization:

typedef void (*spi_init_done_callback_t)(spi_t spi, void *arg);
typedef void (*i2c_init_done_callback_t)(i2c_t i2c, void *arg);

int spi_init(spi_t spi, spi_init_done_callback_t cb, void *arg);
int i2c_init(i2c_t i2c, i2c_init_done_callback_t cb, void *arg);
```C

And than foonet would be initializing like that:

```C
void foonet_init_cb(spi_t spi, void *_dev)
{
    foonet_t dev = _dev;
    /* Initializing foonet ... */
}

int foonet_init(foonet_t dev, foonet_params_t params)
{
    return spi_init(params->spi, foonet_init_cb, dev);
}

and the other drivers would be likewise, all explicitly re-initializing their dependencies. But because every driver tracks its state, it would only initialize on the first call, but on subsequent calls the callback is called right away. Right?

[maribu]

Back to the PR: The only feature sys/event is missing is priorities to be used as it is for this PR, right?

The reason why priorities for interrupt handler thread are required are:

1. Gracefully handle high load: Favor more important drivers over less important drivers when drivers are competing about CPU time
2. Different time requirements:
   • Some drivers need their interrupt to be handled within a bounded time, as otherwise bad things happens. (E.g. in the cc110x (I know, netdevs are not relevant, by I do not have a better example) the RX FIFO needs to be drained before it overflows.)
   • Other drivers are less demanding

But I honestly doubt that a priority queue will solve the second point. Assuming a slow device with a low priority generates an interrupt. The interrupt handler thread would start to handle it, as no other event is in the queue. Right after the thread started working on the low priority event a high priority interrupt comes in. This driver needs its ISR to be called within a bounded time. But the interrupt handler thread would not be preempted, so it would first finish the low priority event before starting on the high priority event.

I think there is no way to let a single thread handle both high priority events with real time requirements and low priority events without real time requirements, unless the driver are reworked to do slow stuff asynchron.

[jcarrano]

@maribu yes.

Here there are two things: one is the drivers initializing their dependencies. The other is callback-based APIs. The problem is that if one uses callback-based APIs then there has to be a way for drivers to wait for their dependencies, and that is what you solve by "re-initialization".

Back to the PR: The only feature sys/event is missing is priorities to be used as it is for this PR, right?

As I said before, interrupts themselves have priorities, so the HW is already handling these cases for you, provided one can do a bit more from interrupt context (which is not the case today).

Another alternative would be to emulate what the hardware does for handling interrupts: not have any priority queue and have each driver statically reserve a "vector" or handler.

[maribu]

As I said before, interrupts themselves have priorities, so the HW is already handling these cases for you [...]

I'm not so sure about that. Most drivers will just set up an interrupt using gpio_init_init(). When the interrupt occurs, some drivers will than have to handle a fast SPI transfer, others maybe a not-too-slow I2C transfer, and others might need to perform a slow I2C transfer in which the external hardware stretches the clock - potentially for ages - before it will finally sends the reply.

I cannot see how the hardware can help me to decide in which category the interrupt falls. As far as I know RIOT does not support to specify the priority of an GPIO interrupt. Does every MCU supports assignment of different priorities of GPIO interrupts, and do they so in a more or less uniform way that allows wrapping a platform independent API over it?

In any way, this would not solve the second issue: If a low priority interrupt already is executed in the single interrupt handler thread, a high priority interrupt will not preempt the low priority driver. (Of course, the interrupt service routine will, but sending an event to the interrupt handler thread will not cause it to save its current state, perform the high priority task outside of interrupt context, and resume the low priority task afterwards.)

provided one can do a bit more from interrupt context (which is not the case today).

I don't see how this would change. Especially how shared resources like an SPI or I2C bus can be used deadlock-free in interrupt context. At least not without adding some magic that would be hard to understand, hard to maintain, and hard to debug. And I don't believe it will come for free regarding RAM/ROM requirements either. I freely admit that I cannot do a fair evaluation of that with only having one of the two approaches implemented.

[maribu]

Back to the PR:

I believe in the end two interrupt service threads will be required: One "fast" handler in which the time required for handling an event has an upper bound, and one "slow" handler in which events are handled in a best-effort way. (Both could be a pseudo-modules, so that when only "fast" drivers are used only the fast handler is used, or when only "slow" drivers are used only the best-effort thread would be enabled at compile time.)

I'm not sure if priority event queues are still needed with this separation. Most of the time the fast event queue will be no longer than one element. And even if we're unlucky, it will still quite likely perform the events in front of the queue fast enough for the last event to still be handled in time. And in the other queue there would be no harm if an event is handled a few dozen milliseconds later than expected, as those drivers are prepared for a "best-effort" handling.

[maribu]

I think I will have time to test on Monday

[gschorcht]

@maribu Since I had some trouble with atomic_int on some platforms, I finally decided to define i as a normal integer variable. From my point of view, it is neither necessary that i is atomic, nor that i is volatile in the test case.

[maribu]

From my point of view, it is neither necessary that i is atomic, nor that i is volatile in the test case.

The problem is that the C language has not understanding of concurrent code (except for that atomic library). A C compiler is allowed to compile the program to anything that has the same observable behavior the c abstract machine would show when executing the code. But when the C abstract machine was defined, no concurrency was taking into account.

E.g. if we have two concurrent contexts: The main thread and an ISR. Lets say the the main thread would loop until work has been queued by the ISR. And the ISR would queue work on interrupts. We could implement this something like this:

unsigned items_in_loop = 0;
void isr(void) {
    enqueue_work();
    items_in_loop++;
}

void main(void) {
    while (1) {
        while (items_in_loop == 0) { /*wait for work */ }
        do_work(dequeue_work());
    }
}

In this case the compiler is allowed to replace while (items_in_loop == 0) {} with while (1) {}, as these are from the no-concurrency perspective of the abstract machine the same: items_in_loop is never changed (when ignoring the concurrent ISR context into account), so there is no need to ever check its value again.

Or also valid would be to transform while (items_in_loop == 0) {} with:

    register int tmp = items_in_loop;
    while (tmp == 0) { }

The same issue applies to i in your test: From the main-thread context it set to zero in line 115, never accesses from that context until 133 where it is checked to be different from zero. The compiler is therefore allowed to assume the value has never changed between and can elide the check.

If I didn't overlook anything, i is only accessed from thread context and never from interrupt context. So you could just wrap the accesses in mutex_lock() and mutex_unlock(). That will disallow the compiler from harmfully optimizing the accesses to i and make them atomic.

(Btw: The accesses to i have to be atomic, look at the example in my previous message on how it could go wrong on AVR otherwise.)

[maribu]

From my point of view, it is neither necessary that i is atomic, nor that i is volatile in the test case.

You're 100% right. I forgot that the interrupt services are not really called in interrupt context. So without LTO this should be perfectly safe. But I think with LTO still mischief could happen.

In any case: I would like the test to actually call irq_event_add() from interrupt context, as this is explicitly the use case for it. How about changing the test as below (click on details)?

diff --git a/tests/sys_irq_handler/main.c b/tests/sys_irq_handler/main.c
index 0f1004d32b..c867233375 100644
--- a/tests/sys_irq_handler/main.c
+++ b/tests/sys_irq_handler/main.c
@@ -33,11 +33,34 @@
 #include <stdio.h>
 
 #include "irq_handler.h"
+#include "mutex.h"
 #include "thread.h"
 #include "xtimer.h"
 
+#define TEST_TIME                   (100 * US_PER_MS)
+#define TEST_REPETITIONS            (10)
+
+static void _int1_service (void *arg);
+static void _int2_service (void *arg);
+
 static char some_stack[THREAD_STACKSIZE_MAIN];
 static int i = 0;
+static int n_int1_handled = 0;
+static int n_int2_handled = 0;
+static int n_already_pending = 0;
+static mutex_t mutex = MUTEX_INIT;
+xtimer_t timer1a = {
+    .callback = _int1_service,
+    .arg = &timer1a
+};
+xtimer_t timer1b = {
+    .callback = _int1_service,
+    .arg = &timer1b
+};
+xtimer_t timer2 = {
+    .callback = _int2_service,
+    .arg = &timer2
+};
 
 /* preallocated and initialized interrupt event objects */
 static irq_event_t _int1_event = IRQ_EVENT_INIT;
@@ -51,14 +74,19 @@ static void _int1_service (void *arg)
     /* registers just the interrupt event and returns */
     if (irq_event_add(&_int1_event) == -EALREADY) {
         puts("int1 is already pending");
+        n_already_pending++;
     }
 }
 
 /* interrupt handler for interrupts from source 1 */
 static void _int1_handler(void *ctx)
 {
-    (void)ctx;
+    xtimer_t *timer = ctx;
+    xtimer_set(timer, TEST_TIME);
     puts("int1 handled");
+    mutex_lock(&mutex);
+    n_int1_handled++;
+    mutex_unlock(&mutex);
 }
 
 /* interrupt service routine for a simulated interrupt from source 2 */
@@ -69,14 +97,19 @@ static void _int2_service (void *arg)
     /* registers just the interrupt event and returns */
     if (irq_event_add(&_int2_event) == -EALREADY) {
         puts("int2 is already pending");
+        n_already_pending++;
     }
 }
 
 /* interrupt handler for interrupts from source 2 */
 static void _int2_handler(void *ctx)
 {
-    (void)ctx;
+    xtimer_t *timer = ctx;
+    xtimer_set(timer, TEST_TIME);
     puts("int2 handled");
+    mutex_lock(&mutex);
+    n_int2_handled++;
+    mutex_unlock(&mutex);
 }
 
 static void *some_thread(void *arg)
@@ -86,7 +119,9 @@ static void *some_thread(void *arg)
     puts("some_thread is starting");
 
     while (1) {
+        mutex_lock(&mutex);
         i++;
+        mutex_unlock(&mutex);
         xtimer_usleep(US_PER_MS);
     }
     return NULL;
@@ -94,13 +129,11 @@ static void *some_thread(void *arg)
 
 int main(void)
 {
-    int k = 10;
-
     _int1_event.isr = _int1_handler;
-    _int1_event.ctx = (void*)sched_active_thread;
+    _int1_event.ctx = &timer1a;
 
     _int2_event.isr = _int2_handler;
-    _int2_event.ctx = (void*)sched_active_thread;
+    _int2_event.ctx = &timer2;
 
     puts("START");
 
@@ -114,25 +147,24 @@ int main(void)
 
     i = 0;
 
-    while (k--) {
-        /*
-         * Simulate two interrupt sources where one interrupt source generates
-         * two interrupts.
-         */
-        _int1_service(0);
-        _int2_service(0);
-        _int1_service(0);
-
-        /*
-         * Suspend the main thread, interrupts should be handled. After handling
-         * the interrupts, some thread should be executed and should increment
-         * i.
-         */
-        xtimer_usleep(500 * US_PER_MS);
-    }
-    if (i > 0) {
+    xtimer_set(&timer1a, TEST_TIME);
+    xtimer_set(&timer1b, TEST_TIME);
+    xtimer_set(&timer2, TEST_TIME);
+
+    xtimer_usleep(TEST_TIME * TEST_REPETITIONS + TEST_TIME/2);
+
+    xtimer_remove(&timer1a);
+    xtimer_remove(&timer1b);
+    xtimer_remove(&timer2);
+
+    mutex_lock(&mutex);
+    if ((i > 0) &&
+        (n_int1_handled == TEST_REPETITIONS) &&
+        (n_int2_handled == TEST_REPETITIONS) &&
+        (n_already_pending == 1)) {
         puts("[SUCCESS]");
     }
+    mutex_unlock(&mutex);
 
     return 0;
 }

[gschorcht]

@maribu Great, I changed it according to your suggestion.

[maribu]

All looks good to me. Please squash :-)

[gschorcht]

@Squashed and static tests are green.

[maribu]

I'd squash the second commit, which only adds some lines to a makefile, into the first commit.

[maribu]

Tested again, still works :-)

ACK

[maribu]

(Some outdated inline commend from me just appeared out of nowhere. I'm not sure what happened, but maybe my browser cached some text field content and inserted it when I wrote the ACK? I hope I could delete it before it cause any confusion.)

[gschorcht]

@maribu Thanks a lot for reviewing, discussing, contributing and of course testing this PR

[maribu]

You're very welcome. Thanks for the contribution. This is really useful for a lot of drivers :-)