RTX5: fix Round-Robin (#228, #1000)

RobertRostohar · RobertRostohar · commit ada4cde81766 · 2021-05-17T11:51:42.000+02:00
Round-Robin timeout value is not reset any more when switching to higher priority threads.
diff --git a/CMSIS/DoxyGen/RTOS2/src/cmsis_os2.txt b/CMSIS/DoxyGen/RTOS2/src/cmsis_os2.txt
@@ -290,9 +290,6 @@ a blocking RTOS-call execution is switched back to thread 3 immediately during t
 At time index 5 thread 3 uses a blocking RTOS-call as well. Thus the scheduler switches back to thread 2 for time index 6.
 At time index 7 the scheduler uses the round-robin mechanism to switch to thread 1 and so on.
 
-\note Refer to limitations this type of scheduling in the \ref systemConfig_rr configuration section.
-
-
 \section MemoryAllocation Memory Allocation 
 
 RTX5 objects (thread, mutex, semaphore, timer, message queue, thread and event flags, as well as memory pool) require
@@ -807,11 +804,6 @@ In other words, threads execute for the duration of their time slice (unless a t
 switches to the next thread that is in \b READY state and has the same priority. If no other thread with the same priority is
 ready to run, the current running thread resumes it execution.
 
-\note
-When switching to higher priority threads, the round-robin timeout value is reset. This might lead to conditions where
-threads with the same priority as the interrupted thread are blocked for a longer period of time or even never called
-again. You can avoid this by using \ref osThreadYield to pass control to the next thread in the \b READY state. 
-
 Round-Robin multitasking is controlled with the <b>\#define OS_ROBIN_ENABLE</b>. The time slice period is configured (in RTX
 timer ticks) with the <b>\#define OS_ROBIN_TIMEOUT</b>.
 
diff --git a/CMSIS/DoxyGen/RTOS2/src/history.txt b/CMSIS/DoxyGen/RTOS2/src/history.txt
@@ -103,6 +103,7 @@
       <td>
        - CVE-2021-27431 vulnerability mitigation.
        - Added OS Initialization for IAR.
+       - Fixed Round-Robin (timeout value is not reset when switching to higher priority threads).
        - Enhanced stack overrun checking.
        - Updated configuration (Event Recorder).
        - Reorganized and optimized IRQ modules.
diff --git a/CMSIS/RTOS2/RTX/Include/rtx_os.h b/CMSIS/RTOS2/RTX/Include/rtx_os.h
@@ -111,7 +111,7 @@ typedef struct osRtxThread_s {
   struct osRtxThread_s    *delay_next;  ///< Link pointer to next Thread in Delay list
   struct osRtxThread_s    *delay_prev;  ///< Link pointer to previous Thread in Delay list
   struct osRtxThread_s   *thread_join;  ///< Thread waiting to Join
-  uint32_t                      delay;  ///< Delay Time
+  uint32_t                      delay;  ///< Delay Time/Round Robin Time Tick
   int8_t                     priority;  ///< Thread Priority
   int8_t                priority_base;  ///< Base Priority
   uint8_t                 stack_frame;  ///< Stack Frame (EXC_RETURN[7..0])
@@ -297,9 +297,9 @@ typedef struct {
     osRtxThread_t         *delay_list;  ///< Delay List
     osRtxThread_t          *wait_list;  ///< Wait List (no Timeout)
     osRtxThread_t     *terminate_list;  ///< Terminate Thread List
+    uint32_t                 reserved;
     struct {                            ///< Thread Round Robin Info
       osRtxThread_t           *thread;  ///< Round Robin Thread
-      uint32_t                   tick;  ///< Round Robin Time Tick
       uint32_t                timeout;  ///< Round Robin Timeout
     } robin;
   } thread;
diff --git a/CMSIS/RTOS2/RTX/RTX5.scvd b/CMSIS/RTOS2/RTX/RTX5.scvd
@@ -370,8 +370,7 @@
       <member name="thread_wait_list"           type="*osRtxThread_t"       offset="48"  info="Wait list (no timeout)"/>
       <member name="thread_terminate_list"      type="*osRtxThread_t"       offset="52"  info="Terminate list"/>
 
-      <member name="thread_robin_thread"        type="*osRtxThread_t"       offset="56"  info="Round Robin thread"/>
-      <member name="thread_robin_tick"          type="uint32_t"             offset="60"  info="Round Robin time tick"/>
+      <member name="thread_robin_thread"        type="*osRtxThread_t"       offset="60"  info="Round Robin thread"/>
       <member name="thread_timeout"             type="uint32_t"             offset="64"  info="Round Robin timeout"/>
 
       <member name="timer_list"                 type="*osRtxTimer_t"        offset="68"  info="Active timer list"/>
@@ -404,6 +403,8 @@
       <member name="mpi_semaphore"              type="*osRtxMpInfo_t"       offset="152" info="Semaphore control blocks"/>
       <member name="mpi_memory_pool"            type="*osRtxMpInfo_t"       offset="156" info="Memory pool control blocks"/>
       <member name="mpi_message_queue"          type="*osRtxMpInfo_t"       offset="160" info="Message queue control blocks"/>
+
+      <var name="robin_tick" type="uint32_t" info="Round Robin time tick (thread_robin_thread.delay)"/>
     </typedef>
 
     <!-- OS Runtime Object Memory Usage structure -->
@@ -822,16 +823,18 @@
         <readlist cond="(mem_list_com[i].len &amp; 1) &amp;&amp; (mem_list_com[i].id == 0xFA)" name="QCB" type="osRtxMessageQueue_t" offset="addr" count="1" />
       </list>
 
-      <!-- Read thread wait list -->
-      <readlist name="TWL" cond="RTX_En &amp;&amp; os_Info.thread_wait_list" type="osRtxThread_t" offset="os_Info.thread_wait_list" next="delay_next"/>
-
       <!-- Validate and process Thread control blocks -->
       <list name="i" start="0" limit="TCB._count">
         <calc>
           TCB[i].cb_valid = (TCB[i].id == 0xF1) &amp;&amp; (TCB[i].state != 0) &amp;&amp; (TCB[i].sp != 0);
           TCB[i].sp_valid = 1;
         </calc>
 
+        <!-- Set Round Robin time tick from the running thread tick value -->
+        <calc cond="(TCB[i].state == 2) &amp;&amp; os_Config.robin_timeout">
+          os_Info.robin_tick = TCB[i].delay;
+        </calc>
+
         <!-- Stack pointer for running thread -->
         <calc cond="(TCB[i].state == 2) &amp;&amp; (__Running == 0)">
           ipsr = __GetRegVal("XPSR") &amp; 0x01FF;
@@ -1099,10 +1102,10 @@
           <item property="Kernel State" value="%E[os_Info.kernel_state]" cond="RTX_En != 0"/>
           <item property="Kernel Tick Count"      value="%d[os_Info.kernel_tick]"     cond="RTX_En != 0"/>
           <item property="Kernel Tick Frequency"  value="%d[os_Config.tick_freq]"     cond="RTX_En != 0" />
-          <item property="Round Robin"            value="Disabled"                      cond="(os_Config.robin_timeout == 0) &amp;&amp; (RTX_En != 0)" />
-          <item property="Round Robin Tick Count" value="%d[os_Info.thread_robin_tick]" cond="(os_Config.robin_timeout > 0)  &amp;&amp; (RTX_En != 0)" />
-          <item property="Round Robin Timeout"    value="%d[os_Config.robin_timeout]"   cond="(os_Config.robin_timeout > 0)  &amp;&amp; (RTX_En != 0)" />
-          <item property="Global Dynamic Memory" value="Not used"                                                                                                               cond="(os_Config.mem_common_size == 0) &amp;&amp; (RTX_En != 0)"/>
+          <item property="Round Robin"            value="Disabled"                    cond="(os_Config.robin_timeout == 0) &amp;&amp; (RTX_En != 0)" />
+          <item property="Round Robin Tick Count" value="%d[os_Info.robin_tick]"      cond="(os_Config.robin_timeout > 0)  &amp;&amp; (RTX_En != 0)" />
+          <item property="Round Robin Timeout"    value="%d[os_Config.robin_timeout]" cond="(os_Config.robin_timeout > 0)  &amp;&amp; (RTX_En != 0)" />
+          <item property="Global Dynamic Memory" value="Not used"                                                                                                                    cond="(os_Config.mem_common_size == 0) &amp;&amp; (RTX_En != 0)"/>
           <item property="Global Dynamic Memory" value="Base: %x[mem_head_com._addr], Size: %d[mem_head_com.size], Used: %d[mem_head_com.used], Max used: %d[mem_head_com.max_used]" cond="(os_Config.mem_common_size != 0) &amp;&amp; (RTX_En != 0)"/>
           <item property="Stack Overrun Check"   value="%t[os_Config.stack_check ? &quot;Enabled&quot; : &quot;Disabled&quot;]" cond="RTX_En != 0"/>
           <item property="Stack Usage Watermark" value="%t[os_Config.stack_wmark ? &quot;Enabled&quot; : &quot;Disabled&quot;]" cond="RTX_En != 0"/>
diff --git a/CMSIS/RTOS2/RTX/Source/rtx_system.c b/CMSIS/RTOS2/RTX/Source/rtx_system.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013-2019 Arm Limited. All rights reserved.
+ * Copyright (c) 2013-2021 Arm Limited. All rights reserved.
  *
  * SPDX-License-Identifier: Apache-2.0
  *
@@ -122,38 +122,40 @@ void osRtxTick_Handler (void) {
   OS_Tick_AcknowledgeIRQ();
   osRtxInfo.kernel.tick++;
 
-  // Process Timers
-  if (osRtxInfo.timer.tick != NULL) {
-    osRtxInfo.timer.tick();
-  }
-
   // Process Thread Delays
   osRtxThreadDelayTick();
 
   osRtxThreadDispatch(NULL);
 
+  // Process Timers
+  if (osRtxInfo.timer.tick != NULL) {
+    osRtxInfo.timer.tick();
+  }
+
   // Check Round Robin timeout
   if (osRtxInfo.thread.robin.timeout != 0U) {
-    if (osRtxInfo.thread.robin.thread != osRtxInfo.thread.run.next) {
-      // Reset Round Robin
-      osRtxInfo.thread.robin.thread = osRtxInfo.thread.run.next;
-      osRtxInfo.thread.robin.tick   = osRtxInfo.thread.robin.timeout;
-    } else {
-      if (osRtxInfo.thread.robin.tick != 0U) {
-        osRtxInfo.thread.robin.tick--;
+    thread = osRtxInfo.thread.run.next;
+    if (thread != osRtxInfo.thread.robin.thread) {
+      osRtxInfo.thread.robin.thread = thread;
+      if (thread->delay == 0U) {
+        // Reset Round Robin
+        thread->delay = osRtxInfo.thread.robin.timeout;
       }
-      if (osRtxInfo.thread.robin.tick == 0U) {
-        // Round Robin Timeout
-        if (osRtxKernelGetState() == osRtxKernelRunning) {
-          thread = osRtxInfo.thread.ready.thread_list;
-          if ((thread != NULL) && (thread->priority == osRtxInfo.thread.robin.thread->priority)) {
-            osRtxThreadListRemove(thread);
-            osRtxThreadReadyPut(osRtxInfo.thread.robin.thread);
-            EvrRtxThreadPreempted(osRtxInfo.thread.robin.thread);
-            osRtxThreadSwitch(thread);
-            osRtxInfo.thread.robin.thread = thread;
-            osRtxInfo.thread.robin.tick   = osRtxInfo.thread.robin.timeout;
-          }
+    }
+    if (thread->delay != 0U) {
+      thread->delay--;
+    }
+    if (thread->delay == 0U) {
+      // Round Robin Timeout
+      if (osRtxKernelGetState() == osRtxKernelRunning) {
+        thread = osRtxInfo.thread.ready.thread_list;
+        if ((thread != NULL) && (thread->priority == osRtxInfo.thread.robin.thread->priority)) {
+          osRtxThreadListRemove(thread);
+          osRtxThreadReadyPut(osRtxInfo.thread.robin.thread);
+          EvrRtxThreadPreempted(osRtxInfo.thread.robin.thread);
+          osRtxThreadSwitch(thread);
+          osRtxInfo.thread.robin.thread = thread;
+          thread->delay = osRtxInfo.thread.robin.timeout;
         }
       }
     }
diff --git a/CMSIS/RTOS2/RTX/Source/rtx_thread.c b/CMSIS/RTOS2/RTX/Source/rtx_thread.c
@@ -313,6 +313,7 @@ static void osRtxThreadDelayRemove (os_thread_t *thread) {
       osRtxInfo.thread.delay_list = thread->delay_next;
     }
   }
+  thread->delay = 0U;
 }
 
 /// Process Thread Delay Tick (executed each System Tick).
@@ -1046,8 +1047,6 @@ static osStatus_t svcRtxThreadSuspend (osThreadId_t thread_id) {
 
     // Update Thread State and put it into Delay list
     thread->state = osRtxThreadBlocked;
-    thread->thread_prev = NULL;
-    thread->thread_next = NULL;
     osRtxThreadDelayInsert(thread, osWaitForever);
   }
 

Original file line number	Diff line number	Diff line change
`@@ -313,6 +313,7 @@ static void osRtxThreadDelayRemove (os_thread_t *thread) {`
`313`	`313`	`osRtxInfo.thread.delay_list = thread->delay_next;`
`314`	`314`	`}`
`315`	`315`	`}`
	`316`	`+ thread->delay = 0U;`
`316`	`317`	`}`
`317`	`318`
`318`	`319`	`/// Process Thread Delay Tick (executed each System Tick).`
`@@ -1046,8 +1047,6 @@ static osStatus_t svcRtxThreadSuspend (osThreadId_t thread_id) {`
`1046`	`1047`
`1047`	`1048`	`// Update Thread State and put it into Delay list`
`1048`	`1049`	`thread->state = osRtxThreadBlocked;`
`1049`		`- thread->thread_prev = NULL;`
`1050`		`- thread->thread_next = NULL;`
`1051`	`1050`	`osRtxThreadDelayInsert(thread, osWaitForever);`
`1052`	`1051`	`}`
`1053`	`1052`