ARM-software / CMSIS-FreeRTOS

FreeRTOS adaptation for CMSIS-RTOS Version 2
https://arm-software.github.io/CMSIS-FreeRTOS/
Other
528 stars 141 forks source link

osThreadFlagsWait will sleep even if there is ready flags. #103

Closed swordligit closed 6 months ago

swordligit commented 6 months ago

Hi, Here is an example, there are two task, phaseA and clock: phaseA:

void phaseA (void *argument)
{
    for (;;) {
            uint32_t flags = osThreadFlagsWait(0x01000, osFlagsWaitAny, osWaitForever);    /* wait for an event flag 0x0001 */
            TRACE(0, "%s flags:%x", __func__, flags);
            osThreadFlagsSet(tid_clock, 0x0100);      /* set signal to clock thread    */
            TRACE(0, "%s begin wait 0x3", __func__);
            flags = osThreadFlagsWait(0x3, osFlagsWaitAny, osWaitForever);    /* wait for an event flag 0x0001 */
            TRACE(0, "%s again flags:%x", __func__, flags);
    }
}

thread clock:

void clock (void *argument)
{
    for (;;) {
        uint32_t flags = osThreadFlagsWait(0x0100, osFlagsWaitAny, osWaitForever);
        TRACE(0, "%s flags:%x", __func__, flags);
        TRACE(0, "%s 1st seting", __func__);
        osThreadFlagsSet(tid_phaseA,  0x1000);            /* set signal to thread 'phaseA' */
        TRACE(0, "%s 2nd seting", __func__);
        osThreadFlagsSet(tid_phaseA,  0x0100);            /* set signal to thread 'phaseA' */
        TRACE(0, "%s 3rd seting", __func__);
        osThreadFlagsSet(tid_phaseA,  0x0003);            /* set signal to thread 'thread phaseA' */
        osDelay(500);                             /* delay 500ms*/
    }
}

Here the task clock will set flags 0x1000, 0x0100 before set 0x0003,(which task phaseA is waiting). after set flag 0x0003, the phaseA will wake up and the get the flags.

The problem is after phaseA take the flags 0x0003, and then it will call osThreadFlagsWait again, but this time phaseA will going into block state, even there are 0x1000 | 0x0100 flags in its thread flags. The RTX5's implementation will just return the left flags(here is 0x1000|0x0100 = 0x1100) and will not go into block state. In the implementation of osThreadFlagsWait :

      rval = xTaskNotifyWait (0, clear, &nval, tout);

The cause maybe after xTaskNotifyWait return, it will set the

        pxCurrentTCB->ucNotifyState[ uxIndexToWait ] = taskNOT_WAITING_NOTIFICATION;

Then next time when call xTaskNotifyWait again, it first judge the ucNotifyState, if it is not taskNOTIFICATION_RECEIVED, it will go into block state:

            /* Only block if a notification is not already pending. */
            if( pxCurrentTCB->ucNotifyState[ uxIndexToWait ] != taskNOTIFICATION_RECEIVED )
                ...
                if( xTicksToWait > ( TickType_t ) 0 )
                {
                    prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );

But these are flags still in the task phaseA. Here is the test code:

void phaseA (void *argument)
{
    for (;;) {
            uint32_t flags = osThreadFlagsWait(0x01000, osFlagsWaitAny, osWaitForever);    /* wait for an event flag 0x0001 */
            TRACE(0, "%s flags:%x", __func__, flags);
            osThreadFlagsSet(tid_clock, 0x0100);      /* set signal to clock thread    */
            TRACE(0, "%s begin wait 0x3", __func__);
            flags = osThreadFlagsWait(0x3, osFlagsWaitAny, osWaitForever);    /* wait for an event flag 0x0001 */
            TRACE(0, "%s again flags:%x", __func__, flags);
    }
}

void clock (void *argument)
{
    for (;;) {
        uint32_t flags = osThreadFlagsWait(0x0100, osFlagsWaitAny, osWaitForever);
        TRACE(0, "%s flags:%x", __func__, flags);
        TRACE(0, "%s 1st seting", __func__);
        osThreadFlagsSet(tid_phaseA,  0x1000);            /* set signal to thread 'phaseA' */
        TRACE(0, "%s 2nd seting", __func__);
        osThreadFlagsSet(tid_phaseA,  0x0100);            /* set signal to thread 'phaseA' */
        TRACE(0, "%s 3rd seting", __func__);
        osThreadFlagsSet(tid_phaseA,  0x0003);            /* set signal to thread 'thread phaseA' */
        osDelay(500);                             /* delay 500ms*/
    }
}

int simu_os_test(void)
{
    osThreadAttr_t threadattr_phaseA;
    memset(&threadattr_phaseA, 0, sizeof(osThreadAttr_t));
    threadattr_phaseA.name = "phaseA";
    threadattr_phaseA.attr_bits = osThreadDetached;
    threadattr_phaseA.stack_size = 1024;
    /* threadattr_phaseA.priority = osPriorityRealtime; */
    threadattr_phaseA.priority = osPriorityNormal;
    tid_phaseA = osThreadNew(phaseA, NULL, &threadattr_phaseA);

    osThreadAttr_t threadattr_clock;
    memset(&threadattr_clock, 0, sizeof(osThreadAttr_t));
    threadattr_clock.name = "clock";
    threadattr_clock.attr_bits = osThreadDetached;
    threadattr_clock.stack_size = 1024;
    /* threadattr_phaseA.priority = osPriorityRealtime; */
    threadattr_phaseA.priority = osPriorityNormal;

    tid_clock  = osThreadNew(clock,  NULL, &threadattr_clock);
    for(;;);
    return 0;
}

and the log:

      614/I/NONE  /  4 | phaseA flags:1000
      614/I/NONE  /  4 | phaseA begin wait 0x3
      614/I/NONE  /  5 | clock flags:100
      614/I/NONE  /  5 | clock 1st seting
      614/I/NONE  /  5 | clock 2nd seting
      614/I/NONE  /  5 | clock 3rd seting
      615/I/NONE  /  4 | phaseA again flags:1103

Br, Yingchun

swordligit commented 6 months ago

I am lucky, https://github.com/ARM-software/CMSIS-FreeRTOS/pull/86 this commit fix my problem. Thanks a lot.

VladimirUmek commented 6 months ago

I'm glad your problem has been fixed already! Many thanks for taking the time to create the report anyway, that's the fastest way to get the issues solved!