From f8295de5836cc2ce07b704725684de786fb4ed18 Mon Sep 17 00:00:00 2001 From: rtel Date: Sat, 1 Aug 2015 07:03:32 +0000 Subject: [PATCH] Preparing for new release... Kernel changes: - Remove an assert that was preventing xQueueSendFromISR() being used to give a mutex from an ISR (mutexes cannot be given using xSemaphoreGiveFromISR()). - Introduce xTaskNotifyAndQueryFromISR() as the interrupt safe version of xTaskNotifyAndQuery(). Common demo task changes: - Update IntSemTest.c to prove the theory that it is safe to give a mutex type semaphore from an interrupt using xQueueSendFromISR() instead of xSemaphoreGiveFromISR(). - Update TaskNotify.c to test the new xTaskNotifyAndQuery() from ISR fuction. git-svn-id: https://svn.code.sf.net/p/freertos/code/trunk@2361 1d2547de-c912-0410-9cb9-b8ca96c0e9e2 --- FreeRTOS/Demo/Common/Minimal/IntSemTest.c | 56 ++++++++++++-- FreeRTOS/Demo/Common/Minimal/TaskNotify.c | 27 ++++++- FreeRTOS/Demo/WIN32-MSVC/main.c | 2 + FreeRTOS/Source/include/task.h | 92 ++++++++++++----------- FreeRTOS/Source/queue.c | 8 +- FreeRTOS/Source/tasks.c | 14 ++-- 6 files changed, 133 insertions(+), 66 deletions(-) diff --git a/FreeRTOS/Demo/Common/Minimal/IntSemTest.c b/FreeRTOS/Demo/Common/Minimal/IntSemTest.c index 69a02943c..13ba9bc14 100644 --- a/FreeRTOS/Demo/Common/Minimal/IntSemTest.c +++ b/FreeRTOS/Demo/Common/Minimal/IntSemTest.c @@ -163,7 +163,7 @@ static SemaphoreHandle_t xMasterSlaveMutex = NULL; /* Flag that allows the master task to control when the interrupt gives or does not give the mutex. There is no mutual exclusion on this variable, but this is only test code and it should be fine in the 32=bit test environment. */ -static BaseType_t xOkToGiveMutex = pdFALSE, xOkToGiveCountingSemaphore = pdFALSE; +static BaseType_t xOkToGiveMutex = pdFALSE, xOkToGiveCountingSemaphore = pdFALSE, xOkToGiveMasterSlaveMutex = pdFALSE; /* Used to coordinate timing between tasks and the interrupt. */ const TickType_t xInterruptGivePeriod = pdMS_TO_TICKS( intsemINTERRUPT_MUTEX_GIVE_PERIOD_MS ); @@ -217,6 +217,8 @@ static void vInterruptMutexMasterTask( void *pvParameters ) static void prvTakeAndGiveInTheSameOrder( void ) { +static BaseType_t xGiveFromTask = pdTRUE; + /* Ensure the slave is suspended, and that this task is running at the lower priority as expected as the start conditions. */ #if( INCLUDE_eTaskGetState == 1 ) @@ -293,10 +295,27 @@ static void prvTakeAndGiveInTheSameOrder( void ) /* Finally give back the shared mutex. This time the higher priority task should run before this task runs again - so this task should have disinherited the priority and the higher priority task should be in the - suspended state again. */ - if( xSemaphoreGive( xMasterSlaveMutex ) != pdPASS ) + suspended state again. Alternatve beetween giving the mutex from this task, + and giving it from the interrupt. */ + if( xGiveFromTask == pdTRUE ) { - xErrorDetected = pdTRUE; + if( xSemaphoreGive( xMasterSlaveMutex ) != pdPASS ) + { + xErrorDetected = pdTRUE; + } + + /* Give the mutex from the interrupt on the next go around. */ + xGiveFromTask = pdFALSE; + } + else + { + /* Wait for the mutex to be given from the interrupt. */ + xOkToGiveMasterSlaveMutex = pdTRUE; + vTaskDelay( xInterruptGivePeriod + ( xInterruptGivePeriod >> 1 ) ); + xOkToGiveMasterSlaveMutex = pdFALSE; + + /* Give the mutex from the task on the next go around. */ + xGiveFromTask = pdTRUE; } if( uxTaskPriorityGet( NULL ) != intsemMASTER_PRIORITY ) @@ -514,15 +533,36 @@ TickType_t xTimeNow; xTimeNow = xTaskGetTickCountFromISR(); if( ( ( TickType_t ) ( xTimeNow - xLastGiveTime ) ) >= pdMS_TO_TICKS( intsemINTERRUPT_MUTEX_GIVE_PERIOD_MS ) ) { - configASSERT( xISRMutex ); if( xOkToGiveMutex != pdFALSE ) { + configASSERT( xISRMutex ); + /* Null is used as the second parameter in this give, and non-NULL - in the other gives for code coverage reasons. */ - xSemaphoreGiveFromISR( xISRMutex, NULL ); + in the other gives, for code coverage reasons. NOTE: This is a + Mutex, so xQueueGiveFromISR() should be used in place of + xSemaphoreGiveFromISR() in case there is a mutex holder that has + inherited a priority (although, in the case of xISRMutex, there + isn't). The "item to queue" parameter is set to NULL as no data is + copied into a mutex.*/ + xQueueSendFromISR( ( QueueHandle_t ) xISRMutex, NULL, NULL ); + + /* Second give attempt should fail. */ + configASSERT( xQueueSendFromISR( xISRMutex, NULL, &xHigherPriorityTaskWoken ) == pdFAIL ); + } + + if( xOkToGiveMasterSlaveMutex != pdFALSE ) + { + configASSERT( xOkToGiveMasterSlaveMutex ); + + /* NOTE: This is a Mutex, so xQueueGiveFromISR() should be used in + place of xSemaphoreGiveFromISR() in case there is a mutex holder + that has inherited a priority (as indeed there is in this case). + The "item to queue" parameter is set to NULL as no data is copied + into a mutex. */ + xQueueSendFromISR( ( QueueHandle_t ) xMasterSlaveMutex, NULL, NULL ); /* Second give attempt should fail. */ - configASSERT( xSemaphoreGiveFromISR( xISRMutex, &xHigherPriorityTaskWoken ) == pdFAIL ); + configASSERT( xQueueSendFromISR( xMasterSlaveMutex, NULL, &xHigherPriorityTaskWoken ) == pdFAIL ); } if( xOkToGiveCountingSemaphore != pdFALSE ) diff --git a/FreeRTOS/Demo/Common/Minimal/TaskNotify.c b/FreeRTOS/Demo/Common/Minimal/TaskNotify.c index 7bf9c53e5..e74d0599e 100644 --- a/FreeRTOS/Demo/Common/Minimal/TaskNotify.c +++ b/FreeRTOS/Demo/Common/Minimal/TaskNotify.c @@ -472,8 +472,10 @@ TickType_t xPeriod; void xNotifyTaskFromISR( void ) { -static BaseType_t xCallCount = 0; +static BaseType_t xCallCount = 0, xAPIToUse = 0; const BaseType_t xCallInterval = pdMS_TO_TICKS( 50 ); +uint32_t ulPreviousValue; +const uint32_t ulUnexpectedValue = 0xff; /* The task performs some tests before starting the timer that gives the notification from this interrupt. If the timer has not been created yet @@ -488,7 +490,28 @@ const BaseType_t xCallInterval = pdMS_TO_TICKS( 50 ); /* It is time to 'give' the notification again. */ xCallCount = 0; - vTaskNotifyGiveFromISR( xTaskToNotify, NULL ); + /* Test using both vTaskNotifyGiveFromISR(), xTaskNotifyFromISR() + and xTaskNotifyAndQueryFromISR(). */ + switch( xAPIToUse ) + { + case 0: vTaskNotifyGiveFromISR( xTaskToNotify, NULL ); + xAPIToUse++; + break; + + case 1: xTaskNotifyFromISR( xTaskToNotify, 0, eIncrement, NULL ); + xAPIToUse++; + break; + + case 2: ulPreviousValue = ulUnexpectedValue; + xTaskNotifyAndQueryFromISR( xTaskToNotify, 0, eIncrement, &ulPreviousValue, NULL ); + configASSERT( ulPreviousValue != ulUnexpectedValue ); + xAPIToUse = 0; + break; + + default:/* Should never get here!. */ + break; + } + ulTimerNotificationsSent++; } } diff --git a/FreeRTOS/Demo/WIN32-MSVC/main.c b/FreeRTOS/Demo/WIN32-MSVC/main.c index 3cd5da9be..f6cc6df0d 100644 --- a/FreeRTOS/Demo/WIN32-MSVC/main.c +++ b/FreeRTOS/Demo/WIN32-MSVC/main.c @@ -302,6 +302,8 @@ volatile uint32_t ulSetToNonZeroInDebuggerToContinue = 0; ( void ) ulLine; ( void ) pcFileName; + printf( "ASSERT! Line %d, file %s\r\n", ulLine, pcFileName ); + taskENTER_CRITICAL(); { /* Stop the trace recording. */ diff --git a/FreeRTOS/Source/include/task.h b/FreeRTOS/Source/include/task.h index 1bae57256..7dd9f87ac 100644 --- a/FreeRTOS/Source/include/task.h +++ b/FreeRTOS/Source/include/task.h @@ -1437,30 +1437,30 @@ void vTaskGetRunTimeStats( char *pcWriteBuffer ) PRIVILEGED_FUNCTION; /*lint !e9 * @param eAction Specifies how the notification updates the task's notification * value, if at all. Valid values for eAction are as follows: * - * eSetBits - - * The task's notification value is bitwise ORed with ulValue. xTaskNofify() - * always returns pdPASS in this case. - * - * eIncrement - - * The task's notification value is incremented. ulValue is not used and - * xTaskNotify() always returns pdPASS in this case. - * - * eSetValueWithOverwrite - - * The task's notification value is set to the value of ulValue, even if the - * task being notified had not yet processed the previous notification (the - * task already had a notification pending). xTaskNotify() always returns - * pdPASS in this case. - * - * eSetValueWithoutOverwrite - - * If the task being notified did not already have a notification pending then - * the task's notification value is set to ulValue and xTaskNotify() will - * return pdPASS. If the task being notified already had a notification - * pending then no action is performed and pdFAIL is returned. - * - * eNoAction - - * The task receives a notification without its notification value being - * updated. ulValue is not used and xTaskNotify() always returns pdPASS in - * this case. + * eSetBits - + * The task's notification value is bitwise ORed with ulValue. xTaskNofify() + * always returns pdPASS in this case. + * + * eIncrement - + * The task's notification value is incremented. ulValue is not used and + * xTaskNotify() always returns pdPASS in this case. + * + * eSetValueWithOverwrite - + * The task's notification value is set to the value of ulValue, even if the + * task being notified had not yet processed the previous notification (the + * task already had a notification pending). xTaskNotify() always returns + * pdPASS in this case. + * + * eSetValueWithoutOverwrite - + * If the task being notified did not already have a notification pending then + * the task's notification value is set to ulValue and xTaskNotify() will + * return pdPASS. If the task being notified already had a notification + * pending then no action is performed and pdFAIL is returned. + * + * eNoAction - + * The task receives a notification without its notification value being + * updated. ulValue is not used and xTaskNotify() always returns pdPASS in + * this case. * * pulPreviousNotificationValue - * Can be used to pass out the subject task's notification value before any @@ -1523,30 +1523,30 @@ BaseType_t xTaskGenericNotify( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNo * @param eAction Specifies how the notification updates the task's notification * value, if at all. Valid values for eAction are as follows: * - * eSetBits - - * The task's notification value is bitwise ORed with ulValue. xTaskNofify() - * always returns pdPASS in this case. + * eSetBits - + * The task's notification value is bitwise ORed with ulValue. xTaskNofify() + * always returns pdPASS in this case. * - * eIncrement - - * The task's notification value is incremented. ulValue is not used and - * xTaskNotify() always returns pdPASS in this case. + * eIncrement - + * The task's notification value is incremented. ulValue is not used and + * xTaskNotify() always returns pdPASS in this case. * - * eSetValueWithOverwrite - - * The task's notification value is set to the value of ulValue, even if the - * task being notified had not yet processed the previous notification (the - * task already had a notification pending). xTaskNotify() always returns - * pdPASS in this case. + * eSetValueWithOverwrite - + * The task's notification value is set to the value of ulValue, even if the + * task being notified had not yet processed the previous notification (the + * task already had a notification pending). xTaskNotify() always returns + * pdPASS in this case. * - * eSetValueWithoutOverwrite - - * If the task being notified did not already have a notification pending then - * the task's notification value is set to ulValue and xTaskNotify() will - * return pdPASS. If the task being notified already had a notification - * pending then no action is performed and pdFAIL is returned. + * eSetValueWithoutOverwrite - + * If the task being notified did not already have a notification pending then + * the task's notification value is set to ulValue and xTaskNotify() will + * return pdPASS. If the task being notified already had a notification + * pending then no action is performed and pdFAIL is returned. * - * eNoAction - - * The task receives a notification without its notification value being - * updated. ulValue is not used and xTaskNotify() always returns pdPASS in - * this case. + * eNoAction - + * The task receives a notification without its notification value being + * updated. ulValue is not used and xTaskNotify() always returns pdPASS in + * this case. * * @param pxHigherPriorityTaskWoken xTaskNotifyFromISR() will set * *pxHigherPriorityTaskWoken to pdTRUE if sending the notification caused the @@ -1563,7 +1563,9 @@ BaseType_t xTaskGenericNotify( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNo * \defgroup xTaskNotify xTaskNotify * \ingroup TaskNotifications */ -BaseType_t xTaskNotifyFromISR( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNotifyAction eAction, BaseType_t *pxHigherPriorityTaskWoken ) PRIVILEGED_FUNCTION; +BaseType_t xTaskGenericNotifyFromISR( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNotifyAction eAction, uint32_t *pulPreviousNotificationValue, BaseType_t *pxHigherPriorityTaskWoken ) PRIVILEGED_FUNCTION; +#define xTaskNotifyFromISR( xTaskToNotify, ulValue, eAction, pxHigherPriorityTaskWoken ) xTaskGenericNotifyFromISR( ( xTaskToNotify ), ( ulValue ), ( eAction ), NULL, ( pxHigherPriorityTaskWoken ) ) +#define xTaskNotifyAndQueryFromISR( xTaskToNotify, ulValue, eAction, pulPreviousNotificationValue, pxHigherPriorityTaskWoken ) xTaskGenericNotifyFromISR( ( xTaskToNotify ), ( ulValue ), ( eAction ), ( pulPreviousNotificationValue ), ( pxHigherPriorityTaskWoken ) ) /** * task. h diff --git a/FreeRTOS/Source/queue.c b/FreeRTOS/Source/queue.c index c35e00626..0a196a1ef 100644 --- a/FreeRTOS/Source/queue.c +++ b/FreeRTOS/Source/queue.c @@ -1219,9 +1219,11 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; if the item size is not 0. */ configASSERT( pxQueue->uxItemSize == 0 ); - /* Normally a mutex would not be given from an interrupt, and doing so is - definitely wrong if there is a mutex holder as priority inheritance makes no - sense for an interrupts, only tasks. */ + /* Normally a mutex would not be given from an interrupt, especially if + there is a mutex holder, as priority inheritance makes no sense for an + interrupts, only tasks. However, on occasions when it is wanted to give + a mutex from an interrupt, use xQueueSendFromISR() in place of + xSemaphoreGiveFromISR(). */ configASSERT( !( ( pxQueue->uxQueueType == queueQUEUE_IS_MUTEX ) && ( pxQueue->pxMutexHolder != NULL ) ) ); /* RTOS ports that support interrupt nesting have the concept of a maximum diff --git a/FreeRTOS/Source/tasks.c b/FreeRTOS/Source/tasks.c index d3ec721c8..49c8a4aaf 100644 --- a/FreeRTOS/Source/tasks.c +++ b/FreeRTOS/Source/tasks.c @@ -3489,12 +3489,6 @@ TCB_t *pxTCB; if( pxMutexHolder != NULL ) { - /* A task can only have an inherited priority if it holds the mutex. - If the mutex is held by a task then it cannot be given from an - interrupt, and if a mutex is given by the holding task then it must - be the running state task. */ - configASSERT( pxTCB == pxCurrentTCB ); - configASSERT( pxTCB->uxMutexesHeld ); ( pxTCB->uxMutexesHeld )--; @@ -4236,7 +4230,7 @@ TickType_t uxReturn; #if( configUSE_TASK_NOTIFICATIONS == 1 ) - BaseType_t xTaskNotifyFromISR( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNotifyAction eAction, BaseType_t *pxHigherPriorityTaskWoken ) + BaseType_t xTaskGenericNotifyFromISR( TaskHandle_t xTaskToNotify, uint32_t ulValue, eNotifyAction eAction, uint32_t *pulPreviousNotificationValue, BaseType_t *pxHigherPriorityTaskWoken ) { TCB_t * pxTCB; eNotifyValue eOriginalNotifyState; @@ -4267,8 +4261,12 @@ TickType_t uxReturn; uxSavedInterruptStatus = portSET_INTERRUPT_MASK_FROM_ISR(); { - eOriginalNotifyState = pxTCB->eNotifyState; + if( pulPreviousNotificationValue != NULL ) + { + *pulPreviousNotificationValue = pxTCB->ulNotifiedValue; + } + eOriginalNotifyState = pxTCB->eNotifyState; pxTCB->eNotifyState = eNotified; switch( eAction ) -- 2.39.5