Interrupts are enabled as a side effect

Kernel
1

nobody wrote on Friday, March 03, 2006:

In the ARM7 ports, the implementation of portENTER_CRITICAL/portEXIT_CRITICAL is done in a way which forces the interrupts to enabled state after calling portEXIT_CRITICAL. This is an unfortunate side-effect in my opinion.
Looking at the AVR port for comparison, the interrupt state is saved by portENTER_CRITICAL and restored by portEXIT_CRITICAL to the original state, which I think is more appropriate.
What this means for the ARM7 port is that you cannot call simple functions like xTaskGetTickCount() from an ISR without the side effect that interrupts are enabled. xTaskGetTickCount() calls portENTER_CRITICAL/portEXIT_CRITICAL internally.
The consequences of enabling interrupts inside an ISR, which means nested interrupts, are often fatal.
Can anyone suggest a safe way of implementing portENTER_CRITICAL/portEXIT_CRITICAL for ARM7 which restores the original interrupt state?

Regards,

Lars Kjellberg
Embedit A/S

2

nobody wrote on Friday, March 03, 2006:

Which port are you looking at?  The ARM ports store the interrupt state in a variable called uxCriticalNesting.  Exiting a critical section only enables interrupts if critical nesting is zero.

When restoring a context the interrupt state returns to what ever it was when the task context was saved as the interrupt enable bit is in the CSR that was saved as part of the task context.

This is my understanding so let me know if this is wrong.

Nesting interrupts is not necessarily a good thing IMHO.  Better to have a simple quick ISR that simply does nothing but clears the interrupt and passes an event to a task for processing (with interrupts enabled).

Nesting can be achieved but requires consideration.  When nesting interrupts must be disabled around at least the increment tick and switch context functions of the task.c code itself.

Comments?

3

nobody wrote on Friday, March 03, 2006:

Well, uxCriticalNesting does not hold the interrupt state as such. It counts the nesting of calls to portENTER_CRITICAL. When uxCriticalNesting reaches "zero" (or the equivalent numerical value), interrupts are force enabled - interrupts are not restored to the state from when portENTER_CRITICAL was called in the first place! This goes as far as I can see for all the ARM7 ports.

4

nobody wrote on Friday, March 03, 2006:

Well, uxCriticalNesting does not hold the interrupt state as such. It counts the nesting of calls to portENTER_CRITICAL. When uxCriticalNesting reaches "zero" (or the equivalent numerical value), interrupts are force enabled - interrupts are not restored to the state from when portENTER_CRITICAL was called in the first place! This goes as far as I can see for all the ARM7 ports.

5

rtel wrote on Friday, March 03, 2006:

Each task maintains its own critical section nesting value. 

If a task is suspended while interrupts are disabled by a call to portENTER_CRITICAL() then when the same task is resumed again interrupts will once again be disabled.  This is not to say that interrupts necessarily remain disabled between the task being suspended and then resumed.  It is possible that in between a task executes that has interrupts enabled.

Likewise, if interrupts are enabled when a task is suspended, when the same task executes again interrupts will still be enabled.

The critical section nesting variable does in effect hold the interrupt state ut only indirectly.  When it is zero interrupts are enabled.  When it is not zero interrupts are disabled.  It is done on a per task basis however.

Regards.

6

nobody wrote on Monday, March 06, 2006:

Well, I made a work-around, which is basically the following:

1) I reverted the implementation of portENTER_CRITICAL and portEXIT_CRITICAL to that of FreeRTOS release 2.5.1. This release stores the previous interrupt state on the stack. However, it is also documented that GCC’s optimization causes this approach to fail.
2) By trial and error, I found that it is GCC’s -Os option which breaks the stack layout. -Os is equivalent to -O2 minus 6 specific optimizations plus some extra space-saving optimizations. I then tried the combination of -O2 and disabling the 6 specific optimizations which is done implicitly by -Os. The list of options is:

-O2 -fno-align-functions -fno-align-jumps -fno-align-loops -fno-align-labels -fno-reorder-blocks -fno-prefetch-loop-arrays

This combination works like a charm, and the code space penalty compared to -Os is only around 2%. I can certainly live with 2% extra code if I can call xTaskGetTickCount() from within my ISR.

7

nobody wrote on Monday, March 06, 2006:

I like the information on the optimisation options but still don’t understand the problem that you are trying to resolve.  Does the code not already do this, albeit with some trade offs compared to your technique.  What exactly is the problem with the code as it is.

I would really appreciate it if you could explain it again for me.

8

nobody wrote on Wednesday, March 08, 2006:

Ok, I’ll try again :slight_smile:

