Resource protect in FromISR* functions?

Kernel
1

swingboard wrote on Tuesday, May 21, 2019:

in queue.c there are two implementations of “IsQueueEmpty” as the follows:

static BaseType_t prvIsQueueEmpty( const Queue_t *pxQueue )
{
BaseType_t xReturn;

    taskENTER_CRITICAL();
    {
        if( pxQueue->uxMessagesWaiting == ( UBaseType_t )  0 )
        {
            xReturn = pdTRUE;
        }
        else
        {
            xReturn = pdFALSE;
        }
    }
    taskEXIT_CRITICAL();

    return xReturn;
}
/*-----------------------------------------------------------*/

BaseType_t xQueueIsQueueEmptyFromISR( const QueueHandle_t xQueue )
{
BaseType_t xReturn;

    configASSERT( xQueue );
    if( ( ( Queue_t * ) xQueue )->uxMessagesWaiting == ( UBaseType_t ) 0 )
    {
        xReturn = pdTRUE;
    }
    else
    {
        xReturn = pdFALSE;
    }

    return xReturn;
} /*lint !e818 xQueue could not be pointer to const because it is a typedef. */

prvIsQueueEmpty will disable interrupts by taskENTER_CRITICAL and enable interrupts by taskEXIT_CRITICAL, but xQueueIsQueueEmptyFromISR without any protection.

here is my assumption:
1, my system have interrupt being nested, eg, interrupt level 5 will preempt the interrupt level 6.
2, when xQueueIsQueueEmptyFromISR is called in interrupt level 6(low priority), and xQueueGenericSendFromISR being called in interrupt level 5(high priority).
3, when xQueueIsQueueEmptyFromISR execute:
3-1, fetch ( Queue_t * ) xQueue )->uxMessagesWaiting
3-2, do compare if (( Queue_t * ) xQueue )->uxMessagesWaiting == 0)
4, if 3-1 is done, but it is preempted by interrupt level 5 which call xQueueGenericSendFromISR which increase the uxMessagesWaiting + 1, but 3-2 is not know this as it have save the value of ( Queue_t * ) xQueue )->uxMessagesWaiting, it is the inconsistent of the value of ( Queue_t * ) xQueue )->uxMessagesWaiting.
5, is it a problem and how to avoid it?

my assumption is that xQueueIsQueueEmptyFromISR should add interrupt protection such as portSET_INTERRUPT_MASK_FROM_ISR before it do 3-1 opration

2

richarddamon wrote on Tuesday, May 21, 2019:

My first feeling is that the only time we need the critical section is if retreiving the uxMessagesWaiting value is not an atomic operation. It may be that the only machines that condition can happen are machines that don’t have nested interrupts (old 8-bit processors)

The interrupt between 3-1 and 3-2 isn’t important (only an interrupt within 3-1 would be) as if it WAS protected by a critical section, all that would happen is that the interrupt would be delayed until the critical section was done, so the answer could still be ‘wrong’. Any sort of empty/full test always has this issue, something can happen between the test and using the results unless you wrap the test and using that result in some form of protection.

3

rtel wrote on Tuesday, May 21, 2019:

The interrupt between 3-1 and 3-2 isn’t important (only an interrupt
within 3-1 would be) as if it WAS protected by a critical section, all
that would happen is that the interrupt would be delayed until the
critical section was done, so the answer could still be ‘wrong’.

Exactly - the value is only at a point in time, it might completely
change by the time the value is actually used outside the function,
unless the function itself is also called within a critical section.

4

swingboard wrote on Wednesday, May 22, 2019:

yeah, the prvIsQueueEmpty is no problem as it fetch&& compare the ‘uxMessagesWaiting’ in the critical section protection, in my opinion, prvIsQueueEmpty’s operation is atomic.

what i really care is the xQueueIsQueueEmptyFromISR, firstly it have no critical section protection and it may be interrupt by other higher priority interrupt which would call some API to change the ‘uxMessagesWaiting’ between 3-1 and 3-2, after the higher priority interrupt return to xQueueIsQueueEmptyFromISR function, the ‘uxMessagesWaiting’ is not the newest to the next 3-2’s compare statement.

as i think xQueueIsQueueEmptyFromISR is not atomic and it is called in ISR so it may need add taskENTER_CRITICAL_FROM_ISR or portSET_INTERRUPT_MASK_FROM_ISR to protoct the ‘uxMessagesWaiting’.

am i think right?

5

swingboard wrote on Wednesday, May 22, 2019:

yeah, if the xQueueIsQueueEmptyFromISR have a critical section protection such as taskENTER_CRITICAL_FROM_ISR or portSET_INTERRUPT_MASK_FROM_ISR, there would no the problem.

but why the freertos’s offical version doesn’t consider this, if it is a bug?

6

richarddamon wrote on Wednesday, May 22, 2019:

The atomic question is the fetch of the value of uxMessagesWaiting value, which is a simple number. As long as reading that number is atomic, the function is as atomic as it can be, and adding a critical section buys nothing.

If anything, I would question why the non-ISR version has the critical section. Unless uxMessagesWaiting is large enough that it can’t be loaded with a single assembly instruction (perhaps being a ‘long’ sized number) there is no issue with it.

My only guess as to a case which needed the code would be a primative, maybe 8 bit processor, that can’t read a 16 bit value atomically, which likely wouldn’t have nested interrupts, and thus not need the protection in the ISR.

If the application needs to be able to test and act atomically, then the application needs the critical section, and no critical section inside the function will help.