xiaoxiang781216 commented on code in PR #17642:
URL: https://github.com/apache/nuttx/pull/17642#discussion_r2660494000
##########
sched/hrtimer/hrtimer.h:
##########
@@ -32,26 +32,32 @@
#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
****************************************************************************/
/* Spinlock protecting access to the hrtimer RB-tree and timer state */
-extern spinlock_t g_hrtimer_spinlock;
+extern seqcount_t g_hrtimer_spinlock;
Review Comment:
ther is no benefit to replace spinlock with seqcount if you don't call
read_seqbegin/read_seqend.
##########
include/nuttx/hrtimer.h:
##########
@@ -134,13 +119,11 @@ 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;
+ hrtimer->cpus = 0;
Review Comment:
remove
##########
sched/hrtimer/hrtimer_cancel.c:
##########
@@ -83,59 +83,29 @@
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);
/* Enter critical section to protect the hrtimer tree and state */
- 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);
+ flags = write_seqlock_irqsave(&g_hrtimer_spinlock);
- 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 the 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 (RB_LEFT(&((hrtimer)->node), entry) == NULL)
Review Comment:
```suggestion
if (RB_LEFT(&hrtimer->node), entry) == NULL)
```
##########
sched/hrtimer/hrtimer.h:
##########
@@ -173,4 +178,66 @@ 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 ||
Review Comment:
remove ()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
