Changing Cortex-M PendSV priority and handling in mutex.c

Hi all,

In Cortex-M MCUs, the PendSV interrupt is used to implement the actual context switch. The way that it is currently used with RIOT causes instability with nRF52 SoftDevice. Fixing this seems to be quite easy (see below), but might cause instability elsewhere.

My apology for this unfortunately quite long mail. However, I think we need to understand the background and discuss the details.

Rationale and background

Hi Pekka,

1. Having PendSV at the same priority with the others, as today, causes instability with nRF52 SoftDevice

I encountered similar trouble when integrating the softdevice the first time. Back then I used the SOFTDEVICE_PRESENT define to skip all RIOT specific ISR priority meddling. Is that not working anymore?

Exactly why PendPV at the same priority with other interrupts causes instability, I don't know.

That's the trouble with system-level blobs. I suggest asking Nordic about it.

2. Having PendSV at the lowest priority increases performance

The PendSV interrupt is designed to be used to perform the actual context switch just before returning from an interrupt to a normal thread context. The basic idea is to switch the thread stack and context only when there are no more interrupts pending. If there are still pending interrupts, running them may cause a higher priority thread to become ready to run. If so, with the current RIOT practice, PendSV may first change the thread, then, without this thread getting any running time, the next pending interrupt may make another thread runnable and trigger another PendSV, causing another thread change. The first thread change is wasted.

The reason pendsv has the same priority is that if had a lower one, other ISRs would preempt it. That in turn messes up the scheduler, unless it disables IRQs while it is running. It also increases ISR stack requirements. Back then we decided that two ISR's occuring at the same time is a corner case and not worth the price of the extra instructions for disable/enable ISRs for *every* context switch. The actual number of cycles spent in sched_run and saving/restoring thread state is <200 cycles. If less than every 100th ISR (<1%) happens to trigger a wasted context switch, saving the two instructions for disable/reenable ISRs in pendsv saves more instructions overall.

Kaspar and Oleg, you seem to have changed this in #3511 [5]. Looking at #3450 [6], I think the main problem was that PendSV calls sched_run(). But, looking at isr_pendsv(), I don't see it calling sched_run(). Only isr_svc() does.

pendsv jumps into svc at it's end. :wink:

So, maybe it is time to revert that change?

We can certainly reconsider.

I think there's also the concept of ISR priority groups, with group priority and subpriority within groups. The former control preemption, the latter control the order of execution should the group priority be the same. Maybe this can be exploited, by putting all ISRs in the same group priority but give pendsv the lowest subpriority.

Kaspar

1. Having PendSV at the same priority with the others, as today, causes instability with nRF52 SoftDevice

I encountered similar trouble when integrating the softdevice the first time. Back then I used the SOFTDEVICE_PRESENT define to skip all RIOT specific ISR priority meddling. Is that not working anymore?

While porting to the nRF SDK 15.0 and SoftDevice 6.0, I needed to rework quite extensively what SOFTDEVICE_PRESENT means in detail terms. Among other details, I needed to change the RIOT CPU_DEFAULT_IRQ_PRIO to 6. Without that basically all attempts to use xtimer caused either blocking threads or hardfaults, sooner or later.

(Yes, I know, Feature update n rf5 sdk 15.0.0 by pekkanikander · Pull Request #9473 · RIOT-OS/RIOT · GitHub is still pending. But it looks like that I will need to fight still quite some time with getting it right and making some smaller PRs before I can go back to it.)

2. Having PendSV at the lowest priority increases performance

So, maybe it is time to revert that change?

We can certainly reconsider.

I think there's also the concept of ISR priority groups, with group priority and subpriority within groups. The former control preemption, the latter control the order of execution should the group priority be the same. Maybe this can be exploited, by putting all ISRs in the same group priority but give pendsv the lowest subpriority.

I changed to using ISR subpriorities of one bit, so that most RIOT interrupts at 6 and PendSV at 7 now have the same priority but PendSV is always executed in the end. So far the system seems stable. :slight_smile:

But I've been running the system with subpriorities now just for less than an hour. Hence, I cannot say much about the real stability yet. Already before adopting subpriorities, but with cpsid and cpsie in isr_svc(), the system was semistable, never crashing but still xtimer_*sleep() blocking, sometimes soon, sometimes only after hours.

While testing with various options and debugging quite a lot, I've seen the variants of xtimer_*sleep both to crash and block a lot. Hence, I strongly suspect that there are some latent bugs there. But I don't properly understand what is going on there. Most likely some interrupt landing in the middle of the PendSV handler somehow interacts with their mutex use, easily causing blocking and easily also corrupting the stacks.

My student had so big problems with xtimer_*sleep related instabilities that he reverted to use xtimer_set_msg instead.

If the current approach turns out to be stable, I'll update #9373 to use this at some point. That won't be enough to make it pass, since apparently there are still some bugs in it in IPSP as well.

--Pekka

But I've been running the system with subpriorities now just for less than an hour. Hence, I cannot say much about the real stability yet. Already before adopting subpriorities, but with cpsid and cpsie in isr_svc(), the system was semistable, never crashing but still xtimer_*sleep() blocking, sometimes soon, sometimes only after hours.

While testing with various options and debugging quite a lot, I've seen the variants of xtimer_*sleep both to crash and block a lot. Hence, I strongly suspect that there are some latent bugs there. But I don't properly understand what is going on there. Most likely some interrupt landing in the middle of the PendSV handler somehow interacts with their mutex use, easily causing blocking and easily also corrupting the stacks.

My student had so big problems with xtimer_*sleep related instabilities that he reverted to use xtimer_set_msg instead.

Unfortunately I sent the message a little bit too early. I still encounter threads, which are using xtimer_usleep, rarely and ramdomly blocking, most probably at the second mutex_lock call in _xtimer_tsleep().

I don't have right now time to dig deeper into that but will rewrite those threads to use xtimer_set_msg instead.

--Pekka

While testing with various options and debugging quite a lot, I've seen the variants of xtimer_*sleep both to crash and block a lot. Hence, I strongly suspect that there are some latent bugs there. But I don't properly understand what is going on there. Most likely some interrupt landing in the middle of the PendSV handler somehow interacts with their mutex use, easily causing blocking and easily also corrupting the stacks.

My student had so big problems with xtimer_*sleep related instabilities that he reverted to use xtimer_set_msg instead.

Unfortunately I sent the message a little bit too early. I still encounter threads, which are using xtimer_usleep, rarely and ramdomly blocking, most probably at the second mutex_lock call in _xtimer_tsleep().

I don't have right now time to dig deeper into that but will rewrite those threads to use xtimer_set_msg instead.

My situation is worse than I thought. Apparently any thread can get blocked, typically at "bl rx" or "bl mutex" state. Other threads continue normally when this happens. Hence, so far I have always had shell access there. Previously I thought that the threads block since messages get somehow lost, but apparently that is not the case.

For our demo in the next week I don't see any other way than have a watchdog that reboots the system if any of the main threads gets blocked.

In the meanwhile, I have to learn how to use the debugger to get a stack trace of the blocked threads...

--Pekka