xiaoxiang781216 commented on code in PR #18131:
URL: https://github.com/apache/nuttx/pull/18131#discussion_r2726168207
##########
sched/hrtimer/hrtimer.h:
##########
@@ -309,5 +309,114 @@ static inline_function bool hrtimer_is_first(FAR
hrtimer_t *hrtimer)
#endif
}
+/****************************************************************************
+ * Name: hrtimer_mark_running
+ *
+ * Description:
+ * Mark the timer as running.
+ *
+ * Input Parameters:
+ * timer - The timer to be marked.
+ * cpu - The CPU core Id.
+ *
+ * Returned Value:
+ * None.
+ *
+ * Assumption:
+ * The caller must hold the lock.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SMP
+# define hrtimer_mark_running(timer, cpu) \
+ (g_hrtimer_running[cpu] = (uintptr_t)(timer))
+# define hrtimer_unmark_running(cpu) hrtimer_mark_running(NULL, cpu)
Review Comment:
move after line 337 and remove line 336
##########
sched/hrtimer/hrtimer.h:
##########
@@ -309,5 +309,114 @@ static inline_function bool hrtimer_is_first(FAR
hrtimer_t *hrtimer)
#endif
}
+/****************************************************************************
+ * Name: hrtimer_mark_running
+ *
+ * Description:
+ * Mark the timer as running.
+ *
+ * Input Parameters:
+ * timer - The timer to be marked.
+ * cpu - The CPU core Id.
+ *
+ * Returned Value:
+ * None.
+ *
+ * Assumption:
+ * The caller must hold the lock.
Review Comment:
don't need after read32/64?
##########
sched/hrtimer/hrtimer.h:
##########
@@ -309,5 +309,114 @@ static inline_function bool hrtimer_is_first(FAR
hrtimer_t *hrtimer)
#endif
}
+/****************************************************************************
+ * Name: hrtimer_mark_running
+ *
+ * Description:
+ * Mark the timer as running.
+ *
+ * Input Parameters:
+ * timer - The timer to be marked.
+ * cpu - The CPU core Id.
+ *
+ * Returned Value:
+ * None.
+ *
+ * Assumption:
+ * The caller must hold the lock.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SMP
+# define hrtimer_mark_running(timer, cpu) \
+ (g_hrtimer_running[cpu] = (uintptr_t)(timer))
+# define hrtimer_unmark_running(cpu) hrtimer_mark_running(NULL, cpu)
+#else
+# define hrtimer_mark_running(timer, cpu) UNUSED(cpu)
+# define hrtimer_unmark_running(cpu)
+#endif
+
+/****************************************************************************
+ * Name: hrtimer_has_ownership
Review Comment:
hrtimer_is_running and correct the comment
##########
sched/hrtimer/hrtimer.h:
##########
@@ -309,5 +309,114 @@ static inline_function bool hrtimer_is_first(FAR
hrtimer_t *hrtimer)
#endif
}
+/****************************************************************************
+ * Name: hrtimer_mark_running
+ *
+ * Description:
+ * Mark the timer as running.
+ *
+ * Input Parameters:
+ * timer - The timer to be marked.
+ * cpu - The CPU core Id.
+ *
+ * Returned Value:
+ * None.
+ *
+ * Assumption:
+ * The caller must hold the lock.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SMP
+# define hrtimer_mark_running(timer, cpu) \
+ (g_hrtimer_running[cpu] = (uintptr_t)(timer))
+# define hrtimer_unmark_running(cpu) hrtimer_mark_running(NULL, cpu)
+#else
+# define hrtimer_mark_running(timer, cpu) UNUSED(cpu)
+# define hrtimer_unmark_running(cpu)
+#endif
+
+/****************************************************************************
+ * Name: hrtimer_has_ownership
+ *
+ * Description:
+ * Check if the CPU core has ownership of the timer.
+ *
+ * Input Parameters:
+ * timer - The timer to be marked.
+ * cpu - The CPU core Id.
+ *
+ * Returned Value:
+ * true if the CPU core has ownership of the timer, false otherwise.
+ *
+ * Assumption:
+ * The caller must hold the lock.
Review Comment:
ditto
##########
sched/hrtimer/hrtimer_cancel.c:
##########
@@ -60,11 +60,13 @@
#ifdef CONFIG_SMP
static inline_function bool hrtimer_is_active(FAR hrtimer_t *hrtimer)
{
- bool is_active = false;
+ int cpu;
+ bool is_active = false;
+ uintptr_t cancelled = (uintptr_t)hrtimer | 0x1u;
Review Comment:
move into hrtimer_is_running like hrtimer_cancel_running
##########
sched/hrtimer/hrtimer_process.c:
##########
@@ -122,22 +119,18 @@ void hrtimer_process(uint64_t now)
flags = spin_lock_irqsave(&g_hrtimer_spinlock);
-#ifdef CONFIG_SMP
- g_hrtimer_running[cpu] = NULL;
-#endif
-
/* 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)
+ if (period != 0u && hrtimer_is_running(hrtimer, cpu))
{
hrtimer->expired += period;
/* Ensure no overflow occurs */
- DEBUGASSERT(hrtimer->expired > period);
+ DEBUGASSERT(hrtimer->expired >= period);
Review Comment:
should keep the origin assert since period can't be zero.
--
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]
