ulTaskNotifyTake: taskENTER_CRITICAL with portYIELD_WITHIN_API

Austin · June 30, 2021, 10:06am

If a task switches out form within a critical section (so with interrupts masked), the next time it is switched back in interrupts will again be disabled again (when the critical nesting depth gets restored) before it starts to run - then it will exit the critical section to re-enable interrupts. The net effect is therefore the same as for the asynchronous method but the context switch occurs the other side of exiting the critical section.

For Cortex A53 with “svc 0”, when context switch occurs, the interrupt is masked. Before someone notify and wake up it, it causes side effect to IRQ trigger. This is my puzzle here.

Austin · June 30, 2021, 10:09am

Hi
On Cortext A53, if App calls ulTaskNotifyTake, it will call portYIELD_WITHIN_API which yields in critical section by “svc 0”. Although App doesn’t directly call portYIELD_WITHIN_API, portYIELD_WITHIN_API is indirectly called through ulTaskNotifyTake

hs2 · June 30, 2021, 11:18am

I know. Again FreeRTOS code takes care about the internals and (usually) works as specified. You’re still supposing that the FreeRTOS ulTaskNotifyTake implementation is broken and doesn’t work, right ?
You didn’t tell the exact problem you have. Is the ISR is never triggered or just sometimes and you miss some interrupts ? How did you verify that the ISR is not triggered as expected ? Did you set a breakpoint in the debugger or use a test flag/counter ?

Austin · June 30, 2021, 1:02pm

I know. Again FreeRTOS code takes care about the internals and (usually) works as specified. You’re still supposing that the FreeRTOS ulTaskNotifyTake implementation is broken and doesn’t work, right ?

It is code analysis. I had doubt about ulTaskeNotifyTake on Context A53. The code switches out (svc 0) in critical section and IRQ is masked.

After digging the code more with the statement from Richard, I am clear now. More specifically, “IRQ is masked” is only in a short time, it is unmasked by the following context switch

The code is well designed and understandable now. Thank you all for the answers.

If a task switches out form within a critical section (so with interrupts masked), the next time it is switched back in interrupts will again be disabled again (when the critical nesting depth gets restored) before it starts to run - then it will exit the critical section to re-enable interrupts. The net effect is therefore the same as for the asynchronous method but the context switch occurs the other side of exiting the critical section.

ghu3 · July 3, 2021, 1:52am

Hi, Hartmut
The portable layer is CoreTex A53 with GIC v4.
The ulTaskNotifyTake which called the " [taskENTER_CRITICAL]" that masked the interrupt . This means all the IRQ with priority lower than configMAX_API_CALL_INTERRUPT_PRIORITY will not delivered to CPU from GIC.

Then in the critical section, the portYIELD_WITHIN_API will be called switch to other highest priority tasks by calling vTaskSwitchContext. There is no place to unmask the IRQ.

The timer interrupt will unmask the interrupt Mask but timer IRQ is blocked in this case also.

Is there any other place to unmask the interrupt?

BTW, if we used xQueueReceive/xQueueSendFromISR instead of ulTaskNotifyTake/vTaskNotifyGiveFromISR pairs, the code worked as expected.
Thanks.

richard-damon · July 3, 2021, 2:09am

For ports that use immediate calls to the scheduler (or via an non-masked SWI) the current interrupt mask is stored as part of the context, so when you switch to the new task, if that wasn’t also in a critical section, that switching will unmask the interrupts.

Look in the port layer context switch code to see this.

hs2 · July 3, 2021, 7:52am

For me it seems that there is still a problem (in your driver, HW ?) which might get hidden e.g. by different execution timings of queues vs. notifications or something else.
I guess (no code in front of me right now) that also in the FreeRTOS queue code portYIELD_WITHIN_API is invoked.
I afraid you’ve to find a reliable solution yourself. Also because you didn’t tell what exactly the problem is

ghu3 · July 3, 2021, 8:31am

Let me show the code. The code work if defined the “USE_QUEUE”. I wrote this code to demo the usage of xQueueReceive and ulTaskNotifyTake difference.

I think the issue is in the call of ulTaskNotifyTake. It called the task switch after mask interrupt. Critical section code should not sleep.

