You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/01/26 12:28:01 UTC

[GitHub] [incubator-nuttx] shiliqinghuan opened a new issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

shiliqinghuan opened a new issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751


   I am testing timer module during porting nuttx. Then found that , for example, I use sigtimedwait to block 500ms,  actually takes 500ms + CONFIG_USEC_PER_TICK.
   
   In debugging ,  find that in wd_start() fuction, the block time increase 1, Is there a special reason?
   
   
    int wd_start(FAR struct wdog_s *wdog, int32_t delay,
                wdentry_t wdentry, wdparm_t arg)
   {
     FAR struct wdog_s *curr;
     FAR struct wdog_s *prev;
     FAR struct wdog_s *next;
     int32_t now;
     irqstate_t flags;
   
     /* Verify the wdog and setup parameters */
   
     if (wdog == NULL || delay < 0)
       {
         return -EINVAL;
       }
   
     /* Check if the watchdog has been started. If so, stop it.
      * NOTE:  There is a race condition here... the caller may receive
      * the watchdog between the time that wd_start is called and
      * the critical section is established.
      */
   
     flags = enter_critical_section();
     if (WDOG_ISACTIVE(wdog))
       {
         wd_cancel(wdog);
       }
   
     /* Save the data in the watchdog structure */
   
     wdog->func = wdentry;         /* Function to execute when delay expires */
     up_getpicbase(&wdog->picbase);
     wdog->arg = arg;
   
     /* Calculate delay+1, forcing the delay into a range that we can handle */
   
     if (delay <= 0)
       {
         delay = 1;
       }
     else if (++delay <= 0)
       {
         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).
      */
   
     nxsched_cancel_timer();
   #endif
   
     /* Do the easy case first -- when the watchdog timer queue is empty. */
   
     if (g_wdactivelist.head == NULL)
       {
   #ifdef CONFIG_SCHED_TICKLESS
         /* Update clock tickbase */
   
         g_wdtickbase = clock_systime_ticks();
   #endif
   
         /* Add the watchdog to the head == tail of the queue. */
   
         sq_addlast((FAR sq_entry_t *)wdog, &g_wdactivelist);
       }
   
     /* There are other active watchdogs in the timer queue */
   
     else
       {
         now = 0;
         prev = curr = (FAR struct wdog_s *)g_wdactivelist.head;
   
         /* Advance to positive time */
   
         while ((now += curr->lag) < 0 && curr->next)
           {
             prev = curr;
             curr = curr->next;
           }
   
         /* Advance past shorter delays */
   
         while (now <= delay && curr->next)
           {
             prev = curr;
             curr = curr->next;
             now += curr->lag;
           }
   
         /* Check if the new wdog must be inserted before the curr. */
   
         if (delay < now)
           {
             /* The relative delay time is smaller or equal to the current delay
              * time, so decrement the current delay time by the new relative
              * delay time.
              */
   
             delay -= (now - curr->lag);
             curr->lag -= delay;
   
             /* Insert the new watchdog in the list */
   
             if (curr == (FAR struct wdog_s *)g_wdactivelist.head)
               {
                 /* Insert the watchdog at the head of the list */
   
                 sq_addfirst((FAR sq_entry_t *)wdog, &g_wdactivelist);
               }
             else
               {
                 /* Insert the watchdog in mid- or end-of-queue */
   
                 sq_addafter((FAR sq_entry_t *)prev, (FAR sq_entry_t *)wdog,
                             &g_wdactivelist);
               }
           }
   
         /* The new watchdog delay time is greater than the curr delay time,
          * so the new wdog must be inserted after the curr. This only occurs
          * if the wdog is to be added to the end of the list.
          */
   
         else
           {
             delay -= now;
             if (!curr->next)
               {
                 sq_addlast((FAR sq_entry_t *)wdog, &g_wdactivelist);
               }
             else
               {
                 next = curr->next;
                 next->lag -= delay;
                 sq_addafter((FAR sq_entry_t *)curr, (FAR sq_entry_t *)wdog,
                             &g_wdactivelist);
               }
           }
       }
   
     /* Put the lag into the watchdog structure and mark it as active. */
   
     wdog->lag = delay;
     WDOG_SETACTIVE(wdog);
   
   #ifdef CONFIG_SCHED_TICKLESS
     /* 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.
      */
   
     nxsched_resume_timer();
   #endif
   
     leave_critical_section(flags);
     return OK;
   }
   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751#issuecomment-768178375


   @shiliqinghuan it should be fine to remove the extra tick,  could you provide a patch to fix it?


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] patacongo edited a comment on issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751#issuecomment-768321262


   What the op is seeing here is not normally timer behavior.  It is a consequence of running the setup up the timer consistent in phase the system time and, hence, experience the maximum error every time.  If the timer is set up a random phase with respect to the system timer, then removing the +1 would introduce serious, subtle errors.  The +1 was added to fix those errors.
   
   For example, suppose the system timer has a 10MS period and you request the a 10MS delay like usleep(10*1000) near the system timer period.  That would result in no delay and a complete failure of the timing if +1 were not added to delay ticks.  That could result in very bad consequences.
   
   Waiting a little too long, on the other hander, is never an error.  That will happen normally in real time system due to interruptions, prioritization, etc.  However, waiting no as long as requested is just an error.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] shiliqinghuan closed issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
shiliqinghuan closed issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751


   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] patacongo commented on issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751#issuecomment-768285491


   > 
   > 
   > @shiliqinghuan it should be fine to remove the extra tick, could you provide a patch to fix it?
   
   No, that we be logic error.  You should not remove the extra tick.  If you remove it, then you will have occasional waits that are shorter that the requested wait.  That is unacceptable behavior for the API.  Again, please read https://cwiki.apache.org/confluence/display/NUTTX/Short+Time+Delays


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] patacongo commented on issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751#issuecomment-768321262


   What the op is seeing here is not normally timer behavior.  It is a consequence of running the setup up the timer consistent in phase the system time and, hence, experience the maximum error every time.  If the timer is set up a random phase with respect to the system timer, then removing the +1 would introduce serious, subtle errors.  The +1 was added to fix those errors.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] shiliqinghuan commented on issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
shiliqinghuan commented on issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751#issuecomment-769699460


   Well, thank you!


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] patacongo edited a comment on issue #2751: the real wait time of sigtimedwait() more than specified time one CONFIG_USEC_PER_TICK

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #2751:
URL: https://github.com/apache/incubator-nuttx/issues/2751#issuecomment-768321262


   What the op is seeing here is not normally timer behavior.  It is a consequence of running the setup up the timer consistent in phase the system time and, hence, experience the maximum error every time.  If the timer is set up a random phase with respect to the system timer, then removing the +1 would introduce serious, subtle errors.  The +1 was added to fix those errors.
   
   For example, suppose the system timer has a 10MS period and you request the a 10MS delay like usleep(10*1000) near the system timer period.  That would result in no delay and a complete failure of the timing if +1 were not added to delay ticks.  That could result in very bad consequences.
   
   Waiting a little too long, on the other hander, is never an error.  That will happen normally in real time system due to interruptions, prioritization, etc.  However, waiting no as long as requested is just an error, always.


----------------------------------------------------------------
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.

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