Timers: In Tickless mode, need to stop the interval timer before inserted a new delay into the timer list. Otherwise, the time is incorrect on the first entry of the list

3 files changed, 145 insertions, 60 deletions

diff --git a/nuttx/sched/sched/sched.h b/nuttx/sched/sched/sched.h
index 47d6838f8..c6a6fd800 100644
--- a/nuttx/sched/sched/sched.h
+++ b/nuttx/sched/sched/sched.h
@@ -234,9 +234,14 @@ int  sched_reprioritize(FAR struct tcb_s *tcb, int sched_priority);
      sched_setpriority(tcb,sched_priority)
 #endif
 
+
 #ifdef CONFIG_SCHED_TICKLESS
+unsigned int sched_timer_cancel(void);
+void sched_timer_resume(void);
 void sched_timer_reassess(void);
 #else
+#  define sched_timer_cancel() (0)
+#  define sched_timer_resume()
 #  define sched_timer_reassess()
 #endif
 
diff --git a/nuttx/sched/sched/sched_timerexpiration.c b/nuttx/sched/sched/sched_timerexpiration.c
index c721005d9..2bc737ecb 100644
--- a/nuttx/sched/sched/sched_timerexpiration.c
+++ b/nuttx/sched/sched/sched_timerexpiration.c
@@ -235,28 +235,28 @@ sched_process_timeslice(unsigned int ticks, bool noswitches)
  *   noswitches - True: Can't do context switches now.
  *
  * Returned Value:
- *   Base code implementation assumes that this function is called from
- *   interrupt handling logic with interrupts disabled.
+ *   The number of ticks to use when setting up the next timer.  Zero if
+ *   there is no interesting event to be timed.
  *
  ****************************************************************************/
 
-static void sched_timer_process(unsigned int ticks, bool noswitches)
+static unsigned int sched_timer_process(unsigned int ticks, bool noswitches)
 {
-  unsigned int nextime = UINT_MAX;
-  bool needtimer = false;
-  uint32_t msecs;
-  uint32_t secs;
-  uint32_t nsecs;
+#if CONFIG_RR_INTERVAL > 0
+  unsigned int cmptime = UINT_MAX;
+#endif
+  unsigned int rettime  = 0;
   unsigned int tmp;
-  int ret;
 
   /* Process watchdogs */
 
   tmp = wd_timer(ticks);
   if (tmp > 0)
     {
-      nextime   = tmp;
-      needtimer = true;
+#if CONFIG_RR_INTERVAL > 0
+      cmptime = tmp;
+#endif
+      rettime  = tmp;
     }
 
 #if CONFIG_RR_INTERVAL > 0
@@ -265,29 +265,52 @@ static void sched_timer_process(unsigned int ticks, bool noswitches)
    */
 
   tmp = sched_process_timeslice(ticks, noswitches);
-  if (tmp > 0 && tmp < nextime)
+  if (tmp > 0 && tmp < cmptime)
     {
-      nextime   = tmp;
-      needtimer = true;
+      rettime  = tmp;
     }
 #endif
 
+  return rettime;
+}
+
+/****************************************************************************
+ * Name:  sched_timer_start
+ *
+ * Description:
+ *   Start the interval timer.
+ *
+ * Input Parameters:
+ *   ticks - The number of ticks defining the timer interval to setup.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static void sched_timer_start(unsigned int ticks)
+{
+  uint32_t msecs;
+  uint32_t secs;
+  uint32_t nsecs;
+  int ret;
+
   /* Set up the next timer interval (or not) */
 
   g_timer_interval = 0;
