Skip to content

Conversation

@Fix-Point
Copy link
Contributor

Summary

This PR is part of the #17675. It introduces a series of fixes and improvements to the high-resolution timer (hrtimer) subsystem, primarily focusing on functional correctness, synchronization in SMP environments, and API enhancements.

The changes were necessary to address several issues:

  1. SMP Synchronization: On 32-bit platforms, read/write operations to the global g_hrtimer_running array were not atomic, potentially leading to data races. This is fixed by replacing the spinlock with a sequence lock (seqcount) to protect concurrent access.
  2. Timer State Management: The previous mechanism using hrtimer->expired as a version number was flawed as it's not monotonic. The new implementation introduces an ownership encoding scheme (using the LSB of the pointer in g_hrtimer_running) to enforce the invariant that a cancelled timer cannot be modified again, ensuring correct behavior during concurrent cancellation and callback execution.
  3. API Improvements: A new hrtimer_gettime() API is added to allow querying the remaining delay of a timer in nanoseconds. For code reusability, the internal hrtimer_gettime() helper is moved to the public interface as clock_systime_nsec().
  4. Functional Fixes: Corrects the timer programming logic for tickless and non-tickless alarm modes, ensures relative delays in hrtimer_start are capped at HRTIMER_MAX_DELAY to prevent overflow, and simplifies the cancellation logic.
  5. Code Simplification: The hrtimer_cancel and hrtimer_cancel_sync functions are refactored to have clearer semantics (asynchronous vs. synchronous cancellation) and a helper function hrtimer_wait is introduced to consolidate the waiting logic.

The main modifications include:

  • Adding clock_systime_nsec() inline function to clock.h.
  • Defining HRTIMER_MAX_DELAY and updating documentation in hrtimer.h.
  • Adding the hrtimer_gettime() API prototype.
  • Replacing the global spinlock with a sequence lock (g_hrtimer_lock) in the internal header and implementation files.
  • Rewriting SMP ownership tracking using the new encoding scheme.
  • Adding a new source file hrtimer_gettime.c.
  • Updating hrtimer_start, hrtimer_cancel, hrtimer_process, and related functions to use the new synchronization and state management mechanisms.

Impact

  • Users (Developers):
    • The semantics of hrtimer_cancel and hrtimer_cancel_sync are now more explicitly documented. hrtimer_cancel provides limited ownership (allows restarting but not freeing), while hrtimer_cancel_sync provides full ownership (allows safe deallocation). Users must choose the appropriate function based on their needs to avoid concurrency errors.
    • A new API hrtimer_gettime(FAR hrtimer_t *timer) is available to query the time until a timer's next expiration in nanoseconds.
    • The utility function clock_systime_nsec() is available for other parts of the system to get the current system time in nanoseconds.
  • Build Process: The build system files (CMakeLists.txt, Make.defs) are updated to include the new hrtimer_gettime.c source file.
  • Hardware (SMP): The changes significantly improve the correctness of hrtimer operations in SMP environments by ensuring proper synchronization and state management across cores. For non-SMP systems, the synchronization overhead might change slightly due to the lock type change.
  • Documentation: The in-code documentation for the modified functions (hrtimer_cancel, hrtimer_cancel_sync, hrtimer_start) has been updated to reflect the new behavior, assumptions, and return values. This provides clearer guidance for developers.
  • Security & Robustness: The fixes prevent potential race conditions and invalid state transitions in SMP, leading to a more stable system. Capping the relative delay prevents overflow-related undefined behavior.
  • Compatibility: The return value of hrtimer_cancel now has an additional meaning: a value > 0 indicates the timer callback is running. Code that only checked for negative values (errors) might need adjustment if it intends to handle the "running" case. The internal locking mechanism change is transparent to the API but ensures correct concurrent operation.

Test

Tested on rv-virt:smp, ostest passed.

This commit fixed the functional correctness issues in tick mode
and non-tickless alarm mode.
- In tick mode, we do not need reprogram the timer.
- In non-tickless alarm mode, the up_timer_start receive the relative time as the parameter.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
@github-actions github-actions bot added Area: OS Components OS Components issues Size: L The size of the change in this PR is large labels Jan 23, 2026
@Fix-Point Fix-Point force-pushed the hrtimer_refactor_p2 branch 2 times, most recently from 9f8b1ef to 7059bf8 Compare January 23, 2026 07:28
Since the hrtimer->expired is not monotonic, it can not be used as the
version. This commit added the ownership encoding to ensure the
invariant that the cancelled the timer can not modify the hrtimer again.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
On 32-bit platform with 64-bit pointer, read/write to the
`g_hrtimer_running` is not atomic, we should protect it with the
seqcount to ensure the value we read is correct.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit added hrtimer_wait to simplify the wait.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit move the hrtimer_gettime to clock_systime_nsec to improve
the code reusability.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit added hrtimer_gettime API to support the query of the
rest of the delay time of the hrtimer in nanoseconds.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit simplified the hrtimer_cancel.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit fixed the functional correctness issue in hrtimer_start by
adding HRTIMER_MAX_DELAY fr the HRTIMER_MODE_REL.

Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
@Fix-Point Fix-Point force-pushed the hrtimer_refactor_p2 branch from 7059bf8 to 533e2e6 Compare January 23, 2026 07:35
Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fix-Point thanks for these modifications, but I think you need to update the Documentation https://nuttx.apache.org/docs/latest/reference/os/time_clock.html#high-resolution-timer-interfaces to reflect the this you are doing here. Otherwise users will end up with an incorrect Documentation

@acassis acassis requested a review from anchao January 23, 2026 13:46
@acassis
Copy link
Contributor

acassis commented Jan 23, 2026

@wangchdo Please help to review this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OS Components OS Components issues Size: L The size of the change in this PR is large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants