Tickless critical sections

lantczak wrote on Monday, October 03, 2016:

I see possible issue in using critical sections in function vPortSuppressTicksAndSleep().

		/* Stop the SysTick momentarily.  The time the SysTick is stopped for
		is accounted for as best it can be, but using the tickless mode will
		inevitably result in some tiny drift of the time maintained by the
		kernel with respect to calendar time. */
		portNVIC_SYSTICK_CTRL_REG &= ~portNVIC_SYSTICK_ENABLE_BIT;

		/* Calculate the reload value required to wait xExpectedIdleTime
		tick periods.  -1 is used because this code will execute part way
		through one of the tick periods. */
		ulReloadValue = portNVIC_SYSTICK_CURRENT_VALUE_REG + ( ulTimerCountsForOneTick * ( xExpectedIdleTime - 1UL ) );
		if( ulReloadValue > ulStoppedTimerCompensation )
		{
			ulReloadValue -= ulStoppedTimerCompensation;
		}

		/* Enter a critical section but don't use the taskENTER_CRITICAL()
		method as that will mask interrupts that should exit sleep mode. */
		__asm volatile( "cpsid i" );
		__asm volatile( "dsb" );
		__asm volatile( "isb" );

If interrupt occur after SYSTICK stop and before critical section it will provide inaccuracy in uReloadValue calculation.

Similar issue is present in wakeup case:

			if( xModifiableIdleTime > 0 )
			{
				__asm volatile( "dsb" );
				__asm volatile( "wfi" );
				__asm volatile( "isb" );
			}
			configPOST_SLEEP_PROCESSING( xExpectedIdleTime );

			/* Stop SysTick.  Again, the time the SysTick is stopped for is
			accounted for as best it can be, but using the tickless mode will
			inevitably result in some tiny drift of the time maintained by the
			kernel with respect to calendar time. */
			ulSysTickCTRL = portNVIC_SYSTICK_CTRL_REG;
			portNVIC_SYSTICK_CTRL_REG = ( ulSysTickCTRL & ~portNVIC_SYSTICK_ENABLE_BIT );

			/* Re-enable interrupts - see comments above the cpsid instruction()
			above. */
			__asm volatile( "cpsie i" );

The interrupt is enabled and systick is not running if IRQ occur before systick start then time spend in IRQ will cause inaccuracy.

Improvement proposition:
The critical sections should be moved to protect regions when systick reload value is calculated. Do you agree?

Additional possible improvement:
There is a define portMISSED_COUNTS_FACTOR used to tune tickles time drift. It would be better to use Program Counter to calculate ‘real’ time when systick was off and use it to compensate time drift

Best regards
Lukasz Antczak

rtel wrote on Monday, October 03, 2016:

Thank you for your feedback.

If you stop and start the clock it is inevitable that there will be
drift, and the ulStoppedTimerCompensation variable is used to alleviate
that, but not fix that. I think that is made clear in the comments and
documentation pages.

If interrupt occur after SYSTICK stop and before critical section it
will provide inaccuracy in uReloadValue calculation.

If that happens then the call to eTaskConfirmSleepModeStatus() which
occurs immediately after the snippet you posted will return eAbortSleep
because xYieldPending would be true - and the entry into sleep mode
would be aborted and the systick returned to its original frequency.

Similar issue is present in wakeup case:

         if( xModifiableIdleTime > 0 )
         {
             __asm volatile( "dsb" );
             __asm volatile( "wfi" );
             __asm volatile( "isb" );
         }
         configPOST_SLEEP_PROCESSING( xExpectedIdleTime );

         /* Stop SysTick.  Again, the time the SysTick is stopped for is
         accounted for as best it can be, but using the tickless mode will
         inevitably result in some tiny drift of the time maintained by the
         kernel with respect to calendar time. */
         ulSysTickCTRL = portNVIC_SYSTICK_CTRL_REG;
         portNVIC_SYSTICK_CTRL_REG = ( ulSysTickCTRL & ~portNVIC_SYSTICK_ENABLE_BIT );

         /* Re-enable interrupts - see comments above the cpsid instruction()
         above. */
         __asm volatile( "cpsie i" );

The interrupt is enabled and systick is not running if IRQ occur before
systick start then time spend in IRQ will cause inaccuracy.

It is necessary to stop the counter so it is possible to determine how
long the system was asleep for. It is also desirable that an interrupt
that caused the system to leave a sleep state gets processed as quickly
as possible - extending the critical section could improve time keeping
but it would be at the expense of responsiveness. Adding additional
compensation calculations would again improve time keeping at the
expense of responsiveness.

What you are looking at is the ‘default’ implementation. The needs of
each application are different, and even the clock used is likely to be
different as using the SysTick does not allow entry into deep sleeps
where the SysTick would not even be running. Therefore all the default
implementations (for all the compilers) are weakly defined symbols to
allow you to replace the default with any implementation you need.

lantczak wrote on Monday, October 03, 2016:

Thank you for fast response.

Small clarification:
The xYieldPending will be set only if IRQ is switching some task to ready state. If IRQ is doing some work and doesn’t switch task from block to ready state then IRQ execution time will cause inaccuracy.
Is my understanding correct?

I will try to use cycle counter to calculate time when systick was off. It should solve time drift issue.

rtel wrote on Monday, October 03, 2016:

The “suppress ticks and sleep” function is called with the scheduler
suspended, which means any tick interrupt occurring will set
xYieldPending to true.