Fix-Point commented on PR #13984:
URL: https://github.com/apache/nuttx/pull/13984#issuecomment-2403727329

   I am using this test case to reproduce the bug.
   
   ```c
   #include <nuttx/nuttx.h>
   #include <nuttx/config.h>
   #include <stdio.h>
   
   #define read_sysreg(reg)                         \
     ({                                             \
       uint64_t __val;                              \
       __asm__ volatile ("mrs %0, " STRINGIFY(reg)  \
                       : "=r" (__val) :: "memory"); \
       __val;                                       \
     })
   
   static inline uint64_t arm64_arch_timer_count(void)
   {
     return read_sysreg(cntvct_el0);
   }
   
   static inline uint64_t arm64_arch_timer_get_cntfrq(void)
   {
     return read_sysreg(cntfrq_el0);
   }
   
   static inline uint64_t time_us(void)
   {
     return arm64_arch_timer_count() * 1000000 / arm64_arch_timer_get_cntfrq();
   }
   
   inline uint64_t minutes_in_ticks(uint64_t mins)
   {
     return mins * 60 * 1000000 / CONFIG_USEC_PER_TICK;
   }
   
   int main(int argc, FAR char *argv[])
   {
     sem_t sem;
     uint64_t t1;
     uint64_t t2;
     sem_init(&sem, 0, 0);
   
     for (int64_t i = 0; i < minutes_in_ticks(10); i++)
       {
         t1 = time_us();
         nxsem_tickwait(&sem, 1);
         t2 = time_us();
   
         if (i % 100 == 0)
           {
             printf("%lu %lu %lu\n",t1,t2,t2-t1);
           }
   
         if (t2 - t1 < CONFIG_USEC_PER_TICK)
           {
             printf("ERROR: slept %lu us instead of minimum %u\n", t2-t1, 
CONFIG_USEC_PER_TICK);
           }
   
         assert(t2-t1 > CONFIG_USEC_PER_TICK); 
       }
   
     return 0;
   }
   ```
   
   1. The problem that tickwait_uninterruptible may wake up one tick too early 
is caused by the clock drift. I tried the following scenarios:
   - Without this patch and delay + 1, bug reproduced.
   - With this patch, bug fixed.
   - Without https://github.com/apache/nuttx/pull/13674, bug reproduced.
   - Without https://github.com/apache/nuttx/pull/13674 but with this patch, 
bug fixed.
   
   2. The previous method of aligning down to ticks has a problem. If the 
current count obtained when setting the timer is aligned down and is 1 larger 
than the current system tick, it will still cause clock drift. The key is not 
to use the count obtained when setting the timer as a basis for delay, instead 
we should use current system tick.
   


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

Reply via email to