casaroli commented on code in PR #21001:
URL: https://github.com/apache/nuttx/pull/21001#discussion_r4392211828
##########
sched/hrtimer/hrtimer.h:
##########
@@ -540,18 +634,138 @@ static inline_function void hrtimer_wait(FAR hrtimer_t
*timer)
{
if (!up_interrupt_context() && !is_idle_task(this_task()))
{
- nxsched_usleep(HRTIMER_CANCEL_SYNC_DELAY_US);
+ usleep(HRTIMER_CANCEL_SYNC_DELAY_US);
Review Comment:
Kernel code must not call `usleep()` (a cancellation point that sets
`errno`), and `nxsched_usleep()` exists on master
(`include/nuttx/sched.h:1849`).
Fix: drop commit "Fix nxsched_usleep compilation error." and keep
`nxsched_usleep()`.
##########
sched/wdog/wdog.h:
##########
@@ -176,11 +179,9 @@ static inline_function void wd_timer_start(clock_t tick,
bool in_expiration)
#ifdef CONFIG_SCHED_TICKLESS_ALARM
up_alarm_tick_start(next_tick);
#else
- clock_t now = clock_systime_ticks();
-
- /* A passed deadline must fire now: timers may read the delay unsigned */
-
- up_timer_tick_start(clock_compare(next_tick, now) ? 0 : next_tick - now);
+ clock_t now = clock_systime_ticks();
+ clock_t delay = clock_compare(now, next_tick) ? next_tick - now : 0;
Review Comment:
This line gives the same result as master (37ccc03dfec already returns 0 for
a passed deadline); the commit only removes the comment.
Fix: drop commit "Fix unsigned underflow in wd_timer_start delay
calculation.".
##########
sched/Kconfig:
##########
@@ -2174,4 +2175,23 @@ config HRTIMER_TREE
endchoice
+config HRTIMER_PRECISION_SHIFT
+ int "HRTimer precision shift (0 = full precision)"
+ default 0
+ range 0 63
Review Comment:
With `range 0 63`, a value such as 40 rounds a timer by up to 18 minutes.
Fix: use `range 0 20`.
##########
sched/hrtimer/hrtimer.h:
##########
@@ -540,18 +634,138 @@ static inline_function void hrtimer_wait(FAR hrtimer_t
*timer)
{
if (!up_interrupt_context() && !is_idle_task(this_task()))
{
- nxsched_usleep(HRTIMER_CANCEL_SYNC_DELAY_US);
+ usleep(HRTIMER_CANCEL_SYNC_DELAY_US);
}
/* Otherwise, spin-wait is enough. */
}
}
/* Finally, we acquire the ownership of this hrtimer. */
+#endif
+}
+
+/****************************************************************************
+ * Name: hrtimer_process
+ *
+ * Description:
+ * Called from the timer interrupt handler to process expired
+ * high-resolution timers. If a timer has expired, its callback
+ * function will be executed in the context of the timer interrupt.
+ *
+ * Input Parameters:
+ * now - The current time (nsecs).
+ *
+ * Returned Value:
+ * None.
+ *
+ * Assumption:
+ * This function can only be called in interrupt context.
+ *
+ ****************************************************************************/
+
+static inline_function void hrtimer_process(uint64_t now)
+{
+ uint64_t delay;
+ int cpu = this_cpu();
+ irqstate_t flags = hrtimer_lock();
+ FAR hrtimer_t *hrtimer = hrtimer_get_first();
+ uint64_t expired = hrtimer->expired;
+ hrtimer_entry_t func = hrtimer->func;
+
+#ifdef CONFIG_SCHED_TICKLESS
+ /* Invalidate the g_hrtimer_expected */
+
+ g_hrtimer_expected = now;
+#endif
+
+ /* Acquire the lock and seize the ownership of the hrtimer queue.
+ * Then check if the timer has expired.
+ */
+
+ while (HRTIMER_TIME_BEFORE_EQ(expired, now))
+ {
+ /* Remove the expired timer from the timer queue */
+
+ hrtimer_remove(hrtimer);
+
+ hrtimer_mark_running(hrtimer, cpu);
+
+ /* Leave critical section before invoking the callback */
+
+ hrtimer_unlock(flags);
+
+ /* Invoke the timer callback */
+
+ delay = func(hrtimer, expired);
+
+ /* Ensure the delay is valid. */
+
+ DEBUGASSERT(HRTIMER_TIME_BEFORE_EQ(expired + delay,
+ now + HRTIMER_MAX_DELAY));
+
+ /* Re-enter critical section to update timer state */
+
+ flags = hrtimer_lock();
+
+ /* If the timer is periodic and has not been rearmed or
+ * cancelled concurrently, calculate next expiration and
+ * re-insert into the timer queue.
+ */
+
+ if (delay != 0u && hrtimer_is_running(hrtimer, cpu))
+ {
+ hrtimer->expired = expired + delay;
+ hrtimer->func = func;
+ hrtimer_insert(hrtimer);
+ }
+
+ /* Fetch the earliest active timer */
+
+ hrtimer = hrtimer_get_first();
+ expired = hrtimer->expired;
+ func = hrtimer->func;
+ }
+
+ hrtimer_unmark_running(cpu);
+
+ /* Note that the g_hrtimer_guard.expired is not monotonic. */
+
+ g_hrtimer_guard.expired = now + HRTIMER_GUARD_DELAY;
+
+ /* Schedule the next timer expiration */
+
+ hrtimer_reprogram(expired);
+
+ /* Release the lock and give up the ownership of the hrtimer queue. */
+
+ hrtimer_unlock(flags);
}
+
+/****************************************************************************
+ * Name: hrtimer_initialize
+ *
+ * Description:
+ * Called from the nx_start to initialize the hrtimer module.
+ *
+ * Input Parameters:
+ * None.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static inline_function void hrtimer_initialize(void)
Review Comment:
Nothing calls `hrtimer_initialize()`, and the static initializers already
queue the guard.
Fix: remove it.
##########
include/sys/tree_type.h:
##########
@@ -0,0 +1,55 @@
+/****************************************************************************
Review Comment:
This file copies BSD-2-Clause code from `sys/tree.h` without the license
text, and `LICENSE` does not list it.
Fix: copy the license block from `include/sys/tree.h` and add
`include/sys/tree_type.h` to `LICENSE`.
--
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]
