Problem with taskYIELD() when used in ISR

I am using the PIC port with version V2.6.0 of  FreeRTOS. I have an ISR handling all  I2C activity.  The application task places data in a queue then blocks on a semaphore waiting for the ISR to ‘give’ it.  The code at the end of the ISR is as follows: if( cSemaphoreGiveFromISR( SSPdata.SSPSemaphore,(  portCHAR ) pdFALSE ) )     taskYIELD();        I’ve discovered that the call to taskYIELD sometimes does not unblock the waiting task immediately,  in this case the unblock occurs at the next Tick.  With a slow tick rate this delay can, of-course, be significant. This seems to be due to the following: If  the scheduler is suspended when the ISR is invoked: 1.  The call to taskYIELD() at the end of the ISR saves the context and calls vTaskSwitchContext(). This sees that the scheduler is suspended so returns to taskYIELD() without a task switch. 2. When the ISR returns, and cTaskResumeAll() executes, the task placed in the xPendingReadyList  by the call to cSemaphoreGiveFromISR  gets moved to the ready list. However, there is not a call to taskYIELD during cTaskResumeAll(),  unless one or more ticks were lost during the schedulers suspension. The result is that the context switch has to wait for the next tick. If when cSemaphoreGiveFromISR( ) is called, the scheduler is not suspended, then the task switch occurs immediately, as I would have expected. By setting a port bit just before the call to cSemaphoreGiveFromISR( ) , then having the  blocked task reset it when it unblocks, I can see that the time spent with the port bit high varies by up to one complete tick. I have a feeling that I’ve missed something important here, but despite my best efforts, can’t see what. FreeRTOS is a very impressive piece of work, and must have required Herculean effort to get it to this stage. Regards, John Franklin.

Problem with taskYIELD() when used in ISR

