Run Time Stats: portGET_RUN_TIME_COUNTER_VALUE Interrupt Guarding

Kernel
1

Hi

I am using an STM32H750B with two 16 bit timers chained together to form a 32 bit timer.

To get the full 32 bits for the time value I read the count registers of the two timers.

To prevent loss of sync between the timer values due to interrupts I do the following:

__inline uint32_t RunTime(void)
{
#if(USE_RUNTIME_STATS_TIMER_32_BITS == 1)
	uint32_t u32Temp;

//	taskENTER_CRITICAL();
	taskDISABLE_INTERRUPTS();
	u32Temp = (uint32_t)((uint16_t)(RUN_TIMER_HI_HANDLE.Instance->CNT) << 16) + (uint16_t)((RUN_TIMER_LO_HANDLE.Instance->CNT));
//	taskEXIT_CRITICAL();
	taskENABLE_INTERRUPTS();

	return u32Temp;
#else
	return (uint32_t)RUN_TIMER_LO_HANDLE.Instance->CNT;
#endif
}

I tried a taskENTER_CRITICAL() before the calc and a taskEXIT_CRITICAL() after the calc, but this killed the system. I then tried the taskDISABLE_INTERRUPTS() and its friend taskENABLE_INTERRUPTS() and the system behaved OK.

The RunTime function is called from the FreeRTOS function:

void vTaskSwitchContext( void )
{
	if( uxSchedulerSuspended != ( UBaseType_t ) pdFALSE )
	{
		/* The scheduler is currently suspended - do not allow a context
		switch. */
		xYieldPending = pdTRUE;
	}
	else
	{
		xYieldPending = pdFALSE;
		traceTASK_SWITCHED_OUT();

		#if ( configGENERATE_RUN_TIME_STATS == 1 )
		{
			#ifdef portALT_GET_RUN_TIME_COUNTER_VALUE
				portALT_GET_RUN_TIME_COUNTER_VALUE( ulTotalRunTime );
			#else
				ulTotalRunTime = portGET_RUN_TIME_COUNTER_VALUE();
			#endif ...

Just wondering whether my approach is OK and that it will not have nasty side effects.

Kind Regards
Rob

2

I had the same problem when reading a clock in an STM32Fx and solved it like this:

    {
        uint32_t ulCounts[2];
        uint32_t ulSlowCount;

        for( ;; )
        {
            ulCounts[ 0 ] = timer_handle.Instance->CNT;
            ulSlowCount = ulInterruptCount;
            ulCounts[ 1 ] = timer_handle.Instance->CNT;
            if( ulCounts[ 1 ] >= ulCounts[ 0 ] )
            {
                /* TIMER_IRQ_HANDLER() has not occurred in between. */
                break;
            }
        }
        ullReturn = ( uint64_t )ulSlowCount * ulReloadCount + ulCounts[ 1 ];
    }

In you case, I would think of something like this:

    uint32_t ulHi;
    uint32_t ulLow[ 2 ];
    uint32_t u32Temp;
    for( ;; )
    {
        ulLow[ 0 ] = ( uint32_t ) RUN_TIMER_LO_HANDLE.Instance->CNT;
        ulHi       = ( uint32_t ) RUN_TIMER_HI_HANDLE.Instance->CNT;
        ulLow[ 1 ] = ( uint32_t ) RUN_TIMER_LO_HANDLE.Instance->CNT;
        if( ulLow[ 1 ] >= ulLow[ 0 ] )
        {
            break;
        }
        /* else, a timer overflow interrupt has occurred. */
    }
    u32Temp = ( ulHi << 16 ) | ulLow[ 1 ];
    return u32Temp;
3

One thing to note is that the concept of trying to avoid the counting of time from interrupts in the routine to read the counter is a concept that just introduces a race condition into its definition. That interrupt could have occurred just before the call to disable the interrupts, and thus still counted, so is attempting to determine something very dependent on the exact timing of events.

The other thing to note is that vTaskSwitchContext is generally called inside an interrupt routine, as most ports use a software-generated interrupt to trigger this. If you had looked at WHAT was killing your system, you would have found an assert on not being in an ISR. If you wanted to use a critical section there, it would have needed to have been the FromISR versions.

Also, as others have noted, there is a danger of the lower counter rolling over between the reading of the lower counter and the upper counter. One way to solve this is to use one of the 32-bit counters (if that part has them), or you need to read and check read one of the counters. @htibosch shows a method of reading the lower counter twice, I personally prefer to read the upper counter twice, before and after the lower counter, and if they differ do it again. This protects from the (hopefully rare) case where there is a full cycle of the lower counter between the two upper counter reads. Something like:

    uint32_t ulLo, ulHi1;
    uint32_t ulHi2 = RUN_TIMER_HI_HANDLE.Instance->CNT;
    do {
        ulHi1 = ulHi2;
        ulLo  = RUN_TIMER_LO_HANDLE.Instance->CNT;
        ulHi2 = RUN_TIMER_HI_HANDLE.Instance->CNT;
    } while (ulHi1 != ulHi2);
    return (ulHi1 << 16) | ulLo;
3 Likes
4

I figured it was due to the critical function being called by the task being called from the system tick ISR, routine. I did think of using the ISR version, but turning of IRQ’s worked so I left it at that.

The system does have two 32 bit timers, but I had to use them for a H Bridge pwm driver as I was constrained by pin-outs. Damn those pinouts!! I think your idea of reading the upper counter twice and comparing them is a good one. The machine can do this really fast in registers so it should not have much overhead. This is the best method and I will implement this. Turning off the interrupts has not caused any issues although it could affect fast stuff like ethernet in a heavily loaded system.

Thanks to you and Hein for your help.

Kind regards
Rob

5

Turning off interrupts only makes the problem less likely, not eliminate it. You still have the problem that between the two reads, the lower counter can roll-over, no matter what you do (unless you programmatically pause the counters). Any time consumed by an interrupt, could have still happened, it just needed to occur before turning off the interrupts.

The Run Time stats system in FreeRTOS already inherently counts interrupts that occur during an ISR as using time in the task it interrupts, so including it here is just being consistent.

Another thing to note is that most of the STM32 processors the optional CYCCNT register, that can also be used for timing. This cycles fast enough on some processors you may want to enable the 64-bit run-time accumulation and have your routine keep track of overflows.

6

That’s another good idea. I will have a look at it.

Kind regards
Rob