This is an automated email from the ASF dual-hosted git repository. ligd pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 1020bc74e8198827e987e9b40c3431da0ad67b7c Author: wangchengdong <[email protected]> AuthorDate: Mon Jan 5 19:58:02 2026 +0800 [EXPERIMENTAL]sched/hrtimr: Allow running/armed hrtimer to be restarted Allow running/armed hrtimer to be restarted to fix hrtimer bug: #17567 Co-authored-by: ouyangxiangzhen <[email protected]> Signed-off-by: Chengdong Wang <[email protected]> --- include/nuttx/hrtimer.h | 38 ++++-------- sched/hrtimer/hrtimer.h | 117 +++++++++++++++++++++++++++++++++---- sched/hrtimer/hrtimer_cancel.c | 86 ++++++++++++++------------- sched/hrtimer/hrtimer_initialize.c | 9 ++- sched/hrtimer/hrtimer_process.c | 58 +++++++++++------- sched/hrtimer/hrtimer_start.c | 79 +++++++------------------ 6 files changed, 226 insertions(+), 161 deletions(-) diff --git a/include/nuttx/hrtimer.h b/include/nuttx/hrtimer.h index 3f68adf6f08..0bf3cc90996 100644 --- a/include/nuttx/hrtimer.h +++ b/include/nuttx/hrtimer.h @@ -51,20 +51,6 @@ enum hrtimer_mode_e HRTIMER_MODE_REL /* Relative delay from now */ }; -/* High-resolution timer states - * - * State transitions are managed internally by the hrtimer framework. - * Callers must not modify the state directly. - */ - -enum hrtimer_state_e -{ - HRTIMER_STATE_INACTIVE = 0, /* Timer is inactive and not queued */ - HRTIMER_STATE_ARMED, /* Timer is armed and waiting for expiry */ - HRTIMER_STATE_RUNNING, /* Timer callback is currently executing */ - HRTIMER_STATE_CANCELED /* Timer canceled (callback may be running) */ -}; - /* Forward declarations */ struct hrtimer_s; @@ -79,7 +65,8 @@ typedef struct hrtimer_node_s hrtimer_node_t; * timer context and must not block. */ -typedef uint32_t (*hrtimer_cb)(FAR struct hrtimer_s *hrtimer); +typedef CODE uint64_t +(*hrtimer_cb)(FAR hrtimer_t *hrtimer, uint64_t expired); /* Red-black tree node used to order hrtimers by expiration time */ @@ -97,11 +84,9 @@ struct hrtimer_node_s struct hrtimer_s { - hrtimer_node_t node; /* RB-tree node for sorted insertion */ - enum hrtimer_state_e state; /* Current timer state */ - hrtimer_cb func; /* Expiration callback function */ - FAR void *arg; /* Argument passed to callback */ - uint64_t expired; /* Absolute expiration time (ns) */ + hrtimer_node_t node; /* RB-tree node for sorted insertion */ + hrtimer_cb func; /* Expiration callback function */ + uint64_t expired; /* Absolute expiration time (ns) */ }; /**************************************************************************** @@ -134,13 +119,10 @@ extern "C" ****************************************************************************/ static inline_function -void hrtimer_init(FAR hrtimer_t *hrtimer, - hrtimer_cb func, - FAR void *arg) +void hrtimer_init(FAR hrtimer_t *hrtimer, hrtimer_cb func) { - hrtimer->state = HRTIMER_STATE_INACTIVE; - hrtimer->func = func; - hrtimer->arg = arg; + memset(hrtimer, 0, sizeof(hrtimer_t)); + hrtimer->func = func; } /**************************************************************************** @@ -202,7 +184,7 @@ int hrtimer_cancel_sync(FAR hrtimer_t *hrtimer); * * Input Parameters: * hrtimer - Timer instance to start - * ns - Expiration time in nanoseconds + * expired - Expiration time in nanoseconds * mode - HRTIMER_MODE_ABS or HRTIMER_MODE_REL * * Returned Value: @@ -210,7 +192,7 @@ int hrtimer_cancel_sync(FAR hrtimer_t *hrtimer); ****************************************************************************/ int hrtimer_start(FAR hrtimer_t *hrtimer, - uint64_t ns, + uint64_t expired, enum hrtimer_mode_e mode); #undef EXTERN diff --git a/sched/hrtimer/hrtimer.h b/sched/hrtimer/hrtimer.h index de206e25eb7..09192ce65fd 100644 --- a/sched/hrtimer/hrtimer.h +++ b/sched/hrtimer/hrtimer.h @@ -32,6 +32,20 @@ #include <nuttx/clock.h> #include <nuttx/hrtimer.h> +#ifdef CONFIG_HRTIMER + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +/* Red-black tree head for managing active high-resolution timers. + * + * Timers are ordered by expiration time, the earliest expiring timer + * being the left-most (minimum) node in the tree. + */ + +RB_HEAD(hrtimer_tree_s, hrtimer_node_s); + /**************************************************************************** * Public Data ****************************************************************************/ @@ -44,13 +58,11 @@ extern spinlock_t g_hrtimer_spinlock; extern struct hrtimer_tree_s g_hrtimer_tree; -/**************************************************************************** - * Public Types - ****************************************************************************/ - -/* Red-black tree head for managing active hrtimers */ +/* Array of pointers to currently running high-resolution timers + * for each CPU in SMP configurations. Index corresponds to CPU ID. + */ -RB_HEAD(hrtimer_tree_s, hrtimer_node_s); +extern FAR hrtimer_t *g_hrtimer_running[CONFIG_SMP_NCPUS]; /**************************************************************************** * Public Function Prototypes @@ -147,14 +159,13 @@ int hrtimer_starttimer(uint64_t ns) * b - Pointer to the second hrtimer node. * * Returned Value: - * >0 if b expires before a - * 0 if a and b expire at the same time - * <0 if b expires after a + * <0 if a expires before b + * >=0 if a expires after b ****************************************************************************/ static inline_function int hrtimer_compare(FAR const hrtimer_node_t *a, - FAR const hrtimer_node_t *b) + FAR const hrtimer_node_t *b) { FAR const hrtimer_t *atimer = (FAR const hrtimer_t *)a; FAR const hrtimer_t *btimer = (FAR const hrtimer_t *)b; @@ -173,4 +184,90 @@ int hrtimer_compare(FAR const hrtimer_node_t *a, RB_PROTOTYPE(hrtimer_tree_s, hrtimer_node_s, entry, hrtimer_compare); +/**************************************************************************** + * Name: hrtimer_is_armed + * + * Description: + * Test whether a timer is currently armed (inserted into the RB-tree). + * + * Returned Value: + * true if armed, false otherwise. + ****************************************************************************/ + +static inline_function bool hrtimer_is_armed(FAR hrtimer_t *hrtimer) +{ + /* RB-tree root has NULL parent, so root must be checked explicitly */ + + return RB_PARENT(&hrtimer->node, entry) != NULL || + RB_ROOT(&g_hrtimer_tree) == &hrtimer->node; +} + +/**************************************************************************** + * Name: hrtimer_remove + * + * Description: + * Remove a timer from the RB-tree and mark it as unarmed. + ****************************************************************************/ + +static inline_function void hrtimer_remove(FAR hrtimer_t *hrtimer) +{ + RB_REMOVE(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node); + + /* Explicitly clear parent to mark the timer as unarmed */ + + RB_PARENT(&hrtimer->node, entry) = NULL; +} + +/**************************************************************************** + * Name: hrtimer_insert + * + * Description: + * Insert a timer into the RB-tree according to its expiration time. + ****************************************************************************/ + +static inline_function void hrtimer_insert(FAR hrtimer_t *hrtimer) +{ + RB_INSERT(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node); +} + +/**************************************************************************** + * Name: hrtimer_get_first + * + * Description: + * Return the earliest expiring armed timer. + * + * Returned Value: + * Pointer to the earliest timer, or NULL if none are armed. + ****************************************************************************/ + +static inline_function FAR hrtimer_t *hrtimer_get_first(void) +{ + return (FAR hrtimer_t *)RB_MIN(hrtimer_tree_s, &g_hrtimer_tree); +} + +/**************************************************************************** + * Name: hrtimer_is_first + * + * Description: + * Test whether the given high-resolution timer is the earliest + * expiring timer in the RB-tree. + * + * In a red-black tree ordered by expiration time, the earliest timer + * is represented by the left-most node. Therefore, a timer is the + * earliest one if it has no left child. + * + * Input Parameters: + * hrtimer - Pointer to the high-resolution timer to be tested. + * + * Returned Value: + * true - The timer is the earliest expiring armed timer. + * false - The timer is not the earliest timer. + ****************************************************************************/ + +static inline_function bool hrtimer_is_first(FAR hrtimer_t *hrtimer) +{ + return RB_LEFT(&hrtimer->node, entry) == NULL; +} + +#endif /* CONFIG_HRTIMER */ #endif /* __SCHED_HRTIMER_HRTIMER_H */ diff --git a/sched/hrtimer/hrtimer_cancel.c b/sched/hrtimer/hrtimer_cancel.c index bf974d240aa..27e53f0725f 100644 --- a/sched/hrtimer/hrtimer_cancel.c +++ b/sched/hrtimer/hrtimer_cancel.c @@ -27,9 +27,9 @@ #include <nuttx/config.h> #include <nuttx/arch.h> #include <nuttx/clock.h> - #include <errno.h> +#include "sched/sched.h" #include "hrtimer/hrtimer.h" /**************************************************************************** @@ -40,6 +40,40 @@ #define HRTIMER_CANCEL_SYNC_DELAY_MS 5 +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: hrtimer_is_active + * + * Description: + * Check whether a high-resolution timer is currently running on any CPU. + * + * Input Parameters: + * hrtimer - Pointer to the high-resolution timer to check. + * + * Returned Value: + * true - The timer is active on at least one CPU. + * false - The timer is not active. + ****************************************************************************/ + +static inline_function bool hrtimer_is_active(FAR hrtimer_t *hrtimer) +{ + bool is_active = false; + + for (int i = 0; i < CONFIG_SMP_NCPUS; i++) + { + if (g_hrtimer_running[i] == hrtimer) + { + is_active = true; + break; + } + } + + return is_active; +} + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -83,8 +117,8 @@ int hrtimer_cancel(FAR hrtimer_t *hrtimer) { FAR hrtimer_t *first; - irqstate_t flags; - int ret = OK; + irqstate_t flags; + int ret = OK; DEBUGASSERT(hrtimer != NULL); @@ -92,50 +126,20 @@ int hrtimer_cancel(FAR hrtimer_t *hrtimer) flags = spin_lock_irqsave(&g_hrtimer_spinlock); - /* Capture the current earliest timer before any modification */ - - first = (FAR hrtimer_t *)RB_MIN(hrtimer_tree_s, &g_hrtimer_tree); - - switch (hrtimer->state) + if (hrtimer_is_armed(hrtimer)) { - case HRTIMER_STATE_ARMED: - { - /* Remove the timer from the active tree */ - - RB_REMOVE(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node); - hrtimer->state = HRTIMER_STATE_INACTIVE; - break; - } + hrtimer_remove(hrtimer); + } - case HRTIMER_STATE_RUNNING: - { - /* The callback is currently executing. - * - * Mark the timer as canceled so it will not be re-armed or - * executed again. The running callback is allowed to complete. - * - * NOTE: The timer node is expected to have already been removed - * from the tree when the callback started executing. - */ - - hrtimer->state = HRTIMER_STATE_CANCELED; - break; - } + /* Mark timer as cancelled */ - case HRTIMER_STATE_INACTIVE: - case HRTIMER_STATE_CANCELED: - default: - { - ret = -EINVAL; - break; - } - } + hrtimer->expired = UINT64_MAX; /* If the canceled timer was the earliest one, update the hardware timer */ - if ((ret == OK) && (first == hrtimer)) + if (hrtimer_is_first(hrtimer)) { - first = (FAR hrtimer_t *)RB_MIN(hrtimer_tree_s, &g_hrtimer_tree); + first = hrtimer_get_first(); if (first != NULL) { ret = hrtimer_starttimer(first->expired); @@ -196,7 +200,7 @@ int hrtimer_cancel_sync(FAR hrtimer_t *hrtimer) * and the state becomes inactive. */ - while (hrtimer->state != HRTIMER_STATE_INACTIVE) + while (hrtimer_is_active(hrtimer)) { if (cansleep) { diff --git a/sched/hrtimer/hrtimer_initialize.c b/sched/hrtimer/hrtimer_initialize.c index 02ac80d3ce6..0bde89d9c2f 100644 --- a/sched/hrtimer/hrtimer_initialize.c +++ b/sched/hrtimer/hrtimer_initialize.c @@ -32,6 +32,12 @@ * Public Data ****************************************************************************/ +/* Array of pointers to currently running high-resolution timers + * for each CPU in SMP configurations. Index corresponds to CPU ID. + */ + +FAR hrtimer_t *g_hrtimer_running[CONFIG_SMP_NCPUS]; + /* Global spinlock protecting the high-resolution timer subsystem. * * This spinlock serializes access to the hrtimer red-black tree and @@ -49,8 +55,7 @@ spinlock_t g_hrtimer_spinlock = SP_UNLOCKED; * The tree is ordered by absolute expiration time. */ -struct hrtimer_tree_s g_hrtimer_tree = - RB_INITIALIZER(g_hrtimer_tree); +struct hrtimer_tree_s g_hrtimer_tree = RB_INITIALIZER(g_hrtimer_tree); /**************************************************************************** * Public Functions diff --git a/sched/hrtimer/hrtimer_process.c b/sched/hrtimer/hrtimer_process.c index 3e7a3c938f6..d5b9183e1ec 100644 --- a/sched/hrtimer/hrtimer_process.c +++ b/sched/hrtimer/hrtimer_process.c @@ -28,6 +28,7 @@ #include <assert.h> #include <nuttx/arch.h> #include <nuttx/clock.h> + #include "hrtimer/hrtimer.h" /**************************************************************************** @@ -54,7 +55,7 @@ * if no timers remain. * * Input Parameters: - * ts - Pointer to the current high-resolution timestamp. + * now - Current high-resolution timestamp. * * Returned Value: * None. @@ -69,9 +70,11 @@ void hrtimer_process(uint64_t now) { FAR hrtimer_t *hrtimer; - uint64_t expired; - uint32_t period = 0; irqstate_t flags; + hrtimer_cb func; + uint64_t expired; + uint64_t period; + int cpu = this_cpu(); /* Lock the hrtimer RB-tree to protect access */ @@ -79,53 +82,64 @@ void hrtimer_process(uint64_t now) /* Fetch the earliest active timer */ - hrtimer = (FAR hrtimer_t *)RB_MIN(hrtimer_tree_s, &g_hrtimer_tree); + hrtimer = hrtimer_get_first(); while (hrtimer != NULL) { + func = hrtimer->func; + + /* Ensure the timer callback is valid */ + + DEBUGASSERT(func != NULL); + + expired = hrtimer->expired; + /* Check if the timer has expired */ - if (!clock_compare(hrtimer->expired, now)) + if (!clock_compare(expired, now)) { break; } /* Remove the expired timer from the active tree */ - RB_REMOVE(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node); - - /* Ensure the timer callback is valid */ + hrtimer_remove(hrtimer); - DEBUGASSERT(hrtimer->func != NULL); + g_hrtimer_running[cpu] = hrtimer; - hrtimer->state = HRTIMER_STATE_RUNNING; + /* Leave critical section before invoking the callback */ spin_unlock_irqrestore(&g_hrtimer_spinlock, flags); /* Invoke the timer callback */ - period = hrtimer->func(hrtimer); + period = func(hrtimer, expired); + + /* Re-enter critical section to update timer state */ flags = spin_lock_irqsave(&g_hrtimer_spinlock); - if ((hrtimer->state == HRTIMER_STATE_CANCELED) || (period == 0)) - { - /* Timer is canceled or one-shot; mark it inactive */ + g_hrtimer_running[cpu] = NULL; - hrtimer->state = HRTIMER_STATE_INACTIVE; - } - else - { - /* Restart the periodic timer */ + /* If the timer is periodic and has not been rearmed or + * cancelled concurrently, + * compute next expiration and reinsert into RB-tree + */ + if (period > 0 && hrtimer->expired == expired) + { hrtimer->expired += period; - hrtimer->state = HRTIMER_STATE_ARMED; - RB_INSERT(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node); + + /* Ensure no overflow occurs */ + + DEBUGASSERT(hrtimer->expired > period); + + hrtimer_insert(hrtimer); } /* Fetch the next earliest timer */ - hrtimer = (FAR hrtimer_t *)RB_MIN(hrtimer_tree_s, &g_hrtimer_tree); + hrtimer = hrtimer_get_first(); } /* Schedule the next timer expiration */ diff --git a/sched/hrtimer/hrtimer_start.c b/sched/hrtimer/hrtimer_start.c index b3bde40eee0..caba6fc50f8 100644 --- a/sched/hrtimer/hrtimer_start.c +++ b/sched/hrtimer/hrtimer_start.c @@ -31,53 +31,6 @@ #include "hrtimer/hrtimer.h" -/**************************************************************************** - * Private Functions - ****************************************************************************/ - -/**************************************************************************** - * Name: hrtimer_insert - * - * Description: - * Insert the given high-resolution timer into the active timer RB-tree. - * If the timer is already in the tree, it will be replaced. - * If the inserted timer becomes the earliest timer in the tree, the - * hardware timer will be configured to fire at its expiration. - * - * Input Parameters: - * hrtimer - Pointer to the hrtimer structure to be inserted. - * - * Returned Value: - * OK (0) on success; negated errno value on failure. - * - * Assumptions/Notes: - * - This function should be called with interrupts disabled or under - * spinlock protection to ensure RB-tree integrity. - * - If the timer is currently running, insertion is rejected with -EBUSY. - ****************************************************************************/ - -static inline int hrtimer_insert(FAR hrtimer_t *hrtimer) -{ - DEBUGASSERT(hrtimer != NULL); - - /* Insert (or replace) the timer into the RB-tree ordered by expiration */ - - RB_INSERT(hrtimer_tree_s, &g_hrtimer_tree, &hrtimer->node); - - /* Mark the timer as armed */ - - hrtimer->state = HRTIMER_STATE_ARMED; - - /* If the inserted timer is now the earliest, start hardware timer */ - - if (&hrtimer->node == RB_MIN(hrtimer_tree_s, &g_hrtimer_tree)) - { - return hrtimer_starttimer(hrtimer->expired); - } - - return OK; -} - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -91,7 +44,7 @@ static inline int hrtimer_insert(FAR hrtimer_t *hrtimer) * * Input Parameters: * hrtimer - Pointer to the hrtimer structure. - * ns - Expiration time in nanoseconds. Interpretation + * expired - Expiration time in nanoseconds. Interpretation * depends on mode. * mode - Timer mode (HRTIMER_MODE_ABS or HRTIMER_MODE_REL). * @@ -107,7 +60,7 @@ static inline int hrtimer_insert(FAR hrtimer_t *hrtimer) ****************************************************************************/ int hrtimer_start(FAR hrtimer_t *hrtimer, - uint64_t ns, + uint64_t expired, enum hrtimer_mode_e mode) { irqstate_t flags; @@ -119,30 +72,40 @@ int hrtimer_start(FAR hrtimer_t *hrtimer, flags = spin_lock_irqsave(&g_hrtimer_spinlock); - /* Reject start if the timer is already running or armed */ - - if ((hrtimer->state == HRTIMER_STATE_RUNNING) || - (hrtimer->state == HRTIMER_STATE_ARMED)) + if (hrtimer_is_armed(hrtimer)) { - spin_unlock_irqrestore(&g_hrtimer_spinlock, flags); - return -EBUSY; + hrtimer_remove(hrtimer); } /* Compute absolute expiration time */ if (mode == HRTIMER_MODE_ABS) { - hrtimer->expired = ns; + hrtimer->expired = expired; } else { - hrtimer->expired = hrtimer_gettime() + ns; + hrtimer->expired = hrtimer_gettime() + expired; } + /* Ensure expiration time does not overflow */ + + DEBUGASSERT(hrtimer->expired >= expired); + /* Insert the timer into the RB-tree */ - ret = hrtimer_insert(hrtimer); + hrtimer_insert(hrtimer); + + /* If the inserted timer is now the earliest, start hardware timer */ + + if (hrtimer_is_first(hrtimer)) + { + ret = hrtimer_starttimer(hrtimer->expired); + } + + /* Release spinlock and restore interrupts */ spin_unlock_irqrestore(&g_hrtimer_spinlock, flags); + return ret; }
