Extension of osKernelGetTickCount() to provide monotonic tick counter is in error.

[tobermory]

The explanation of the RTOS2 api call osKernelGetTickCount(), at

https://www.keil.com/pack/doc/CMSIS/RTOS2/html/group__CMSIS__RTOS__KernelCtrl.html#ga84bcdbf2fb76b10c8df4e439f0c7e11b

includes a discussion on how to adapt the 32-bit counter, which may rollover, into a 64-bit one which likely will not (or will take much longer to do so). The code presented is

uint64_t GetTick(void) {
  static uint32_t tick_h = 0U;
  static uint32_t tick_l = 0U;
  uint32_t tick;
  tick = osKernelGetTickCount();
  if (tick < tick_l) {
    tick_h++;
  }
  tick_l = tick;
  return (((uint64_t)tick_h << 32) | tick_l);
}

This is not thread-safe. tick_h could be incremented twice, by two threads, for only one physical wrap of the underlying tick counter. The consequence is that, to the calling program, time has jumped forward by 48 days (@ 1ms).

[tobermory]

To be more specific, if this code is interrupted at the statement "tick_h++;" and another thread calls this function, tick_h could be incremented twice, leading to an apparent jump forward in time by 2^32 ticks.

[tobermory]

The same code is presented in the CMSIS 6 documentation:

https://arm-software.github.io/CMSIS_6/v6.0.0/RTOS2/group__CMSIS__RTOS__KernelCtrl.html#ga84bcdbf2fb76b10c8df4e439f0c7e11b

[JonatanAntoni]

Hi @tobermory,

Thanks for pointing this out.

Yes, you are right, the given code example is not thread safe. Another limitation is that it relies some code calling GetTick() at least once per rollover cycle.

You'd need to introduce a mutex into the function to make it actually thread safe. The drawback is that this adds additional costs to GetTick() which might not be desired. Without details about the application its hard to find the best implementation.

Depending on the instruction set of your device, one could use atomic sequences to achieve thread safeness without adding the overhead of a RTOS mutex. Alternatively, one could split the getter and setter functions for the 64-bit counter and have the setter only be used from a single thread. Plenty of options but no one-size that'd fit all, I think.