The Task side:

QueueHandle_t Global_Queue_Handle = 0;
TaskHandle_t receive_task_handle = NULL;
static void receive_task_user(void *p)
{
    int rx_int=0;

    (void)p;
    printf("start receive_task_user\n");
    mdelay(300);
    while(1) {
#ifndef USE_QUEUE
        if(xQueueReceive(Global_Queue_Handle, &rx_int, 1000)) {
#else
        if (ulTaskNotifyTake(pdTRUE, portMAX_DELAY) == 0) {
#endif
            printf("Received %d.\n", rx_int);
        } else {
            printf("Failed to receive from queue.\n");
        }

    }
}

static void test_queue_isr(void)
{
    Global_Queue_Handle = xQueueCreate(3, sizeof(int));

    /* Create Tasks */
    xTaskCreate(receive_task_user, "rx", 1024, NULL, 1, &receive_task_handle);
}

In the Timer IRQ handler:

    ......
    static uint32_t i = 0;
    BaseType_t xHigherPriorityTaskWoken = pdFALSE;

    if ( i % 20 == 0) {
        printf("Send %d to receiver task.\n", i);
#ifndef USE_QUEUE
        if(!**xQueueSendFromISR**(Global_Queue_Handle, &i, &xHigherPriorityTaskWoken)) {
            printf("Failed to send to queue.\n");
        }
#else
        printf("send notification from timer\n");
        **vTaskNotifyGiveFromISR**(receive_task_handle, &xHigherPriorityTaskWoken);
#endif
    }

	portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
        .......

RAc · July 3, 2021, 8:58am

Just as a side question, do you always do diagnostic output via printf() even in your ISR? You shouldn’t.

EDIT: Why not? That’s been answered about a bazillion times before, but just for completeness’s sake:

Owing to the Heisenberg effect, you should generally not use printf anywhere if you need to analyse real time behavior (in terms of stack usage and additional cycles, it significantly changes the runtime behavior of your application). In your case, the interrupt stack may overflow and/or the timing (which by definition is crucial in ISRs) will be severely affected.
Unless you have full control over the full control flow of your printf() routine, you risk that it does things down the call chain that are not allowed in ISRs such as attempting to claim muteces.

hs2 · July 3, 2021, 9:18am

Sorry - I still don’t know what the problem is… Do you miss some notifications ? Do you never get a notification ?
I didn’t use notifications as binary semaphore yet. But the API docs tell that ulTaskNotifyTake returns the value before it’s cleared/decremented and you check for 0 being returned. That doesn’t seem right
Could you just omit checking the return value and retry ?
BTW I also second RAc’s comment regarding printf in ISRs.

ghu3 · July 3, 2021, 12:32pm

The problem is in the implement of ulTaskNotifyTake that it masked the interrupt in the critical section. And our interrupt handler will not be triggered and there is a deadlock.

ulTaskNotifyTake equal to ulTaskGenericNotifyTake
FreeRTOS-Kernel/blob/main/tasks.c

The deadlock is happen in this way.

There is a task A which use ulTaskNotifyTake to get a notification from an interrupt B
In ulTaskNotifyTake, the interrupts are masked. Source code showed bellow.
The interrupt will never be delivered because it is masked. the vTaskNotifyGiveFromISR will not be called in the interrupt handler.
Deadlock happen.

    uint32_t ulTaskGenericNotifyTake( UBaseType_t uxIndexToWait,
                                      BaseType_t xClearCountOnExit,
                                      TickType_t xTicksToWait )
    {
        uint32_t ulReturn;

        configASSERT( uxIndexToWait < configTASK_NOTIFICATION_ARRAY_ENTRIES );

        taskENTER_CRITICAL();
        {
            /* Only block if the notification count is not already non-zero. */
            if( pxCurrentTCB->ulNotifiedValue[ uxIndexToWait ] == 0UL )
            {
                /* Mark this task as waiting for a notification. */
                pxCurrentTCB->ucNotifyState[ uxIndexToWait ] = taskWAITING_NOTIFICATION;

                if( xTicksToWait > ( TickType_t ) 0 )
                {
                    prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
                    traceTASK_NOTIFY_TAKE_BLOCK( uxIndexToWait );

                    /* All ports are written to allow a yield in a critical
                     * section (some will yield immediately, others wait until the
                     * critical section exits) - but it is not something that
                     * application code should ever do. */
                    portYIELD_WITHIN_API();

The taskEnter_CRITICALA() masked the interrupt.
portYIELD_WITHIN_API(); supposed to be wakeup by the interrupt handler. But it could not as interrupt will not happen after portYIELD_WITHIN_API();

ghu3 · July 3, 2021, 12:37pm

Hi, RAc
The printf is a lightweight implement for embedded system. No mutex in our implement.
I understand that it should not be used in production code or some timing related debug.
Thanks for pointed it out.

Thanks.

hs2 · July 3, 2021, 12:52pm

Even if interrupts are temporarily masked an interrupt occurred meanwhile is still pending and fires/invokes the ISR as soon as the interrupts are unmasked e.g. when finally leaving the critical section.
So no - there shouldn’t or can’t be a deadlock as you suppose in 4) just a probably small latency.
I don’t think that ulTaskGenericNotifyTake is broken in the way you describe that it masks interrupts and leaves them masked somehow forever.
This would fundamentally break task notifications in general and they couldn’t be used at all. But they’re working.
Next try: Could you please tell the actual problem/symptom you have when using notifications ? Do you get any notification from the ISR ? Do you sometimes miss timer events / notifications ? Is the timer periodic ?
Note that using a queue is semantically different to using a binary semaphore scheme (either using a real semaphore or a notification).

ghu3 · July 3, 2021, 1:44pm

Hi, Richard
In the task switch to unmask the IRQ will make the ulTaskNotifyTake work.

BTW, if task A yield the CPU in critical section, and task B unmask it during context switch, does this make sense? Why not unmask inside the ulTaskNotifyTake before yield?

richard-damon · July 3, 2021, 2:16pm

The current code for the task switch should be doing this, it is based on the guidelines for writing ports. Is this a supported port, or have you put together something of your own based on similar processors.

Why the code doesn’t ulTaskNotifyTake unmask itself? likely because it knows by the design rules of FreeRTOS ports that it doesn’t need to. Note the big comment right before it explaining what it is expecting. My guess is that allowing an interrupt between the prvAddCurrentTaskToDelayedList and the portYIELD_WITHIN_API() might cause issues.

ghu3 · July 3, 2021, 11:38pm

Hi, Richard
Thanks. I am writing a new port based on Cortex-A53 with new GIC V4. The current port in FreeRTOS is based on GIC v2. The interrupt part is re-written in my new port. The IRQ re-priority masked is missed in the new implement.

The “interrupt priority mask” is a per-task variable but actually there is one copy in system. It is a little sophisticated.

I don’t see any issues to unmask the IRQ before portYIELD_WITHIN_API() because the IRQ or task switch are similar in this case.

Or does it make sense to implememt portYIELD_WITHIN_API() as this?

taskEXIT_CRITICAL();
portYield();
taskENTER_CRITICAL();

richard-damon · July 3, 2021, 11:56pm

Sounds like your port may not be following the expectations of the system. My understanding is that for system with synchronous task switching (like via SWI, which isn’t masked), then the task switching code is supposed to save the current ISR mask level into the task context that gets saved, and load a new mack level from the task being switched in. This is basically required to allow the system to do the portYIELD_WITHIN_API correctly inside a critical section.

The issue is that the code enters the critical section and and adjust some of the task lists in a way that might not be suitable for a general ending of the critical section and arbitrary ISR code running that might change task status until the task is switched out by the yield operation.

The above code also might not work, as I don’t know if ALL usages of port_YIELD_WITHIN_API() are inside critical sections, which it is assuming.

ghu3 · July 4, 2021, 12:00am

Thanks very much. That explained the tricky part and why it is designed that way.

aggarg · July 6, 2021, 9:43pm

Did you see this port which is recently added: https://github.com/FreeRTOS/FreeRTOS-Kernel/tree/main/portable/GCC/ARM_CA53_64_BIT_SRE

Thanks.

ghu3 · July 7, 2021, 1:11am

I don’t know this. Will take a look at. Thanks.