Re: My newyear's resolution is...

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

    Ok, I've spent some time looking at your patch set.  I modified your
    tvtohz() changes to guarentee a minimum return value of 1 without
    the unconditional + 1 that was messing up the original calculation, which
    you fixed, as you suggested.  That patch is enclosed.  I cleaned up
    the rest of the function a bit too (not tested as yet).

    This simplifies your kern_time.c patch and maintains the original tvtohz()
    API, but there is still an issue with the last bit of your kern_time.c
    patch:

    +		if (tv.tv_sec == 0 && tv.tv_usec < tick)
    +			return (0);

    The problem is that this may break the guarentee that it sleeps at least
    as long as requested.  The original tvtohz() calculation only guarentees
    that the ticks calculation will be at least enough, not that it will be
    more then enough, but if the calculation occurs just before a clock
    interrupt then the '1 tick minimum' might only be a few microseconds.

    I think the reason this did not show up in your tests is because the
    tsleep() will synchronize with a clock tick anyway, so all your loops
    except the first one will be hitting nanosleep() just after a clock
    interrupt instead of just before.

    So if we calculate less then a tick left to go in the nanosleep loop,
    we still either have to tsleep for a tick, or hard-loop or soft switch
    until the timeout completes.  Soft switching is an option... it involves
    calling uio_yield() in a loop instead of tsleep()ing until the remainder
    of the timeout has been dealt with.  We could make nanosleep() extremely
    accurate in this fashion (at least as long as there are no other 
    cpu bound processes running) but it would waste a lot more cpu even
    with a uio_yield()

					-Matt
					Matthew Dillon 
					<dillon@xxxxxxxxxxxxx>

Index: kern_clock.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.12
diff -u -r1.12 kern_clock.c
--- kern_clock.c	17 Oct 2003 07:30:42 -0000	1.12
+++ kern_clock.c	7 Jan 2004 04:16:24 -0000
@@ -268,35 +268,23 @@
 }
 
 /*
- * Compute number of ticks in the specified amount of time.
+ * Compute number of ticks for the specified amount of time.  If the 
+ * representation overflows, return INT_MAX.  The minimum return value is 1
+ * tick.
+ *
+ * Note that limit checks must take into account microseconds, which is
+ * done simply by using the smaller signed long maximum instead of
+ * the unsigned long maximum.
+ *
+ * If ints have 32 bits, then the maximum value for any timeout in
+ * 10ms ticks is 248 days.
  */
 int
-tvtohz(tv)
-	struct timeval *tv;
+tvtohz(struct timeval *tv)
 {
-	unsigned long ticks;
+	int ticks;
 	long sec, usec;
 
-	/*
-	 * 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
-	 * to avoid overflow.
-	 *
-	 * Otherwise, if the number of ticks in the whole seconds part of
-	 * the time difference fits in a long, then convert the parts to
-	 * ticks separately and add, using similar rounding methods and
-	 * overflow avoidance.  This method would work in the previous
-	 * case but it is slightly slower and assumes that hz is integral.
-	 *
-	 * Otherwise, round the time difference down to the maximum
-	 * representable value.
-	 *
-	 * If ints have 32 bits, then the maximum value for any timeout in
-	 * 10ms ticks is 248 days.
-	 */
 	sec = tv->tv_sec;
 	usec = tv->tv_usec;
 	if (usec < 0) {
@@ -313,17 +301,14 @@
 		       sec, usec);
 #endif
 		ticks = 1;
-	} else if (sec <= LONG_MAX / 1000000)
-		ticks = (sec * 1000000 + (unsigned long)usec + (tick - 1))
-			/ tick + 1;
-	else if (sec <= LONG_MAX / hz)
-		ticks = sec * hz
-			+ ((unsigned long)usec + (tick - 1)) / tick + 1;
-	else
-		ticks = LONG_MAX;
-	if (ticks > INT_MAX)
+	} else if (sec <= INT_MAX / hz) {
+		ticks = (int)(sec * hz + ((u_long)usec + (tick - 1)) / tick);
+	} else {
 		ticks = INT_MAX;
-	return ((int)ticks);
+	}
+	if (ticks == 0)
+		ticks = 1;
+	return (ticks);
 }
 
 /*