-  if (needtimer)
+  if (ticks > 0)
     {
       struct timespec ts;
 
       /* Save new timer interval */
 
-      g_timer_interval = nextime;
+      g_timer_interval = ticks;
 
       /* Convert ticks to a struct timespec that up_timer_start() can
        * understand.
        */
 
-      msecs = TICK2MSEC(nextime);
+      msecs = TICK2MSEC(ticks);
       secs  = msecs / MSEC_PER_SEC;
       nsecs = (msecs - (secs * MSEC_PER_SEC)) * NSEC_PER_MSEC;
 
@@ -328,37 +351,43 @@ static void sched_timer_process(unsigned int ticks, bool noswitches)
 void sched_timer_expiration(void)
 {
   unsigned int elapsed;
+  unsigned int nexttime;
+
+  /* Get the interval associated with las expiration */
 
   elapsed          = g_timer_interval;
   g_timer_interval = 0;
-  sched_timer_process(elapsed, false);
+
+  /* Process the timer ticks and set up the next interval (or not) */
+
+  nexttime = sched_timer_process(elapsed, false);
+  sched_timer_start(nexttime);
 }
 
 /****************************************************************************
- * Name:  sched_timer_reassess
+ * Name:  sched_timer_cancel
  *
  * Description:
- *   It is necessary to re-assess the timer interval in several
- *   circumstances:
+ *   Stop the current timing activity.  This is currently called just before
+ *   a new entry is inserted at the head of a timer list and also as part
+ *   of the processing of sched_timer_reassess().
  *
- *   - If the watchdog at the head of the expiration list changes (or if its
- *     delay changes.  This can occur as a consequence of the actions of
- *     wd_start() or wd_cancel().
- *   - Any context switch occurs, changing the task at the head of the
- *     ready-to-run list.  The task at the head of list may require
- *     different timeslice processing (or no timeslice at all).
- *   - When pre-emption is re-enabled.  A previous time slice may have
- *     expired while pre-emption was enabled and now needs to be executed.
+ *   This function(1) cancels the current timer, (2) determines how much of
+ *   the interval has elapsed, (3) completes any partially timed events
+ *   (including updating the delay of the timer at the head of the timer
+ *   list), and (2) returns the number of ticks that would be needed to
+ *   resume timing and complete this delay.
  *
  * Input Parameters:
  *   None
  *
  * Returned Value:
- *   None
+ *   Number of timer ticks that would be needed to complete the delay (zero
+ *   if the timer was not active).
  *
  ****************************************************************************/
 
-void sched_timer_reassess(void)
+unsigned int sched_timer_cancel(void)
 {
   struct timespec ts;
   unsigned int ticks;
@@ -382,6 +411,71 @@ void sched_timer_reassess(void)
 
   elapsed          = g_timer_interval - ticks;
   g_timer_interval = 0;
-  sched_timer_process(elapsed, true);
+
+  /* Process the timer ticks and return the next interval */
+
+  return sched_timer_process(elapsed, true);
+}
+
+/****************************************************************************
+ * Name:  sched_timer_resume
+ *
+ * Description:
+ *   Re-assess the next deadline and restart the interval timer.  This is
+ *   called from wd_start() after it has inserted a new delay into the
+ *   timer list.
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void sched_timer_resume(void)
+{
+  unsigned int nexttime;
+
+  /* Reassess the next deadline (by simply processing a zero ticks expired)
+   * and set up the next interval (or not).
+   */
+
+  nexttime = sched_timer_process(0, true);
+  sched_timer_start(nexttime);
+}
+
+/****************************************************************************
+ * Name:  sched_timer_reassess
+ *
+ * Description:
+ *   It is necessary to re-assess the timer interval in several
+ *   circumstances:
+ *
+ *   - If the watchdog at the head of the expiration list changes (or if its
+ *     delay changes.  This can occur as a consequence of the actions of
+ *     wd_start() or wd_cancel().
+ *   - Any context switch occurs, changing the task at the head of the
+ *     ready-to-run list.  The task at the head of list may require
+ *     different timeslice processing (or no timeslice at all).
+ *   - When pre-emption is re-enabled.  A previous time slice may have
+ *     expired while pre-emption was enabled and now needs to be executed.
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void sched_timer_reassess(void)
+{
+  unsigned int nexttime;
+
+  /* Cancel and restart the timer */
+
+  nexttime = sched_timer_cancel();
+  sched_timer_start(nexttime);
 }
 #endif /* CONFIG_SCHED_TICKLESS */
diff --git a/nuttx/sched/wdog/wd_start.c b/nuttx/sched/wdog/wd_start.c
index 8488eb898..6ba19da39 100644
--- a/nuttx/sched/wdog/wd_start.c
+++ b/nuttx/sched/wdog/wd_start.c
@@ -233,9 +233,6 @@ int wd_start(WDOG_ID wdog, int delay, wdentry_t wdentry,  int argc, ...)
   FAR wdog_t *next;
   int32_t    now;
   irqstate_t saved_state;
-#ifdef CONFIG_SCHED_TICKLESS
-  bool       reassess = false;
-#endif
   int        i;
 
   /* Verify the wdog */
@@ -288,6 +285,16 @@ int wd_start(WDOG_ID wdog, int delay, wdentry_t wdentry,  int argc, ...)
       delay--;
     }
 
+#ifdef CONFIG_SCHED_TICKLESS
+  /* Cancel the interval timer that drives the timing events.  This will cause
+   * wd_timer to be called which update the delay value for the first time
+   * at the head of the timer list (there is a possibility that it could even
+   * remove it).
+   */
+
+  (void)sched_timer_cancel();
+#endif
+
   /* Do the easy case first -- when the watchdog timer queue is empty. */
 
   if (g_wdactivelist.head == NULL)
@@ -295,14 +302,6 @@ int wd_start(WDOG_ID wdog, int delay, wdentry_t wdentry,  int argc, ...)
       /* Add the watchdog to the head == tail of the queue. */
 
       sq_addlast((FAR sq_entry_t*)wdog, &g_wdactivelist);
-
-#ifdef CONFIG_SCHED_TICKLESS
-      /* Whenever the watchdog at the head of the queue changes, then we
-       * need to reassess the interval timer setting.
-       */
-
-      reassess = true;
-#endif
     }
 
   /* There are other active watchdogs in the timer queue */
@@ -348,14 +347,6 @@ int wd_start(WDOG_ID wdog, int delay, wdentry_t wdentry,  int argc, ...)
               /* Insert the watchdog at the head of the list */
 
               sq_addfirst((FAR sq_entry_t*)wdog, &g_wdactivelist);
-
-#ifdef CONFIG_SCHED_TICKLESS
-              /* If the watchdog at the head of the queue changes, then we
-               * need to reassess the interval timer setting.
-               */
-
-              reassess = true;
-#endif
             }
           else
             {
@@ -395,17 +386,12 @@ int wd_start(WDOG_ID wdog, int delay, wdentry_t wdentry,  int argc, ...)
   wdog->active = true;
 
 #ifdef CONFIG_SCHED_TICKLESS
-  /* Reassess the interval timer that will generate the next interval event.
-   * In many cases, this will be unnecessary:  This is really only necessary
-   * when the watchdog timer at the head of the queue is change.  If the
-   * timer is inserted later in the queue then the timer at the head is
-   * unchanged.
+  /* Resume the interval timer that will generate the next interval event.
+   * If the timer at the head of the list changed, then this will pick that
+   * new delay.
    */
 
-  if (reassess)
-    {
-      sched_timer_reassess();
-    }
+  sched_timer_resume();
 #endif
 
   irqrestore(saved_state);