> 
> FWIW, my first X11 game I ever wrote, which was similar to the
> game "LodeRunner", used a select() timeout for the timing loop

select(),nanosleep(),poll(),etc all sleep one tick longer _except_
when previous syscall was interrupted by a signal or when input 
became available.

this is due to tvtohz ( sys/kern/kern_clock.c ) allways adding 1
to 'allow for the current tick to expire'.

this badly breaks some - maybe poor - code I have written which
hooks both i/o polling and timers ( set_fd, set_timer,kill_timer
in the style of javascript) on a select() loop.
(random timeouts would be better here!)

I'm not sure but this seems to affect also the Xkb ( I use to set
autorepeat to delay 10,repeat 120 ) when playing my tetris ...

why not testing for zero result instead of adding 1 in tvtohz ?
        if (ticks > INT_MAX)
                ticks = INT_MAX;
+       else if (ticks == 0)
+               ticks = 1;
        return ((int)ticks);

I modified kern_clock.c and other places which assume that tvtohz
adds 1 in 4.4 long time ago and recently in 5.0 and haven't seen
so far any harm at all.

I attached my trivial patch and a little test program.

Best regards
adi

notes:
tvtohz has been renamed to hzto in NetBSD but is the same.

of course you can write
  ticks =
    (sec * hz) + (((unsigned long)usec + (tick - 1)) / tick) ?: 1;
but this wouldn't work with other compiler than GCC ( does
FreeBSD compile with other compiler anyway ?)
/* select [msec] - display select() timeout inaccuracy as '[+-] sec usec' */

#include <sys/types.h>
#include <sys/time.h>
#include <sys/sysctl.h>
#include <unistd.h>
#include <errno.h>
#include <signal.h>
#include <time.h>
#include <err.h>
#include <stdio.h>

fd_set rdfs;
int ms;

void sig(int unused) {}

int main(int argc,char **argv)
{
        int k,mib[2];
        struct timeval tv,savtv,otv;
        struct clockinfo clock;
        char buf[256];
        signal(SIGQUIT,sig);
        if (argc > 1) ms = atoi(argv[1]);
        if (!ms) ms = 500;
        savtv.tv_usec = ms % 1000 * 1000; savtv.tv_sec = ms / 1000;
        mib[0] = CTL_KERN; mib[1] = KERN_CLOCKRATE;
        k = sizeof clock;
        if (sysctl(mib,2,&clock,&k,NULL,0)) err(1,"sysctl");
        printf("%d\n",clock.tick);
        savtv.tv_usec -= savtv.tv_usec % clock.tick;
redo:
        tv = savtv; gettimeofday(&otv,NULL);
        for(;;) {
                FD_SET(0,&rdfs);
                k = select(1,&rdfs,NULL,NULL,&tv);
                gettimeofday(&tv,NULL);
                timersub(&tv,&otv,&tv);
                switch(k) {
                case 0:
                        if (timercmp(&tv,&savtv,<))
                                { timersub(&savtv,&tv,&tv);k = '-'; }
                        else
                                { timersub(&tv,&savtv,&tv);k = '+'; }
                        printf("%c%4d s %10d us\n",k,tv.tv_sec,tv.tv_usec);
                        goto redo;
                case -1:
                        if (errno != EINTR) err(1,"select");
                        printf("\t--interrupt\n");
                        break;
                default:
                        read(0,buf,256);
                        printf("\t--input\n");
                        break;
                }
                timersub(&savtv,&tv,&tv);
                if (tv.tv_sec < 0) timerclear(&tv);
        }
}

/* use ^M for input and ^\ for interrupt
   unpatched kernel:
+   0 s       9938 us

        --input
+   0 s       9959 us
+   0 s       9972 us
+   0 s       9930 us
^\      --interrupt
+   0 s       9954 us
+   0 s       9954 us

   patched kernel:
-   0 s         44 us
^\      --interrupt
-   0 s          3 us
-   0 s        100 us

        --input
-   0 s         44 us
-   0 s         47 us
 (this on xterm,on console results are better)
*/
diff -Naur osrc/sys/kern/kern_clock.c src/sys/kern/kern_clock.c
--- osrc/sys/kern/kern_clock.c  Tue Nov 19 23:58:52 2002
+++ src/sys/kern/kern_clock.c   Mon Dec  2 23:47:36 2002
@@ -257,8 +257,7 @@
         * If the number of usecs in the whole seconds part of the time
         * difference fits in a long, then the total number of usecs will
         * fit in an unsigned long.  Compute the total and convert it to