> If  the scheduler is suspended when the ISR is invoked: > 1.  The call to taskYIELD() at the end of the ISR saves the context and calls > vTaskSwitchContext(). This sees that the scheduler is suspended so returns to > taskYIELD() without a task switch. This is correct.  A good optimisation might be for cSemaphoreGiveFromISR() not to return true if the scheduler is suspended.  This would prevent the save/restore of the context and could be achieved with the following modification: Look for the function cQueueSendFromISR() in queue.c.  In this function there is one place where pdTRUE can be returned.  Change this to: if(  ucSchedulerSuspended == pdFALSE )     return pdTRUE; else     return pdFALSE; Two points to note: 1) You would have to make ucSchedulerSuspended accessible.  Currently it is declared static in tasks.c. 2) I have not tested this! I will have a look at this possible optimisation, and if satisfactory include it in the V3.0.0 release. A similar change could be made for the ISR queue receive function. > 2. When the ISR returns, and cTaskResumeAll() executes, the task placed in the > xPendingReadyList  by the call to cSemaphoreGiveFromISR  gets moved to the ready > list. However, there is not a call to taskYIELD during cTaskResumeAll(),  unless > one or more ticks were lost during the schedulers suspension. The result is > that the context switch has to wait for the next tick. If a task is unblocked while the scheduler is locked it is placed in the pending read list (as you say) and the context switch does not occur.  A context switch cannot occur when the scheduler is suspended. Here is another optimisation that will make the kernel behave how you want: Go to the function cTaskResumeAll() in tasks.c.  Find the line: if( usCurrentNumberOfTasks > ( unsigned portSHORT ) 0 ) Within this if statement there is a while loop that takes tasks from the pending list and places them in the ready list.  You will then see an if() statement that catches up with any ticks that occurred while the scheduler was suspended.   If any ticks were missed then a YIELD() occurs – but what you want if for a YIELD() to occur if either a tick was missed OR a task was removed from the pending list.  This could be done thus:    if( usCurrentNumberOfTasks > ( unsigned portSHORT ) 0 )    {     ____portCHAR cYieldRequired = pdFALSE;  // <<<<< new variable.     ____/* Move any readied tasks from the pending list into the     ____appropriate ready list. */     ____while( ( pxTCB = ( tskTCB * ) listGET_OWNER_OF_HEAD_ENTRY(  ( ( xList * ) &xPendingReadyList ) ) ) != NULL )     ____{     ________vListRemove( &( pxTCB->xEventListItem ) );     ________vListRemove( &( pxTCB->xGenericListItem ) );     ________prvAddTaskToReadyQueue( pxTCB );        ________// <<<< Added if statement.     ________if( pxTCB->ucPriority > pxCurrentTCB->ucPriority )     ________{     ____________cYieldRequired = pdTRUE     ________}     ____}     ____/* If any ticks occurred while the scheduler was suspended then     ____they should be processed now.  This ensures the tick count does not     ____slip, and that any delayed tasks are resumed at the correct time. */     ____if( ucMissedTicks > 0 )     ____{     ________cYieldRequired = pdTRUE;  // <<<<< New line     ________while( ucMissedTicks > 0 )     ________{     ____________vTaskIncrementTick();     ____________–ucMissedTicks;     ________}     ____}     ____if( cYieldRequired )     ____{     ________/* As we have processed some ticks it is appropriate to yield     ________to ensure the highest priority task that is ready to run is     ________the task actually running. */     ________cAlreadyYielded = ( signed portCHAR ) pdTRUE;     ________taskYIELD();     ____}    } This way the cYieldRequired flag gets set to true if EITHER a tick was missed OR a task with higher priority than the current task was removed from the pending list. Hope this makes sence – I have put in all the _____ to try and retain the indentation. Again I have not tried this but it seems to be another good candidate for the V3.0.0 release. Let me know what you think or if you have any problems.

Problem with taskYIELD() when used in ISR

Many thanks for such a pompt and comprehensive reply. Actaully, I did modify cTaskResumeAll() along the lines you suggest. However, without most of your subtleties, I was probably invoking taskYIELD() more often than necessary. Although this seemed to work, i.e. the blocked task always wakes as soon as the ISR ‘gives’ the semaphore, it produced erratic behaviour with occasional crashes. It could, of-course, be a bug in my application, or a stack overflow etc. I checked the stacks and free ram space etc, but failed to find any cause for concern. I concluded that I had misunderstood something and had somehow corrupted the RTOS kernal execution. I will try your suggestions and get back to you. If necessary I will create a simple app just to test this new functionality. I will also include my original modifications. Thanks again for the help. Regards, John Franklin

Problem with taskYIELD() when used in ISR

Hello Richard. Apologies for the delay. As you will see I have been busy. I implemented the changes to   cTaskResumeAll(), as per your suggestions. This did indeed produce the required behaviour, i.e. the waiting task is always woken immediately on return from the ISR rather than having to wait for the next Tick. However, as with my own modification to cTaskResumeAll(), I started getting erratic behaviour in my application, and occasional crashes. The symptoms were seemingly almost random. Minor unrelated changes to my application code would apparently cure the problem. I have finally tracked down the cause which, surprisingly, appears to be a bug in the C18 v2.20 compiler and nothing to do with the freeRTOS changes. The compiler produced code that precedes the invocation of a function was, sometimes, incorrectly pushing the function parameters onto the stack. The code did not take care of  the situation in which the stack pointer crosses from one bank to the next, i.e. only the lsb of the stack pointer was being incremented. This is in spite of the memory model being selected as multi-bank..  I tried a simple demo app on another PC just to check that the MPLAB settings hadnt been screwed up in some way, but with exactly the same result. Moving to the latter C18 version  v2.40 cured the problem. Any code modifications that slightly altered the placing of the stack in ram could push the stack pointer away from a stack boundary so masking the problem and explaining the apparent random behaviour. So in conclusion, the modifications to cTaskResumeAll()  seem to work fine. Thanks again for the help. Regards, John Franklin.

Problem with taskYIELD() when used in ISR

Thanks for sharing this valuable information.