Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Dec 2002 00:40:24 +0200
From:      Enache Adrian <enache@rdslink.ro>
To:        freebsd-hackers@freebsd.org
Cc:        enache@rdslink.ro
Subject:   Re: [nephtes@openface.ca: [Xmame] Use of usleep() with -sleepidle]
Message-ID:  <20021204224024.GA3416@ratsnest.hole>
In-Reply-To: <3DEE50D2.39605F07@mindspring.com>
References:  <20021202151816.GJ83264@pcwin002.win.tue.nl> <20021202114019.R31106-100000@patrocles.silby.com> <20021204113154.GA205@pcwin002.win.tue.nl> <20021204114915.Q41338-100000@patrocles.silby.com> <3DEE50D2.39605F07@mindspring.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--mP3DRpeJDSE+ciuQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

> 
> 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 ?)

--mP3DRpeJDSE+ciuQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="select.c"

/* 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)
*/

--mP3DRpeJDSE+ciuQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=tvtohzpatch

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;
 		}
 

--mP3DRpeJDSE+ciuQ--

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021204224024.GA3416>