-        * ticks, rounding up and adding 1 to allow for the current tick
-        * to expire.  Rounding also depends on unsigned long arithmetic
+        * ticks.  Rounding also depends on unsigned long arithmetic
         * to avoid overflow.
         *
         * Otherwise, if the number of ticks in the whole seconds part of
@@ -291,14 +290,15 @@
                ticks = 1;
        } else if (sec <= LONG_MAX / 1000000)
                ticks = (sec * 1000000 + (unsigned long)usec + (tick - 1))
-                       / tick + 1;
+                       / tick;
        else if (sec <= LONG_MAX / hz)
-               ticks = sec * hz
-                       + ((unsigned long)usec + (tick - 1)) / tick + 1;
+               ticks = sec * hz + ((unsigned long)usec + (tick - 1)) / tick;
        else
                ticks = LONG_MAX;
        if (ticks > INT_MAX)
                ticks = INT_MAX;
+       else if (ticks == 0)
+               ticks = 1;
        return ((int)ticks);
 }
 
diff -Naur osrc/sys/kern/kern_time.c src/sys/kern/kern_time.c
--- osrc/sys/kern/kern_time.c   Tue Dec  3 00:14:05 2002
+++ src/sys/kern/kern_time.c    Mon Dec  2 23:33:04 2002
@@ -527,10 +527,6 @@
  * Else compute next time timer should go off which is > current time.
  * This is where delay in processing this timeout causes multiple
  * SIGALRM calls to be compressed into one.
- * tvtohz() always adds 1 to allow for the time until the next clock
- * interrupt being strictly less than 1 clock tick, but we don't want
- * that here since we want to appear to be in sync with the clock
- * interrupt even when we're delayed.
  */
 void
 realitexpire(void *arg)
@@ -555,7 +551,7 @@
                if (timevalcmp(&p->p_realtimer.it_value, &ctv, >)) {
                        ntv = p->p_realtimer.it_value;
                        timevalsub(&ntv, &ctv);
-                       callout_reset(&p->p_itcallout, tvtohz(&ntv) - 1,
+                       callout_reset(&p->p_itcallout, tvtohz(&ntv),
                            realitexpire, p);
                        splx(s);
                        PROC_UNLOCK(p);
diff -Naur osrc/sys/kern/kern_timeout.c src/sys/kern/kern_timeout.c
--- osrc/sys/kern/kern_timeout.c        Thu Sep  5 14:42:03 2002
+++ src/sys/kern/kern_timeout.c Mon Dec  2 23:56:45 2002
@@ -399,15 +399,17 @@
                return;
        else if (time_change->tv_sec <= LONG_MAX / 1000000)
                delta_ticks = (time_change->tv_sec * 1000000 +
-                              time_change->tv_usec + (tick - 1)) / tick + 1;
+                              time_change->tv_usec + (tick - 1)) / tick;
        else if (time_change->tv_sec <= LONG_MAX / hz)
                delta_ticks = time_change->tv_sec * hz +
-                             (time_change->tv_usec + (tick - 1)) / tick + 1;
+                             (time_change->tv_usec + (tick - 1)) / tick;
        else
                delta_ticks = LONG_MAX;
 
        if (delta_ticks > INT_MAX)
                delta_ticks = INT_MAX;
+       else if (delta_ticks == 0)
+               delta_ticks = 1;
 
        /* 
         * Now rip through the timer calltodo list looking for timers
diff -Naur osrc/sys/net/bpf.c src/sys/net/bpf.c
--- osrc/sys/net/bpf.c  Tue Nov 19 23:58:58 2002
+++ src/sys/net/bpf.c   Mon Dec  2 23:58:24 2002
@@ -774,12 +774,8 @@
                {
                        struct timeval *tv = (struct timeval *)addr;
 
-                       /*
-                        * Subtract 1 tick from tvtohz() since this isn't
-                        * a one-shot timer.
-                        */
                        if ((error = itimerfix(tv)) == 0)
-                               d->bd_rtout = tvtohz(tv) - 1;
+                               d->bd_rtout = tvtohz(tv);
                        break;
                }
 

Reply via email to