search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
11k stars 6.69k forks source link

Sys clock - ticks_t definition for 32bit unsigned and 64bit signed breaks Z_TICK_ABS #80974

Open optical-o opened 3 weeks ago

optical-o commented 3 weeks ago

Strangely, the two types are changing from unsigned to signed. I have encountered this in a single-threaded application. I'm not sure if it impacts anything else but there are probably mistakes in more places because of this not just in the single-threaded version. The usage of Z_TICK_ABS assumes that the type is signed.

The 32bit unsinged variant breaks Z_TICK_ABS in nothread.c so that the result of Z_TICK_ABS cannot be lower than zero because of the unsigned 32bit integer resulting in k_sleeping being always set virtually forever.

optical-o commented 3 weeks ago

After investigation and deep dive into the implementation in shed.c i concluded that the implementation in nothread.c is wrong. The expected_wakeup_tick does not make sense here since we need the number of ticks to busy_wait. It appears that it was incorrectly copied from the sched.c in commit by @nashif.

optical-o commented 3 weeks ago

Case of signed timer 64-bit integer for sleep ticks = 1 results in calculation: Z_TICK_ABS(1) = Z_FOREVER(-1) - 1 - (1) -> -2 Which triggers: branch: expected_wakeup_ticks = ticks + sys_clock_tick_get_32(); This is by my opinion incorrect and causes the sleep not to sleep tick but depending on the sys_clock_tick_get_32(). I think that the entire branch for the nothread with sys_clock_tick_get_32 does not belong there. Am iIcorrect or just chasing ghosts by my lack of knowledge of the zephyr kernel timing system ?

mmahadevan108 commented 3 weeks ago

@optical-o , can you submit a PR with your suggested fix