Add critical sections while accessing LW mutex variables

kar-rahul-aws · Jan 14, 2025 · f6d66ff · f6d66ff
1 parent 1ca8ee8
commit f6d66ff
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 50 deletions.
diff --git a/light-weight-mutex.c b/light-weight-mutex.c
@@ -27,7 +27,7 @@
         TickType_t startTime = xTaskGetTickCount();
 
         /* Check the pxMutex pointer is not NULL. */
-        if( pxMutex == NULL )
+        if( ( pxMutex == NULL ) || ( pxMutex->owner != NULL ) )
         {
             xReturn = pdFALSE;
             goto exit;
@@ -44,30 +44,34 @@
         }
         #endif
 
-        while( pdTRUE )
+        for( ; ; )
         {
-            if( Atomic_CompareAndSwap_u32( &pxMutex->owner, ( uintptr_t ) currentTask, expectedOwner ) )
+            taskENTER_CRITICAL();
             {
-                pxMutex->lock_count = 1;
-                xReturn = pdTRUE;
-                goto exit;
-            }
-
-            if( expectedOwner == ( uintptr_t ) currentTask )
-            {
-                pxMutex->lock_count++;
-                xReturn = pdTRUE;
-                goto exit;
-            }
+                if( Atomic_CompareAndSwap_u32( &pxMutex->owner, ( uintptr_t ) currentTask, expectedOwner ) )
+                {
+                    pxMutex->lock_count = 1;
+                    xReturn = pdTRUE;
+                    goto exit;
+                }
 
-            if( xTicksToWait != portMAX_DELAY )
-            {
-                if( ( xTaskGetTickCount() - startTime ) >= xTicksToWait )
+                if( expectedOwner == ( uintptr_t ) currentTask )
                 {
+                    pxMutex->lock_count++;
                     xReturn = pdTRUE;
                     goto exit;
                 }
+
+                if( xTicksToWait != portMAX_DELAY )
+                {
+                    if( ( xTaskGetTickCount() - startTime ) >= xTicksToWait )
+                    {
+                        xReturn = pdTRUE;
+                        goto exit;
+                    }
+                }
             }
+            taskEXIT_CRITICAL();
 
             vTaskPlaceOnEventList( &( pxMutex->xTasksWaitingForMutex ), xTicksToWait );
             taskYIELD();
@@ -100,27 +104,31 @@
         }
         #endif
 
-        if( Atomic_Load_u32( &pxMutex->owner ) != ( uintptr_t ) currentTask )
+        taskENTER_CRITICAL();
         {
-            return pdFALSE;
-        }
-
-        pxMutex->lock_count--;
+            if( Atomic_Load_u32( &pxMutex->owner ) != ( uintptr_t ) currentTask )
+            {
+                return pdFALSE;
+            }
 
-        if( pxMutex->lock_count == 0U )
-        {
-            uintptr_t expectedOwner = ( uintptr_t ) currentTask;
+            pxMutex->lock_count--;
 
-            if( !Atomic_CompareAndSwap_u32( &pxMutex->owner, 0, expectedOwner ) )
+            if( pxMutex->lock_count == 0U )
             {
-                /* This should never happen if used correctly */
-                configASSERT( pdFALSE );
-                return pdFALSE;
+                uintptr_t expectedOwner = ( uintptr_t ) currentTask;
+
+                if( !Atomic_CompareAndSwap_u32( &pxMutex->owner, 0, expectedOwner ) )
+                {
+                    /* This should never happen if used correctly */
+                    configASSERT( pdFALSE );
+                    return pdFALSE;
+                }
             }
-        }
 
-        /* Get the new owner, if any. */
-        prvAssignLWMutexOwner( pxMutex );
+            /* Get the new owner, if any. */
+            prvAssignLWMutexOwner( pxMutex );
+        }
+        taskEXIT_CRITICAL();
 
         return pdTRUE;
     }

diff --git a/tasks.c b/tasks.c
@@ -38,7 +38,7 @@
 /* FreeRTOS includes. */
 #include "FreeRTOS.h"
 #if ( configUSE_LW_MUTEXES == 1 )
-	#include "atomic.h"
+    #include "atomic.h"
 #endif /* configUSE_LW_MUTEXES == 1 */
 #include "task.h"
 #include "timers.h"
@@ -5411,31 +5411,36 @@ static void prvAddCurrentTaskToDelayedList( TickType_t xTicksToWait,
 }
 
 #if ( configUSE_LW_MUTEXES == 1 )
-	void prvAssignLWMutexOwner( LightWeightMutex_t * const pxMutex )
-	{
-		TCB_t * pxUnblockedTCB = NULL;
-		TaskHandle_t newOwner = NULL;
-		if( listLIST_IS_EMPTY( &( pxMutex->xTasksWaitingForMutex ) ) == pdFALSE )
-        {
-			pxUnblockedTCB = listGET_OWNER_OF_HEAD_ENTRY( &( pxMutex->xTasksWaitingForMutex ) );
-			newOwner = ( TaskHandle_t )pxUnblockedTCB;
-            Atomic_Store_u32( &pxMutex->owner, ( uintptr_t ) newOwner );
-            pxMutex->lock_count = 1;
+    void prvAssignLWMutexOwner( LightWeightMutex_t * const pxMutex )
+    {
+        TCB_t * pxUnblockedTCB = NULL;
+        TaskHandle_t newOwner = NULL;
 
-			if( xTaskRemoveFromEventList( &( pxMutex->xTasksWaitingForMutex ) ) != pdFALSE )
+        vTaskSuspendAll();
+        {
+            if( listLIST_IS_EMPTY( &( pxMutex->xTasksWaitingForMutex ) ) == pdFALSE )
             {
-				taskYIELD_IF_USING_PREEMPTION();
+                pxUnblockedTCB = listGET_OWNER_OF_HEAD_ENTRY( &( pxMutex->xTasksWaitingForMutex ) );
+                newOwner = ( TaskHandle_t ) pxUnblockedTCB;
+                Atomic_Store_u32( &pxMutex->owner, ( uintptr_t ) newOwner );
+                pxMutex->lock_count = 1;
+
+                if( xTaskRemoveFromEventList( &( pxMutex->xTasksWaitingForMutex ) ) != pdFALSE )
+                {
+                    taskYIELD_IF_USING_PREEMPTION();
+                }
+                else
+                {
+                    mtCOVERAGE_TEST_MARKER();
+                }
             }
             else
             {
                 mtCOVERAGE_TEST_MARKER();
             }
         }
-        else
-        {
-            mtCOVERAGE_TEST_MARKER();
-        }
-	}
+        ( void ) xTaskResumeAll();
+    }
 #endif /* configUSE_LW_MUTEXES == 1 */
 
 /* Code below here allows additional code to be inserted into this source file,