vPortValidateInterruptPriority unnecessarily restrictive

ilgitano wrote on Tuesday, March 31, 2015:

I added the above as a feature request (#89) But I’m thinking it’s a bug. I am developing on an ARM cortex M3 device, IAR C compiler.

The issue for me is that I have a very high speed UART driver, with it’s priority set above FreeRTOS deliberately. It tests __get_BASEPRI() to see if FreeRTOS is in a critical section, and if not may make a call to SemGiveFromISR(), but if it is in a critical section it posts some info to a queue and calls SemGiveFromISR() when it has left the critical section.

This works well on 7.4.0, with configASSERT enabled. On 7.5.2 however, I get an assert failure. The assert is checking for an interrupt priority above FreeRTOS and failing the test.

It seems to me that a better fitting test here would not to be the priority level, but if the interrupt is attempting to make a call to FreeRTOS from within a critical section. That would allow high priority interrupts to call FreeRTOS services when possible, while catching poorly set up NVIC configurations just as effectively.

For my case, the interrupt simply empties/fills char buffers at high speed, notifying application code at much lower priority only when the buffer is empty/full, which is a much less frequent event. Can’t use a DMA because control codes are embedded which need to be processed in the interrupt (it’s a muxer)

Is it reasonable to call this a bug?

Is there a better way of doing this?

rtel wrote on Tuesday, March 31, 2015:

I think the assert() has saved many hundreds of people from having to try and debug intermittent and un-reproducible glitches in their applications, so I would not say it was unnecessarily restrictive. You can of course just not define configASSERT().

In your case, you are breaking the usage rules of the API, and that is generating an assert() exactly as intended. If the FAQ and API documents say not to do what you are doing, it is probably unfair to say it is a bug if the code tries to warn you of the fact.

Unfortunately your alternative suggestion, of checking if the code is in a critical section, will not work reliably (although it may seem to work for a while) because critical sections are implemented differently in the interrupt safe API - and the tick interrupt sets BASEPRI without going through the enter/exit critical functions (as to do so would take too long).

In your case I would suggest the following as an alternative: Find an interrupt that is not being used (the CPU will probably only use a handful of a lot of available interrupts). Set the unused interrupt to a lower priority (configMAX_SYSCALL_INTERRUPT_PRIORITY is OK). Then, in your high priority ISR, rather than write to a queue or semaphore, use the Cortex-M SETPENDING register to pend the unused interrupt. The interrupt will then execute immediately that you leave the critical section - you can then post to the queue or give the semaphore from the interrupt’s handler.


richard_damon wrote on Tuesday, March 31, 2015:

I will add that one BIG reason not to only flag the assert in the case of actually being in a critical section is that for most people, you want to catch the error condition quickly, and waiting for the actual overlap may make it never see the problem in development.

Since you are intentionally trying to bypass the restriction, and if using a second interrupt (at lower priority) doesn’t work for you, you could define the configASSERT macro to test a global variable (like i_know_what_i_am_doing) that is set by that ISR before calling the functions that it isn’t specified as being able to, and then clearing it after, and if the variable is set, the assert doesn’t really “fire”.

ilgitano wrote on Wednesday, April 01, 2015:

I am shifting the required calls - by the numbers, about 90% of the time - to a lower priority interrupt already. But I make the call immediately if I’m not in a critical section. I’m still seeing some data loss at 256 kbaud (thats 256baud in + 256kbaud out) but it’s rock solid at the required 115kbaud. I think that may be because I’m trying to do the RTOS call when I can. If I downshift ALL the required calls, it might be reliable up to 256kbaud.

In my testing, I am hammering it pretty hard. I can see that under normal development conditions, any bugs could be pretty rare, so Richards point is relevant.

I hadn’t considered the issue of other interrupts being in the alternate critical sections at the critical moment in my suggestion, as we don’t have any - yet. But it was on my mind when I decided on __get_BASEPRI() == 0. I think that should be < configMAX_SYSCALL_INTERRUPT_PRIORITY though.

We would ideally like to use an unmodified RTOS, and keeping the the asserts enabled in debug builds is enormously helpful, but this particular assert is not helpful. Would it perhaps be possible to break the asserts into two groups? ‘strict’ as in it WILL fail (eg NULL pointers, range checks) and ‘helpful’, as in ‘if you don’t know what you’re doing this might be a problem’? - along the lines of compiler errors and warnings?

rtel wrote on Wednesday, April 01, 2015:

Refresh my memory - How are you passing received data from the interrupt
to the task? If I recall correctly, placing the data in some kind of
RAM buffer, then just signalling the task when the buffer contains a
complete message. I think you said there was some reason why DMA could
not be used, hence I’m interested in how the data is being copied.
Also, how do you detect the end of a complete message - does that
require parsing the received characters inside the ISR?


ilgitano wrote on Thursday, April 02, 2015:

The interrupt fills up a circular queue. When a watermark is reached, that is when some desired amount amount of space, data or timeout occurs, then it queues up a function which will call SemaphoreGiveFromISR. This is a very small (10 entry) local queue. I had hoped to call FromISR directly if I wasn’t in a critical section, but seems that is not to be.

Both these queues, a char circular buffer and the semaphore notification queue, are not part of FreeRTOS.

The received stream is actually directed to one of several buffers, since the interrupt is a simple muxer. Embedded control codes need to be intercepted and interpreted/filtered on the fly, which I cannot do with a DMA.

Right now, I have a hacked version of taskENTER_CRITICAL/taskEXIT_CRITICAL which then calls all the queued FromISR functions. I will be using a pending SW interrupt as described in the first response above when I get to the tidying up phase. Don’t want any unnecessary modifications to FreeRTOS in this.

The muxer is a stream switcher, it just implements 2 control codes: switch channel (destination buffer) and insert literal channel code (so that binary can be sent completely transparently) The control codes are interpreted on the fly by the interrupt. No concept of end-of-message is implemented, just pointing streams of data at the right buffer.

Each of the buffers has a semaphore which is set by the high level code to notify on free space or data limits being reached, or timeout.