[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5432: sched/wdog: fix delay calculation in wd_start()

[GitBox <gi...@apache.org>]

xiaoxiang781216 commented on a change in pull request #5432:
URL: https://github.com/apache/incubator-nuttx/pull/5432#discussion_r800438531



##########
File path: sched/wdog/wd_start.c
##########
@@ -74,6 +74,12 @@
 #  define CALL_FUNC(func, arg) func(arg)
 #endif
 
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static bool g_wdexpiration_context;

Review comment:
       Change to g_in_wd_expiration?

##########
File path: sched/wdog/wd_start.c
##########
@@ -204,15 +214,31 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
   up_getpicbase(&wdog->picbase);
   wdog->arg = arg;
 
-  /* Calculate delay+1, forcing the delay into a range that we can handle */
+  /* Delay for at least one tick should be performed */
 
-  if (delay <= 0)
+  if (delay == 0)
     {
       delay = 1;
     }
-  else if (++delay <= 0)
+  else
     {
-      delay--;
+      /* Calculate delay+1, forcing the delay into a range that we can
+       * handle.  In general wd_start() can be called at any time and we do
+       * not know is it at the beginning in the middle or at the end of a
+       * system clock phase, so we need to add 1 tick to ensure that the
+       * operation will be delayed for at least delay number of ticks.  But
+       * in a special case when wd_start() is called from a wd expiration
+       * callback that is called from a system tick interrupt handler we know
+       * that we are exactly at the beginning of a new phase so we do not
+       * need to add an extra tick in this case.
+       */
+
+      delay += g_wdexpiration_context ? 0 : 1;

Review comment:
       This approach has two issues:
   
   1. If some wdog callback run a bit long, the phase error will be large
   2. Can't fix all possible place(e.g. other interrupt happen near the timer interrupt)
   
   Since the accuracy of wdog is one tick, the change can't improve the accuracy smaller than one tick in general from the statistics perspective. But it improve in the most common case, I am fine with the change.




-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org