The problem is that when exiting from the critical region, interrupts always get enabled - even if they were already disabled on entry to the critical region.
Ok, so you may ask why do I want to enter a critical region when interrupts are already disabled? Well, I use a few utility functions which make use of critical region. These can be called from both interrupt and normal context. I don’t want to implement an interrupt and a normal context version of each such function.

As another example I want to read the current timer tick value within my ISR by calling xTaskGetTickCount. But since xTaskGetTickCount() uses a critical region internally, interrupts are enabled on return - i.e. still within my ISR. That was what I referred to as a side effect. I means that I get nested interrupts, which my application is not at all designed to handle. And again, I wouldn’t like a solution with a specific ISR version of xTaskGetTickCount(), like xTaskGetTickCountFromISR(), which would not need the internal critical region.

To summarize, there are 2 different issues:

1. The current ARM7 port does not restore the interrupt state to its true previous state when exiting from a critical region. This is what i fixed by reverting to the implementation of FreeRTOS version 2.5.1.
2. Saving the interrupt state on the stack is potentially unsafe and very much depending on compiler and optimization setting. The results of my trial-and-error experiments above might not even be valid with the next GCC version. Maintaining a list of valid optimization settings per compiler is a continous effort.

To prove the risk of saving interrupt state on the stack, I can supply another example:
Yesterday I built a FreeRTOS application for an AVR target using the IAR compiler verion 4.12. For certain combinations of optimization, that application also crashes due to stack corruption caused by the critical region macros. The crash typically happens already in the function vTaskStartScheduler().

My suggestion for a long-term solution is to change the programming interface to portENTER_CRITICAL and portEXIT_CRITICAL in such a way that portENTER_CRITICAL returns the current interrupt state, which the application function must store in its own local variable. This state variable must be supplied as a parameter to portEXIT_CRITICAL, which will restore the interrupt state to the value of this parameter. In other words: The developer must supply his own storage for the interrupt state instead of FreeRTOS trying to save it on the stack.
Given this interface, it will be possible to revert the ARM7 implementation to that of release 2.5.1 and thus avoid the nesting counter.

Regards,

Lars Kjellberg

9

nobody wrote on Wednesday, March 08, 2006:

Comments are included below.

>The problem is that when exiting from the critical region, interrupts always
>get enabled - even if they were already disabled on entry to the critical
>region.

This can only be the case if interrupts were disabled by some means other than a call to portENTER_CRITICAL().

>Ok, so you may ask why do I want to enter a critical region when interrupts
>are already disabled? Well, I use a few utility functions which make use of
>critical region. These can be called from both interrupt and normal context.
>I don’t want to implement an interrupt and a normal context version of each
>such function.

Starting to make sense now.  You want to use the ENTER/EXIT from within an ISR without the EXIT enabling interrupts.

>As another example I want to read the current timer tick value within my ISR
>by calling xTaskGetTickCount. But since xTaskGetTickCount() uses a critical
>region internally, interrupts are enabled on return - i.e. still within my ISR.
>That was what I referred to as a side effect. I means that I get nested
>interrupts,

Now I understand :slight_smile:

>To summarize, there are 2 different issues:
>
>1. The current ARM7 port does not restore the interrupt state to its true
>previous
>state when exiting from a critical region. This is what i fixed by reverting
>to the implementation of FreeRTOS version 2.5.1.
>2. Saving the interrupt state on the stack is potentially unsafe and very much
>depending on compiler and optimization setting. The results of my
>trial-and-error
>experiments above might not even be valid with the next GCC version.
>Maintaining
>a list of valid optimization settings per compiler is a continous effort.

The documentation should be more specific on the effect if used from within an ISR.

>To prove the risk of saving interrupt state on the stack, I can supply another
>example:
>Yesterday I built a FreeRTOS application for an AVR target using the IAR
>compiler
>verion 4.12. For certain combinations of optimization, that application also
>crashes due to stack corruption caused by the critical region macros. The crash
>typically happens already in the function vTaskStartScheduler().
>
>My suggestion for a long-term solution is to change the programming interface
>to portENTER_CRITICAL and portEXIT_CRITICAL in such a way that
>portENTER_CRITICAL
>returns the current interrupt state, which the application function must store
>in its own local variable. This state variable must be supplied as a parameter
>to portEXIT_CRITICAL, which will restore the interrupt state to the value of
>this parameter. In other words: The developer must supply his own storage for
>the interrupt state instead of FreeRTOS trying to save it on the stack.
>Given this interface, it will be possible to revert the ARM7 implementation
>to that of release 2.5.1 and thus avoid the nesting counter.

This is in general I think a good idea but at the expense of the application writter having more responsibility.

What do other people think?

10

nobody wrote on Thursday, March 09, 2006:

I think I can live with the way it is.
A function that enables interrupts globally should
be documented that way.
Maybe it’s not necessary to use xTaskGetTickCount()
and critical nesting in an ISR?