From: David van Moolenbroek <david@minix3.org>
Date: Thu, 21 Jul 2016 15:31:27 +0000 (+0000)
Subject: Various timer improvements
X-Git-Url: http://zhaoyanbai.com/repos/dig.html?a=commitdiff_plain;h=cfd712b4245f67a5631cc14e950ce43b18455602;p=minix.git

Various timer improvements

Now that clock_t is an unsigned value, we can also allow the system
uptime to wrap.  Essentially, instead of using (a <= b) to see if time
a occurs no later than time b, we use (b - a <= CLOCK_MAX / 2).  The
latter value does not exist, so instead we add TMRDIFF_MAX for that
purpose.

We must therefore also avoid using values like 0 and LONG_MAX as
special values for absolute times.  This patch extends the libtimers
interface so that it no longer uses 0 to indicate "no timeout".
Similarly, TMR_NEVER is now used as special value only when
otherwise a relative time difference would be used.  A minix_timer
structure is now considered in use when it has a watchdog function set,
rather than when the absolute expiry time is not TMR_NEVER.  A few new
macros in <minix/timers.h> help with timer comparison and obtaining
properties from a minix_timer structure.

This patch also eliminates the union of timer arguments, instead using
the only union element that is only used (the integer).  This prevents
potential problems with e.g. live update.  The watchdog function
prototype is changed to pass in the argument value rather than a
pointer to the timer structure, since obtaining the argument value was
the only current use of the timer structure anyway.  The result is a
somewhat friendlier timers API.

The VFS select code required a few more invasive changes to restrict
the timer value to the new maximum, effectively matching the timer
code in PM.  As a side effect, select(2) has been changed to reject
invalid timeout values.  That required a change to the test set, which
relied on the previous, erroneous behavior.

Finally, while we're rewriting significant chunks of the timer code
anyway, also covert it to KNF and add a few more explanatory comments.

Change-Id: Id43165c3fbb140b32b90be2cca7f68dd646ea72e
---

diff --git a/minix/drivers/hid/pckbd/pckbd.c b/minix/drivers/hid/pckbd/pckbd.c
index 236cef088..ca56657d2 100644
--- a/minix/drivers/hid/pckbd/pckbd.c
+++ b/minix/drivers/hid/pckbd/pckbd.c
@@ -44,7 +44,7 @@ static struct inputdriver pckbd_tab = {
  * The watchdog timer function, implementing all but the actual reset.
  */
 static void
-kbd_watchdog(minix_timer_t *UNUSED(tmrp))
+kbd_watchdog(int arg __unused)
 {
 	kbd_watchdog_set = 0;
 	if (!kbdout.avail)
diff --git a/minix/drivers/storage/ahci/ahci.c b/minix/drivers/storage/ahci/ahci.c
index 2b148be2f..e4d2938a1 100644
--- a/minix/drivers/storage/ahci/ahci.c
+++ b/minix/drivers/storage/ahci/ahci.c
@@ -232,7 +232,7 @@ static void port_set_cmd(struct port_state *ps, int cmd, cmd_fis_t *fis,
 	u8_t packet[ATAPI_PACKET_SIZE], prd_t *prdt, int nr_prds, int write);
 static void port_issue(struct port_state *ps, int cmd, clock_t timeout);
 static int port_exec(struct port_state *ps, int cmd, clock_t timeout);
-static void port_timeout(minix_timer_t *tp);
+static void port_timeout(int arg);
 static void port_disconnect(struct port_state *ps);
 
 static char *ahci_portname(struct port_state *ps);
@@ -1712,7 +1712,7 @@ static void port_intr(struct port_state *ps)
 /*===========================================================================*
  *				port_timeout				     *
  *===========================================================================*/
-static void port_timeout(minix_timer_t *tp)
+static void port_timeout(int arg)
 {
 	/* A timeout has occurred on this port. Figure out what the timeout is
 	 * for, and take appropriate action.
@@ -1720,8 +1720,8 @@ static void port_timeout(minix_timer_t *tp)
 	struct port_state *ps;
 	int port, cmd;
 
-	port = GET_PORT(tmr_arg(tp)->ta_int);
-	cmd = GET_TAG(tmr_arg(tp)->ta_int);
+	port = GET_PORT(arg);
+	cmd = GET_TAG(arg);
 
 	assert(port >= 0 && port < hba_state.nr_ports);
 
diff --git a/minix/drivers/storage/floppy/floppy.c b/minix/drivers/storage/floppy/floppy.c
index d55427bcc..fbdb2cc22 100644
--- a/minix/drivers/storage/floppy/floppy.c
+++ b/minix/drivers/storage/floppy/floppy.c
@@ -239,8 +239,8 @@ static u8_t f_results[MAX_RESULTS];/* the controller can give lots of output */
 static minix_timer_t f_tmr_timeout;		/* timer for various timeouts */
 static u32_t system_hz;		/* system clock frequency */
 static void f_expire_tmrs(clock_t stamp);
-static void stop_motor(minix_timer_t *tp);
-static void f_timeout(minix_timer_t *tp);
+static void stop_motor(int arg);
+static void f_timeout(int arg);
 
 static struct device *f_prepare(devminor_t device);
 static struct device *f_part(devminor_t minor);
@@ -784,13 +784,13 @@ static void start_motor(void)
 /*===========================================================================*
  *				stop_motor				     *
  *===========================================================================*/
-static void stop_motor(minix_timer_t *tp)
+static void stop_motor(int arg)
 {
 /* This routine is called from an alarm timer after several seconds have
  * elapsed with no floppy disk activity.  It turns the drive motor off.
  */
   int s;
-  motor_status &= ~(1 << tmr_arg(tp)->ta_int);
+  motor_status &= ~(1 << arg);
   if ((s=sys_outb(DOR, (motor_status << MOTOR_SHIFT) | ENABLE_INT)) != OK)
 	panic("Sys_outb in stop_motor() failed: %d", s);
 }
@@ -1198,7 +1198,7 @@ static int f_intr_wait(void)
 /*===========================================================================*
  *				f_timeout				     *
  *===========================================================================*/
-static void f_timeout(minix_timer_t *UNUSED(tp))
+static void f_timeout(int arg __unused)
 {
 /* This routine is called when a timer expires.  Usually to tell that a
  * motor has spun up, but also to forge an interrupt when it takes too long
diff --git a/minix/drivers/tty/pty/tty.c b/minix/drivers/tty/pty/tty.c
index a2a5924f2..151bcf086 100644
--- a/minix/drivers/tty/pty/tty.c
+++ b/minix/drivers/tty/pty/tty.c
@@ -23,7 +23,7 @@
 /* A device exists if at least its 'devread' function is defined. */
 #define tty_active(tp)	((tp)->tty_devread != NULL)
 
-static void tty_timed_out(minix_timer_t *tp);
+static void tty_timed_out(int arg);
 static void settimer(tty_t *tty_ptr, int enable);
 static void in_transfer(tty_t *tp);
 static int tty_echo(tty_t *tp, int ch);
@@ -1290,11 +1290,11 @@ static int tty_init(int UNUSED(type), sef_init_info_t *UNUSED(info))
 /*===========================================================================*
  *				tty_timed_out				     *
  *===========================================================================*/
-static void tty_timed_out(minix_timer_t *tp)
+static void tty_timed_out(int arg)
 {
 /* This timer has expired. Set the events flag, to force processing. */
   tty_t *tty_ptr;
-  tty_ptr = &tty_table[tmr_arg(tp)->ta_int];
+  tty_ptr = &tty_table[arg];
   tty_ptr->tty_min = 0;			/* force read to succeed */
   tty_ptr->tty_events = 1;		
 }
diff --git a/minix/drivers/tty/tty/arch/i386/console.c b/minix/drivers/tty/tty/arch/i386/console.c
index b2b773c09..5f48c2b79 100644
--- a/minix/drivers/tty/tty/arch/i386/console.c
+++ b/minix/drivers/tty/tty/arch/i386/console.c
@@ -128,7 +128,7 @@ static void flush(console_t *cons);
 static void parse_escape(console_t *cons, int c);
 static void scroll_screen(console_t *cons, int dir);
 static void set_6845(int reg, unsigned val);
-static void stop_beep(minix_timer_t *tmrp);
+static void stop_beep(int arg);
 static void cons_org0(void);
 static void disable_console(void);
 static void reenable_console(void);
@@ -895,7 +895,7 @@ clock_t dur;
 /*===========================================================================*
  *				stop_beep				     *
  *===========================================================================*/
-static void stop_beep(minix_timer_t *UNUSED(tmrp))
+static void stop_beep(int arg __unused)
 {
 /* Turn off the beeper by turning off bits 0 and 1 in PORT_B. */
   u32_t port_b_val;
diff --git a/minix/drivers/tty/tty/tty.c b/minix/drivers/tty/tty/tty.c
index d017af6eb..dc37c48a3 100644
--- a/minix/drivers/tty/tty/tty.c
+++ b/minix/drivers/tty/tty/tty.c
@@ -59,7 +59,7 @@ unsigned long rs_irq_set = 0;
 
 struct kmessages kmess;
 
-static void tty_timed_out(minix_timer_t *tp);
+static void tty_timed_out(int arg);
 static void settimer(tty_t *tty_ptr, int enable);
 static void in_transfer(tty_t *tp);
 static int tty_echo(tty_t *tp, int ch);
@@ -1573,11 +1573,11 @@ static void tty_init()
 /*===========================================================================*
  *				tty_timed_out				     *
  *===========================================================================*/
-static void tty_timed_out(minix_timer_t *tp)
+static void tty_timed_out(int arg)
 {
 /* This timer has expired. Set the events flag, to force processing. */
   tty_t *tty_ptr;
-  tty_ptr = &tty_table[tmr_arg(tp)->ta_int];
+  tty_ptr = &tty_table[arg];
   tty_ptr->tty_min = 0;			/* force read to succeed */
   tty_ptr->tty_events = 1;		
 }
diff --git a/minix/include/minix/timers.h b/minix/include/minix/timers.h
index 82f162fda..cfe921ea6 100644
--- a/minix/include/minix/timers.h
+++ b/minix/include/minix/timers.h
@@ -17,50 +17,61 @@
 #include <limits.h>
 
 #include <sys/types.h>
+#include <minix/const.h>
 #include <minix/u64.h>
 #include <minix/minlib.h>
 #include <minix/endpoint.h>
 
-struct minix_timer;
-typedef void (*tmr_func_t)(struct minix_timer *tp);
-typedef union { int ta_int; long ta_long; void *ta_ptr; } ixfer_tmr_arg_t;
+typedef void (*tmr_func_t)(int arg);
 
 /* A minix_timer_t variable must be declare for each distinct timer to be used.
  * The timers watchdog function and expiration time are automatically set
- * by the library function tmrs_settimer, but its argument is not.
+ * by the library function tmrs_settimer, but its argument is not.  In general,
+ * the timer is in use when it has a callback function.
  */
 typedef struct minix_timer
 {
   struct minix_timer	*tmr_next;	/* next in a timer chain */
-  clock_t 	tmr_exp_time;	/* expiration time */
-  tmr_func_t	tmr_func;	/* function to call when expired */
-  ixfer_tmr_arg_t tmr_arg;	/* random argument */
+  clock_t 		tmr_exp_time;	/* expiration time (absolute) */
+  tmr_func_t		tmr_func;	/* function to call when expired */
+  int			tmr_arg;	/* integer argument */
 } minix_timer_t;
 
-/* Used when the timer is not active. */
-#define TMR_NEVER    ((clock_t) -1 < 0) ? ((clock_t) LONG_MAX) : ((clock_t) -1)
-#undef TMR_NEVER
-#define TMR_NEVER	((clock_t) LONG_MAX)
+/*
+ * Clock times may wrap.  Thus, we must only ever compare relative times, which
+ * means they must be no more than half the total maximum time value apart.
+ * The clock_t type is unsigned (int or long), thus we take half that.
+ */
+#define TMRDIFF_MAX		(INT_MAX)
+
+/* This value must be used only instead of a timer difference value. */
+#define TMR_NEVER		((clock_t)TMRDIFF_MAX + 1)
 
 /* These definitions can be used to set or get data from a timer variable. */
-#define tmr_arg(tp) (&(tp)->tmr_arg)
-#define tmr_exp_time(tp) (&(tp)->tmr_exp_time)
+#define tmr_exp_time(tp)	((tp)->tmr_exp_time)
+#define tmr_is_set(tp)		((tp)->tmr_func != NULL)
+/*
+ * tmr_is_first() returns TRUE iff the first given absolute time is sooner than
+ * or equal to the second given time.
+ */
+#define tmr_is_first(a,b)	((clock_t)(b) - (clock_t)(a) <= TMRDIFF_MAX)
+#define tmr_has_expired(tp,now)	tmr_is_first((tp)->tmr_exp_time, (now))
 
 /* Timers should be initialized once before they are being used. Be careful
  * not to reinitialize a timer that is in a list of timers, or the chain
  * will be broken.
  */
-#define tmr_inittimer(tp) (void)((tp)->tmr_exp_time = TMR_NEVER, \
-	(tp)->tmr_next = NULL)
+#define tmr_inittimer(tp) (void)((tp)->tmr_func = NULL, (tp)->tmr_next = NULL)
 
 /* The following generic timer management functions are available. They
  * can be used to operate on the lists of timers. Adding a timer to a list
  * automatically takes care of removing it.
  */
-clock_t tmrs_clrtimer(minix_timer_t **tmrs, minix_timer_t *tp, clock_t *new_head);
-void tmrs_exptimers(minix_timer_t **tmrs, clock_t now, clock_t *new_head);
-clock_t tmrs_settimer(minix_timer_t **tmrs, minix_timer_t *tp, clock_t exp_time,
-	tmr_func_t watchdog, clock_t *new_head);
+int tmrs_settimer(minix_timer_t **tmrs, minix_timer_t *tp, clock_t exp_time,
+	tmr_func_t watchdog, int arg, clock_t *old_head, clock_t *new_head);
+int tmrs_clrtimer(minix_timer_t **tmrs, minix_timer_t *tp, clock_t *old_head,
+	clock_t *new_head);
+int tmrs_exptimers(minix_timer_t **tmrs, clock_t now, clock_t *new_head);
 
 #define PRINT_STATS(cum_spenttime, cum_instances) {		\
 		if(ex64hi(cum_spenttime)) { util_stacktrace(); printf(" ( ??? %lu %lu)\n",	\
@@ -110,7 +121,7 @@ clock_t tmrs_settimer(minix_timer_t **tmrs, minix_timer_t *tp, clock_t exp_time,
  */
 
 void init_timer(minix_timer_t *tp);
-void set_timer(minix_timer_t *tp, int ticks, tmr_func_t watchdog, int arg);
+void set_timer(minix_timer_t *tp, clock_t ticks, tmr_func_t watchdog, int arg);
 void cancel_timer(minix_timer_t *tp);
 void expire_timers(clock_t now);
 
diff --git a/minix/kernel/arch/earm/pre_init.c b/minix/kernel/arch/earm/pre_init.c
index e52efb91b..445f6acd5 100644
--- a/minix/kernel/arch/earm/pre_init.c
+++ b/minix/kernel/arch/earm/pre_init.c
@@ -418,7 +418,7 @@ kinfo_t *pre_init(int argc, char **argv)
  * longer used and the "real" implementations are visible
  */
 void send_diag_sig(void) { }
-void minix_shutdown(minix_timer_t *t) { arch_shutdown(0); }
+void minix_shutdown(int how) { arch_shutdown(how); }
 void busy_delay_ms(int x) { }
 int raise(int n) { panic("raise(%d)\n", n); }
 int kern_phys_map_ptr( phys_bytes base_address, vir_bytes io_size, int vm_flags,
diff --git a/minix/kernel/arch/i386/arch_system.c b/minix/kernel/arch/i386/arch_system.c
index a3bd73daa..d8709f7fb 100644
--- a/minix/kernel/arch/i386/arch_system.c
+++ b/minix/kernel/arch/i386/arch_system.c
@@ -371,7 +371,7 @@ static void ser_debug(const int c)
 	switch(c)
 	{
 	case 'Q':
-		minix_shutdown(NULL);
+		minix_shutdown(0);
 		NOT_REACHABLE;
 #ifdef CONFIG_SMP
 	case 'B':
diff --git a/minix/kernel/arch/i386/pre_init.c b/minix/kernel/arch/i386/pre_init.c
index f71739f7d..9a0e85dd5 100644
--- a/minix/kernel/arch/i386/pre_init.c
+++ b/minix/kernel/arch/i386/pre_init.c
@@ -246,6 +246,6 @@ kinfo_t *pre_init(u32_t magic, u32_t ebx)
 }
 
 void send_diag_sig(void) { }
-void minix_shutdown(minix_timer_t *t) { arch_shutdown(0); }
+void minix_shutdown(int how) { arch_shutdown(how); }
 void busy_delay_ms(int x) { }
 int raise(int sig) { panic("raise(%d)\n", sig); }
diff --git a/minix/kernel/clock.c b/minix/kernel/clock.c
index f627ccb29..30ab57d97 100644
--- a/minix/kernel/clock.c
+++ b/minix/kernel/clock.c
@@ -36,7 +36,6 @@ static void load_update(void);
  * When a timer expires its watchdog function is run by the CLOCK task. 
  */
 static minix_timer_t *clock_timers;	/* queue of CLOCK timers */
-static clock_t next_timeout;	/* monotonic time that next timer expires */
 
 /* Number of ticks to adjust realtime by. A negative value implies slowing
  * down realtime, a positive value implies speeding it up.
@@ -154,12 +153,14 @@ int timer_int_handler(void)
 	load_update();
 
 	if (cpu_is_bsp(cpuid)) {
-		/* if a timer expired, notify the clock task */
-		if ((next_timeout <= kclockinfo.uptime)) {
+		/*
+		 * If a timer expired, notify the clock task.  Keep in mind
+		 * that clock tick values may overflow, so we must only look at
+		 * relative differences, and only if there are timers at all.
+		 */
+		if (clock_timers != NULL &&
+		    tmr_has_expired(clock_timers, kclockinfo.uptime))
 			tmrs_exptimers(&clock_timers, kclockinfo.uptime, NULL);
-			next_timeout = (clock_timers == NULL) ?
-				TMR_NEVER : clock_timers->tmr_exp_time;
-		}
 
 #ifdef DEBUG_SERIAL
 		if (kinfo.do_serial_debug)
@@ -230,14 +231,14 @@ time_t get_boottime(void)
 void set_kernel_timer(
   minix_timer_t *tp,			/* pointer to timer structure */
   clock_t exp_time,			/* expiration monotonic time */
-  tmr_func_t watchdog			/* watchdog to be called */
+  tmr_func_t watchdog,			/* watchdog to be called */
+  int arg				/* argument for watchdog function */
 )
 {
 /* Insert the new timer in the active timers list. Always update the 
  * next timeout time by setting it to the front of the active list.
  */
-  tmrs_settimer(&clock_timers, tp, exp_time, watchdog, NULL);
-  next_timeout = clock_timers->tmr_exp_time;
+  (void)tmrs_settimer(&clock_timers, tp, exp_time, watchdog, arg, NULL, NULL);
 }
 
 /*===========================================================================*
@@ -251,9 +252,8 @@ void reset_kernel_timer(
  * active and expired lists. Always update the next timeout time by setting
  * it to the front of the active list.
  */
-  tmrs_clrtimer(&clock_timers, tp, NULL);
-  next_timeout = (clock_timers == NULL) ? 
-	TMR_NEVER : clock_timers->tmr_exp_time;
+  if (tmr_is_set(tp))
+	(void)tmrs_clrtimer(&clock_timers, tp, NULL, NULL);
 }
 
 /*===========================================================================*
diff --git a/minix/kernel/main.c b/minix/kernel/main.c
index d98f10056..546583a0e 100644
--- a/minix/kernel/main.c
+++ b/minix/kernel/main.c
@@ -361,19 +361,18 @@ void prepare_shutdown(const int how)
    * argument passes the shutdown status. 
    */
   printf("MINIX will now be shut down ...\n");
-  tmr_arg(&shutdown_timer)->ta_int = how;
-  set_kernel_timer(&shutdown_timer, get_monotonic() + system_hz, minix_shutdown);
+  set_kernel_timer(&shutdown_timer, get_monotonic() + system_hz,
+      minix_shutdown, how);
 }
 
 /*===========================================================================*
  *				shutdown 				     *
  *===========================================================================*/
-void minix_shutdown(minix_timer_t *tp)
+void minix_shutdown(int how)
 {
 /* This function is called from prepare_shutdown or stop_sequence to bring 
  * down MINIX.
  */
-  int how;
 
 #ifdef CONFIG_SMP
   /* 
@@ -389,8 +388,6 @@ void minix_shutdown(minix_timer_t *tp)
   hw_intr_disable_all();
   stop_local_timer();
 
-  how = tp ? tmr_arg(tp)->ta_int : 0;
-
   /* Show shutdown message */
   direct_cls();
   if((how & RB_POWERDOWN) == RB_POWERDOWN)
diff --git a/minix/kernel/proto.h b/minix/kernel/proto.h
index 44c99e8b4..f8b8dbed1 100644
--- a/minix/kernel/proto.h
+++ b/minix/kernel/proto.h
@@ -23,7 +23,7 @@ void set_adjtime_delta(int32_t);
 clock_t get_monotonic(void);
 void set_boottime(time_t);
 time_t get_boottime(void);
-void set_kernel_timer(minix_timer_t *tp, clock_t t, tmr_func_t f);
+void set_kernel_timer(minix_timer_t *tp, clock_t t, tmr_func_t f, int arg);
 void reset_kernel_timer(minix_timer_t *tp);
 void ser_dump_proc(void);
 
@@ -51,7 +51,7 @@ void fpu_sigcontext(struct proc *, struct sigframe_sigcontext *fr, struct
 #endif
 void kmain(kinfo_t *cbi);
 void prepare_shutdown(int how);
-__dead void minix_shutdown(minix_timer_t *tp);
+__dead void minix_shutdown(int how);
 void bsp_finish_booting(void);
 
 /* proc.c */
diff --git a/minix/kernel/system/do_setalarm.c b/minix/kernel/system/do_setalarm.c
index 1cb1dc754..fe0b30a0e 100644
--- a/minix/kernel/system/do_setalarm.c
+++ b/minix/kernel/system/do_setalarm.c
@@ -14,7 +14,7 @@
 
 #if USE_SETALARM
 
-static void cause_alarm(minix_timer_t *tp);
+static void cause_alarm(int proc_nr_e);
 
 /*===========================================================================*
  *				do_setalarm				     *
@@ -28,34 +28,37 @@ int do_setalarm(struct proc * caller, message * m_ptr)
   clock_t uptime;		/* placeholder for current uptime */
 
   /* Extract shared parameters from the request message. */
-  exp_time = m_ptr->m_lsys_krn_sys_setalarm.exp_time;		/* alarm's expiration time */
-  use_abs_time = m_ptr->m_lsys_krn_sys_setalarm.abs_time;	/* flag for absolute time */
+  exp_time = m_ptr->m_lsys_krn_sys_setalarm.exp_time;
+  use_abs_time = m_ptr->m_lsys_krn_sys_setalarm.abs_time;
   if (! (priv(caller)->s_flags & SYS_PROC)) return(EPERM);
 
   /* Get the timer structure and set the parameters for this alarm. */
   tp = &(priv(caller)->s_alarm_timer);
-  tmr_arg(tp)->ta_int = caller->p_endpoint;
-  tp->tmr_func = cause_alarm; 
 
   /* Return the ticks left on the previous alarm. */
   uptime = get_monotonic(); 
-  if (tp->tmr_exp_time == TMR_NEVER) {
-      m_ptr->m_lsys_krn_sys_setalarm.time_left = TMR_NEVER;
-  } else if (uptime < tp->tmr_exp_time) {
-      m_ptr->m_lsys_krn_sys_setalarm.time_left = (tp->tmr_exp_time - uptime);
+  if (!tmr_is_set(tp)) {
+	m_ptr->m_lsys_krn_sys_setalarm.time_left = TMR_NEVER;
+  } else if (tmr_is_first(uptime, tp->tmr_exp_time)) {
+	m_ptr->m_lsys_krn_sys_setalarm.time_left = tp->tmr_exp_time - uptime;
   } else {
-      m_ptr->m_lsys_krn_sys_setalarm.time_left = 0;
+	m_ptr->m_lsys_krn_sys_setalarm.time_left = 0;
   }
 
   /* For the caller's convenience, also return the current time. */
   m_ptr->m_lsys_krn_sys_setalarm.uptime = uptime;
 
-  /* Finally, (re)set the timer depending on the expiration time. */
-  if (exp_time == 0) {
-      reset_kernel_timer(tp);
+  /*
+   * Finally, (re)set the timer depending on the expiration time.  Note that
+   * an absolute time of zero is as valid as any other absolute value, so only
+   * a relative time value of zero resets the timer.
+   */
+  if (!use_abs_time && exp_time == 0) {
+	reset_kernel_timer(tp);
   } else {
-      tp->tmr_exp_time = (use_abs_time) ? exp_time : exp_time + get_monotonic();
-      set_kernel_timer(tp, tp->tmr_exp_time, tp->tmr_func);
+	if (!use_abs_time)
+		exp_time += uptime;
+	set_kernel_timer(tp, exp_time, cause_alarm, caller->p_endpoint);
   }
   return(OK);
 }
@@ -63,13 +66,12 @@ int do_setalarm(struct proc * caller, message * m_ptr)
 /*===========================================================================*
  *				cause_alarm				     *
  *===========================================================================*/
-static void cause_alarm(minix_timer_t *tp)
+static void cause_alarm(int proc_nr_e)
 {
 /* Routine called if a timer goes off and the process requested a synchronous
- * alarm. The process number is stored in timer argument 'ta_int'. Notify that
+ * alarm. The process number is stored as the timer argument. Notify that
  * process with a notification message from CLOCK.
  */
-  endpoint_t proc_nr_e = tmr_arg(tp)->ta_int;	/* get process number */
   mini_notify(proc_addr(CLOCK), proc_nr_e);	/* notify process */
 }
 
diff --git a/minix/kernel/utility.c b/minix/kernel/utility.c
index 9417e0c77..de67e2dce 100644
--- a/minix/kernel/utility.c
+++ b/minix/kernel/utility.c
@@ -45,7 +45,7 @@ void panic(const char *fmt, ...)
 #endif
 
   /* Abort MINIX. */
-  minix_shutdown(NULL);
+  minix_shutdown(0);
 }
 
 /*===========================================================================*
diff --git a/minix/lib/libsys/sys_setalarm.c b/minix/lib/libsys/sys_setalarm.c
index 7541b9f0d..f3ba0bf98 100644
--- a/minix/lib/libsys/sys_setalarm.c
+++ b/minix/lib/libsys/sys_setalarm.c
@@ -2,8 +2,11 @@
 
 /*
  * Ask the kernel to schedule a synchronous alarm for the caller, using either
- * an absolute or a relative number of clock ticks.  Optionally return the time
- * left on the previous timer (TMR_NEVER if none was set) and the current time.
+ * an absolute or a relative number of clock ticks.  The new alarm replaces any
+ * previously set alarm.  If a relative expiry time of zero is given, the
+ * current alarm is stopped.  Return OK or a negative error code.  On success,
+ * optionally return the time left on the previous timer (TMR_NEVER if none was
+ * set) and the current time.
  */
 int
 sys_setalarm2(clock_t exp_time, int abs_time, clock_t * time_left,
diff --git a/minix/lib/libsys/timers.c b/minix/lib/libsys/timers.c
index c66f485f9..d403bf0c6 100644
--- a/minix/lib/libsys/timers.c
+++ b/minix/lib/libsys/timers.c
@@ -1,4 +1,5 @@
-/* Watchdog timer management. These functions in this file provide a
+/*
+ * Watchdog timer management. These functions in this file provide a
  * convenient interface to the timers library that manages a list of
  * watchdog timers. All details of scheduling an alarm at the CLOCK task
  * are hidden behind this interface.
@@ -16,71 +17,101 @@
 #include <minix/sysutil.h>
 
 static minix_timer_t *timers = NULL;
-static int expiring = 0;
+static int expiring = FALSE;
 
-/*===========================================================================*
- *                              init_timer                                   *
- *===========================================================================*/
-void init_timer(minix_timer_t *tp)
+/*
+ * Initialize the timer 'tp'.
+ */
+void
+init_timer(minix_timer_t * tp)
 {
-        tmr_inittimer(tp);
+
+	tmr_inittimer(tp);
 }
 
-/*===========================================================================*
- *                              set_timer                                    *
- *===========================================================================*/
-void set_timer(minix_timer_t *tp, int ticks, tmr_func_t watchdog, int arg)
+/*
+ * Set the timer 'tp' to trigger 'ticks' clock ticks in the future.  When it
+ * triggers, call function 'watchdog' with argument 'arg'.  The given timer
+ * object must have been initialized with init_timer(3) already.  The given
+ * number of ticks must be between 0 and TMRDIFF_MAX inclusive.  A ticks value
+ * of zero will cause the alarm to trigger on the next clock tick.  If the
+ * timer was already set, it will be canceled first.
+ */
+void
+set_timer(minix_timer_t *tp, clock_t ticks, tmr_func_t watchdog, int arg)
 {
-        clock_t prev_time = 0, next_time;
+	clock_t prev_time, next_time;
+	int r, had_timers;
 
-        /* Set timer argument and add timer to the list. */
-        tmr_arg(tp)->ta_int = arg;
-        prev_time = tmrs_settimer(&timers, tp, getticks() + ticks, watchdog,
-            &next_time);
+	if (ticks > TMRDIFF_MAX)
+		panic("set_timer: ticks value too large: %u", (int)ticks);
 
-        /* Reschedule our synchronous alarm if necessary. */
-        if (expiring == 0 && (! prev_time || prev_time > next_time)) {
-                if (sys_setalarm(next_time, 1) != OK)
-                        panic("set_timer: couldn't set alarm");
+	/* Add the timer to the list. */
+	had_timers = tmrs_settimer(&timers, tp, getticks() + ticks, watchdog,
+	    arg, &prev_time, &next_time);
+
+	/* Reschedule our synchronous alarm if necessary. */
+	if (!expiring && (!had_timers || next_time != prev_time)) {
+		if ((r = sys_setalarm(next_time, TRUE /*abs_time*/)) != OK)
+			panic("set_timer: couldn't set alarm: %d", r);
         }
 }
 
-/*===========================================================================*
- *                              cancel_timer                                 *
- *===========================================================================*/
-void cancel_timer(minix_timer_t *tp)
+/*
+ * Cancel the timer 'tp'.  The timer object must have been initialized with
+ * init_timer(3) first.  If the timer was not set before, the call is a no-op.
+ */
+void
+cancel_timer(minix_timer_t * tp)
 {
-        clock_t next_time, prev_time;
-        prev_time = tmrs_clrtimer(&timers, tp, &next_time);
-
-        /* If the earliest timer has been removed, we have to set the alarm to
-         * the next timer, or cancel the alarm altogether if the last timer
-         * has been cancelled (next_time will be 0 then).
-         */
-        if (expiring == 0 && (prev_time < next_time || ! next_time)) {
-                if (sys_setalarm(next_time, 1) != OK)
-                        panic("cancel_timer: couldn't set alarm");
+	clock_t next_time, prev_time;
+	int r, have_timers;
+
+	if (!tmr_is_set(tp))
+		return;
+
+	have_timers = tmrs_clrtimer(&timers, tp, &prev_time, &next_time);
+
+	/*
+	 * If the earliest timer has been removed, we have to set the alarm to
+	 * the next timer, or cancel the alarm altogether if the last timer
+	 * has been canceled.
+	 */
+        if (!expiring) {
+		if (!have_timers)
+			r = sys_setalarm(0, FALSE /*abs_time*/);
+		else if (prev_time != next_time)
+			r = sys_setalarm(next_time, TRUE /*abs_time*/);
+		else
+			r = OK;
+
+		if (r != OK)
+                        panic("cancel_timer: couldn't set alarm: %d", r);
         }
 }
 
-/*===========================================================================*
- *                              expire_timers                                *
- *===========================================================================*/
-void expire_timers(clock_t now)
+/*
+ * Expire all timers that were set to expire before/at the given current time.
+ */
+void
+expire_timers(clock_t now)
 {
         clock_t next_time;
+	int r, have_timers;
 
-        /* Check for expired timers. Use a global variable to indicate that
-         * watchdog functions are called, so that sys_setalarm() isn't called
-         * more often than necessary when set_timer or cancel_timer are called
-         * from these watchdog functions. */
-        expiring = 1;
-        tmrs_exptimers(&timers, now, &next_time);
-        expiring = 0;
-
-        /* Reschedule an alarm if necessary. */
-        if (next_time > 0) {
-                if (sys_setalarm(next_time, 1) != OK)
-                        panic("expire_timers: couldn't set alarm");
-        }
+	/*
+	 * Check for expired timers. Use a global variable to indicate that
+	 * watchdog functions are called, so that sys_setalarm() isn't called
+	 * more often than necessary when set_timer or cancel_timer are called
+	 * from these watchdog functions.
+	 */
+	expiring = TRUE;
+	have_timers = tmrs_exptimers(&timers, now, &next_time);
+	expiring = FALSE;
+
+	/* Reschedule an alarm if necessary. */
+	if (have_timers) {
+		if ((r = sys_setalarm(next_time, TRUE /*abs_time*/)) != OK)
+			panic("expire_timers: couldn't set alarm: %d", r);
+	}
 }
diff --git a/minix/lib/libtimers/timers.h b/minix/lib/libtimers/timers.h
deleted file mode 100644
index 794c0124d..000000000
--- a/minix/lib/libtimers/timers.h
+++ /dev/null
@@ -1,6 +0,0 @@
-/* This library provides generic watchdog timer management functionality.
- * See the comments in <minix/timers.h> for details.
- */
-
-#include <minix/timers.h>		/* definitions and function prototypes */
-#include <sys/null.h>
diff --git a/minix/lib/libtimers/tmrs_clr.c b/minix/lib/libtimers/tmrs_clr.c
index 2d0b0f6a7..19d2103cb 100644
--- a/minix/lib/libtimers/tmrs_clr.c
+++ b/minix/lib/libtimers/tmrs_clr.c
@@ -1,39 +1,44 @@
-#include "timers.h"
+#include <minix/timers.h>
 
-/*===========================================================================*
- *				tmrs_clrtimer				     *
- *===========================================================================*/
-clock_t tmrs_clrtimer(tmrs, tp, next_time)
-minix_timer_t **tmrs;				/* pointer to timers queue */
-minix_timer_t *tp;				/* timer to be removed */
-clock_t *next_time;
-{
-/* Deactivate a timer and remove it from the timers queue. 
+/*
+ * Deactivate a timer and remove it from the timers queue.  'tmrs' is a pointer
+ * to the timers queue.  'tp' is a pointer to the timer to be removed, which
+ * generally should be on the queue (but this is not a requirement, and the
+ * kernel abuses this).  If 'prev_time' is non-NULL, it is filled with the
+ * previous timer head time, which always exists since at least 'tp' is on it.
+ * The function returns TRUE if there is still at least one timer on the queue
+ * after this function is done, in which case 'next_time' (if non-NULL) is
+ * filled with the absolute expiry time of the new head timer.
  */
-  minix_timer_t **atp;
-  clock_t prev_time;
+int
+tmrs_clrtimer(minix_timer_t ** tmrs, minix_timer_t * tp, clock_t * prev_time,
+	clock_t * next_time)
+{
+	minix_timer_t **atp;
+	int r;
 
-  if(*tmrs)
-  	prev_time = (*tmrs)->tmr_exp_time;
-  else
-  	prev_time = 0;
+	if (*tmrs != NULL) {
+		if (prev_time != NULL)
+			*prev_time = (*tmrs)->tmr_exp_time;
+		r = TRUE;
+	} else
+		r = FALSE;
 
-  tp->tmr_exp_time = TMR_NEVER;
+	tp->tmr_func = NULL;	/* clear the timer object */
 
-  for (atp = tmrs; *atp != NULL; atp = &(*atp)->tmr_next) {
-	if (*atp == tp) {
-		*atp = tp->tmr_next;
-		break;
+	for (atp = tmrs; *atp != NULL; atp = &(*atp)->tmr_next) {
+		if (*atp == tp) {
+			*atp = tp->tmr_next;
+			break;
+		}
 	}
-  }
 
-  if(next_time) {
-  	if(*tmrs)
-  		*next_time = (*tmrs)->tmr_exp_time;
-  	else	
-  		*next_time = 0;
-  }
+	if (next_time != NULL) {
+		if (*tmrs != NULL)
+			*next_time = (*tmrs)->tmr_exp_time;
+		else
+			*next_time = 0;
+	}
 
-  return prev_time;
+	return r;
 }
-
diff --git a/minix/lib/libtimers/tmrs_exp.c b/minix/lib/libtimers/tmrs_exp.c
index d54ca839f..4ad4b16d1 100644
--- a/minix/lib/libtimers/tmrs_exp.c
+++ b/minix/lib/libtimers/tmrs_exp.c
@@ -1,31 +1,29 @@
-#include "timers.h"
+#include <minix/timers.h>
 
-/*===========================================================================*
- *				tmrs_exptimers				     *
- *===========================================================================*/
-void tmrs_exptimers(tmrs, now, new_head)
-minix_timer_t **tmrs;				/* pointer to timers queue */
-clock_t now;				/* current time */
-clock_t *new_head;
-{
-/* Use the current time to check the timers queue list for expired timers. 
+/*
+ * Use the current time to check the timers queue list for expired timers.
  * Run the watchdog functions for all expired timers and deactivate them.
  * The caller is responsible for scheduling a new alarm if needed.
  */
-  minix_timer_t *tp;
+int
+tmrs_exptimers(minix_timer_t ** tmrs, clock_t now, clock_t * new_head)
+{
+	minix_timer_t *tp;
+	tmr_func_t func;
 
-  while ((tp = *tmrs) != NULL && tp->tmr_exp_time <= now) {
-	*tmrs = tp->tmr_next;
-	tp->tmr_exp_time = TMR_NEVER;
-	(*tp->tmr_func)(tp);
-  }
+	while ((tp = *tmrs) != NULL && tmr_has_expired(tp, now)) {
+		*tmrs = tp->tmr_next;
 
-  if(new_head) {
-  	if(*tmrs)
-  		*new_head = (*tmrs)->tmr_exp_time;
-  	else
-  		*new_head = 0;
-  }
-}
+		func = tp->tmr_func;
+		tp->tmr_func = NULL;
 
+		(*func)(tp->tmr_arg);
+	}
 
+	if (*tmrs != NULL) {
+		if (new_head != NULL)
+			*new_head = (*tmrs)->tmr_exp_time;
+		return TRUE;
+	} else
+		return FALSE;
+}
diff --git a/minix/lib/libtimers/tmrs_set.c b/minix/lib/libtimers/tmrs_set.c
index 6d1cf709c..d87a74ee6 100644
--- a/minix/lib/libtimers/tmrs_set.c
+++ b/minix/lib/libtimers/tmrs_set.c
@@ -1,39 +1,48 @@
-#include "timers.h"
+#include <minix/timers.h>
 
-/*===========================================================================*
- *				tmrs_settimer				     *
- *===========================================================================*/
-clock_t tmrs_settimer(tmrs, tp, exp_time, watchdog, new_head)
-minix_timer_t **tmrs;				/* pointer to timers queue */
-minix_timer_t *tp;				/* the timer to be added */
-clock_t exp_time;			/* its expiration time */
-tmr_func_t watchdog;			/* watchdog function to be run */
-clock_t *new_head;			/* new earliest timer, if non NULL */
-{
-/* Activate a timer to run function 'fp' at time 'exp_time'. If the timer is
- * already in use it is first removed from the timers queue. Then, it is put
- * in the list of active timers with the first to expire in front.
- * The caller responsible for scheduling a new alarm for the timer if needed. 
+/*
+ * Activate a timer to run function 'watchdog' at absolute time 'exp_time', as
+ * part of timers queue 'tmrs'. If the timer is already in use, it is first
+ * removed from the timers queue.  Then, it is put in the list of active timers
+ * with the first to expire in front.  The caller responsible for scheduling a
+ * new alarm for the timer if needed.  To that end, the function returns three
+ * values: its return value (TRUE or FALSE) indicates whether there was an old
+ * head timer; if TRUE, 'old_head' (if non-NULL) is filled with the absolute
+ * expiry time of the old head timer.  If 'new_head' is non-NULL, it is filled
+ * with the absolute expiry time of the new head timer.
  */
-  minix_timer_t **atp;
-  clock_t old_head = 0;
+int
+tmrs_settimer(minix_timer_t ** tmrs, minix_timer_t * tp, clock_t exp_time,
+	tmr_func_t watchdog, int arg, clock_t * old_head, clock_t * new_head)
+{
+	minix_timer_t **atp;
+	int r;
 
-  if(*tmrs)
-  	old_head = (*tmrs)->tmr_exp_time;
+	if (*tmrs != NULL) {
+		if (old_head != NULL)
+			*old_head = (*tmrs)->tmr_exp_time;
+		r = TRUE;
+	} else
+		r = FALSE;
 
-  /* Set the timer's variables. */
-  (void) tmrs_clrtimer(tmrs, tp, NULL);
-  tp->tmr_exp_time = exp_time;
-  tp->tmr_func = watchdog;
+	/* Set the timer's variables. */
+	if (tmr_is_set(tp))
+		(void)tmrs_clrtimer(tmrs, tp, NULL, NULL);
+	tp->tmr_exp_time = exp_time;
+	tp->tmr_func = watchdog;	/* set the timer object */
+	tp->tmr_arg = arg;
 
-  /* Add the timer to the active timers. The next timer due is in front. */
-  for (atp = tmrs; *atp != NULL; atp = &(*atp)->tmr_next) {
-	if (exp_time < (*atp)->tmr_exp_time) break;
-  }
-  tp->tmr_next = *atp;
-  *atp = tp;
-  if(new_head)
-  	(*new_head) = (*tmrs)->tmr_exp_time;
-  return old_head;
-}
+	/*
+	 * Add the timer to the active timers. The next timer due is in front.
+	 */
+	for (atp = tmrs; *atp != NULL; atp = &(*atp)->tmr_next) {
+		if (tmr_is_first(exp_time, (*atp)->tmr_exp_time))
+			break;
+	}
+	tp->tmr_next = *atp;
+	*atp = tp;
 
+	if (new_head != NULL)
+		*new_head = (*tmrs)->tmr_exp_time;
+	return r;
+}
diff --git a/minix/net/lwip/lwip.c b/minix/net/lwip/lwip.c
index ac00f72ff..649ad038e 100644
--- a/minix/net/lwip/lwip.c
+++ b/minix/net/lwip/lwip.c
@@ -36,19 +36,19 @@ static void sys_init(void)
 {
 }
 
-static void arp_watchdog(__unused minix_timer_t *tp)
+static void arp_watchdog(int arg __unused)
 {
 	etharp_tmr();
 	set_timer(&arp_tmr, arp_ticks, arp_watchdog, 0);
 }
 
-static void tcp_fwatchdog(__unused minix_timer_t *tp)
+static void tcp_fwatchdog(int arg __unused)
 {
 	tcp_fasttmr();
 	set_timer(&tcp_ftmr, tcp_fticks, tcp_fwatchdog, 0);
 }
 
-static void tcp_swatchdog(__unused minix_timer_t *tp)
+static void tcp_swatchdog(int arg __unused)
 {
 	tcp_slowtmr();
 	set_timer(&tcp_ftmr, tcp_sticks, tcp_swatchdog, 0);
diff --git a/minix/servers/pm/alarm.c b/minix/servers/pm/alarm.c
index d7ff11ba5..e10f84dca 100644
--- a/minix/servers/pm/alarm.c
+++ b/minix/servers/pm/alarm.c
@@ -13,6 +13,7 @@
 #include <sys/time.h>
 #include <minix/com.h>
 #include <minix/callnr.h>
+#include <assert.h>
 #include "mproc.h"
 
 #define US 1000000UL	/* shortcut for microseconds per second */
@@ -24,7 +25,7 @@ static void getset_vtimer(struct mproc *mp, int nwhich, struct
 	itimerval *value, struct itimerval *ovalue);
 static void get_realtimer(struct mproc *mp, struct itimerval *value);
 static void set_realtimer(struct mproc *mp, struct itimerval *value);
-static void cause_sigalrm(minix_timer_t *tp);
+static void cause_sigalrm(int arg);
 
 /*===========================================================================*
  *				ticks_from_timeval			     *
@@ -252,7 +253,7 @@ get_realtimer(struct mproc *rmp, struct itimerval *value)
   /* First determine remaining time, in ticks, of previous alarm, if set. */
   if (rmp->mp_flags & ALARM_ON) {
 	uptime = getticks();
-  	exptime = *tmr_exp_time(&rmp->mp_timer);
+	exptime = tmr_exp_time(&rmp->mp_timer);
 
   	remaining = exptime - uptime;
 
@@ -300,6 +301,7 @@ struct mproc *rmp;		/* process that wants the alarm */
 clock_t ticks;			/* how many ticks delay before the signal */
 {
   if (ticks > 0) {
+	assert(ticks <= TMRDIFF_MAX);
   	set_timer(&rmp->mp_timer, ticks, cause_sigalrm, rmp->mp_endpoint);
 	rmp->mp_flags |=  ALARM_ON;
   } else if (rmp->mp_flags & ALARM_ON) {
@@ -311,16 +313,15 @@ clock_t ticks;			/* how many ticks delay before the signal */
 /*===========================================================================*
  *				cause_sigalrm				     *
  *===========================================================================*/
-static void cause_sigalrm(tp)
-minix_timer_t *tp;
+static void
+cause_sigalrm(int arg)
 {
   int proc_nr_n;
   register struct mproc *rmp;
 
   /* get process from timer */
-  if(pm_isokendpt(tmr_arg(tp)->ta_int, &proc_nr_n) != OK) {
-  	printf("PM: ignoring timer for invalid endpoint %d\n",
-  		tmr_arg(tp)->ta_int);
+  if(pm_isokendpt(arg, &proc_nr_n) != OK) {
+	printf("PM: ignoring timer for invalid endpoint %d\n", arg);
   	return;
   }
 
@@ -331,8 +332,7 @@ minix_timer_t *tp;
 
   /* If an interval is set, set a new timer; otherwise clear the ALARM_ON flag.
    * The set_alarm call will be calling set_timer from within this callback
-   * from the expire_timers function. This is safe, but we must not use the
-   * "tp" structure below this point anymore.
+   * from the expire_timers function. This is safe.
    */
   if (rmp->mp_interval[ITIMER_REAL] > 0)
 	set_alarm(rmp, rmp->mp_interval[ITIMER_REAL]);
diff --git a/minix/servers/pm/const.h b/minix/servers/pm/const.h
index 01be61f0d..2c48988d3 100644
--- a/minix/servers/pm/const.h
+++ b/minix/servers/pm/const.h
@@ -12,8 +12,7 @@
 
 #define NO_EVENTSUB	((char)-1) /* no current process event subscriber */
 
-#define MAX_CLOCK_T	((unsigned long) 1 << ((sizeof(clock_t) * 8) - 1))
-#define MAX_SECS	( (clock_t) (MAX_CLOCK_T/system_hz) )
+#define MAX_SECS	( (clock_t) (TMRDIFF_MAX/system_hz) )
 				/* max.secs for setitimer() ((2^31-1)/HZ) */
 #define NR_ITIMERS	   3	/* number of supported interval timers */
 
diff --git a/minix/servers/vfs/proto.h b/minix/servers/vfs/proto.h
index ca5351bd1..37763a655 100644
--- a/minix/servers/vfs/proto.h
+++ b/minix/servers/vfs/proto.h
@@ -351,7 +351,6 @@ void select_callback(struct filp *, int ops);
 void select_forget(void);
 void select_reply1(endpoint_t driver_e, devminor_t minor, int status);
 void select_reply2(endpoint_t driver_e, devminor_t minor, int status);
-void select_timeout_check(minix_timer_t *);
 void select_unsuspend_by_endpt(endpoint_t proc);
 
 /* worker.c */
diff --git a/minix/servers/vfs/select.c b/minix/servers/vfs/select.c
index 490fa3802..5b834b231 100644
--- a/minix/servers/vfs/select.c
+++ b/minix/servers/vfs/select.c
@@ -32,6 +32,8 @@
 #define FROM_PROC 0
 #define TO_PROC   1
 
+#define USECPERSEC 1000000	/* number of microseconds in a second */
+
 typedef fd_set *ixfer_fd_set_ptr;
 
 static struct selectentry {
@@ -71,6 +73,7 @@ static void select_return(struct selectentry *);
 static void select_restart_filps(void);
 static int tab2ops(int fd, struct selectentry *e);
 static void wipe_select(struct selectentry *s);
+void select_timeout_check(int s);
 
 static struct fdtype {
 	int (*select_request)(struct filp *, int *ops, int block,
@@ -96,12 +99,13 @@ int do_select(void)
  * timeout and wait for either the file descriptors to become ready or the
  * timer to go off. If no timeout value was provided, we wait indefinitely.
  */
-  int r, nfds, do_timeout = 0, fd, s;
+  int r, nfds, do_timeout, fd, s;
   struct filp *f;
   unsigned int type, ops;
   struct timeval timeout;
   struct selectentry *se;
   vir_bytes vtimeout;
+  clock_t ticks;
 
   nfds = job_m_in.m_lc_vfs_select.nfds;
   vtimeout = job_m_in.m_lc_vfs_select.timeout;
@@ -131,20 +135,21 @@ int do_select(void)
 
   /* Did the process set a timeout value? If so, retrieve it. */
   if (vtimeout != 0) {
-	do_timeout = 1;
 	r = sys_datacopy_wrapper(who_e, vtimeout, SELF, (vir_bytes) &timeout,
 		sizeof(timeout));
+
+	/* No nonsense in the timeval */
+	if (r == OK && (timeout.tv_sec < 0 || timeout.tv_usec < 0 ||
+	    timeout.tv_usec >= USECPERSEC))
+		r = EINVAL;
+
 	if (r != OK) {
 		se->requestor = NULL;
 		return(r);
 	}
-  }
-
-  /* No nonsense in the timeval */
-  if (do_timeout && (timeout.tv_sec < 0 || timeout.tv_usec < 0)) {
-	se->requestor = NULL;
-	return(EINVAL);
-  }
+	do_timeout = 1;
+  } else
+	do_timeout = 0;
 
   /* If there is no timeout, we block forever. Otherwise, we block up to the
    * specified time interval.
@@ -282,22 +287,20 @@ int do_select(void)
   /* Convert timeval to ticks and set the timer. If it fails, undo
    * all, return error.
    */
-  if (do_timeout) {
-	int ticks;
+  if (do_timeout && se->block) {
 	/* Open Group:
 	 * "If the requested timeout interval requires a finer
 	 * granularity than the implementation supports, the
 	 * actual timeout interval shall be rounded up to the next
 	 * supported value."
 	 */
-#define USECPERSEC 1000000
-	while(timeout.tv_usec >= USECPERSEC) {
-		/* this is to avoid overflow with *system_hz below */
-		timeout.tv_usec -= USECPERSEC;
-		timeout.tv_sec++;
+	if (timeout.tv_sec >= (TMRDIFF_MAX - 1) / system_hz) {
+		ticks = TMRDIFF_MAX; /* silently truncate */
+	} else {
+		ticks = timeout.tv_sec * system_hz +
+		    (timeout.tv_usec * system_hz + USECPERSEC-1) / USECPERSEC;
 	}
-	ticks = timeout.tv_sec * system_hz +
-		(timeout.tv_usec * system_hz + USECPERSEC-1) / USECPERSEC;
+	assert(ticks != 0 && ticks <= TMRDIFF_MAX);
 	se->expiry = ticks;
 	set_timer(&se->timer, ticks, select_timeout_check, s);
   }
@@ -742,20 +745,18 @@ void select_forget(void)
 /*===========================================================================*
  *				select_timeout_check	  	     	     *
  *===========================================================================*/
-void select_timeout_check(minix_timer_t *timer)
+void select_timeout_check(int s)
 {
 /* An alarm has gone off for one of the select queries. This function MUST NOT
  * block its calling thread.
  */
-  int s;
   struct selectentry *se;
 
-  s = tmr_arg(timer)->ta_int;
   if (s < 0 || s >= MAXSELECTS) return;	/* Entry does not exist */
 
   se = &selecttab[s];
   if (se->requestor == NULL) return;
-  if (se->expiry <= 0) return;	/* Strange, did we even ask for a timeout? */
+  if (se->expiry == 0) return;	/* Strange, did we even ask for a timeout? */
   se->expiry = 0;
   if (!is_deferred(se))
 	select_return(se);
diff --git a/minix/tests/t40f.c b/minix/tests/t40f.c
index 697b94f1c..47f974914 100644
--- a/minix/tests/t40f.c
+++ b/minix/tests/t40f.c
@@ -59,7 +59,7 @@ static void do_child(void) {
   struct timeval tv;
  
   /* Let the parent do initial read and write tests from and to the pipe. */
-  tv.tv_sec = DO_PAUSE + DO_PAUSE + DO_PAUSE + 1;
+  tv.tv_sec = DO_PAUSE + DO_PAUSE + 1;
   tv.tv_usec = 0;
   (void) select(0, NULL, NULL, NULL, &tv);
 
@@ -109,16 +109,28 @@ static void do_parent(int child) {
   retval = select(fd_ap[0]+1, &fds_read, NULL, NULL, &tv); 
   (void) gettimeofday(&end_time, NULL);     /* Record ending time */
   
-  if(retval != 0) em(3, "Should have timed out");
-  if(compute_diff(start_time, end_time, DO_PAUSE) > DO_DELTA)
-    em(4, "Time difference too large");
-  
+  if(retval != -1) em(3, "Should have failed");
+  if(errno != EINVAL) em(4, "Incorrect error thrown");
+
+  /* Do a few more tests for invalid timeout values. */
+  tv.tv_sec = 0;
+  tv.tv_usec = 1000000;
+  retval = select(fd_ap[0]+1, &fds_read, NULL, NULL, &tv);
+  if (retval != -1) em(0, "Should have failed");
+  if (errno != EINVAL) em(0, "Incorrect error thrown");
+
+  tv.tv_sec = 0;
+  tv.tv_usec = ~0;
+  retval = select(fd_ap[0]+1, &fds_read, NULL, NULL, &tv);
+  if (retval != -1) em(0, "Should have failed");
+  if (errno != EINVAL) em(0, "Incorrect error thrown");
+
   /* Let's wait for another DO_PAUSE seconds, expressed in seconds and micro
      seconds. */
   FD_ZERO(&fds_read);
   FD_SET(fd_ap[0], &fds_read);
   tv.tv_sec = DO_PAUSE - 1;
-  tv.tv_usec = (DO_PAUSE - tv.tv_sec) * 1000000L;
+  tv.tv_usec = 999999L; /* close enough */
   
   (void) gettimeofday(&start_time, NULL);   /* Record starting time */
   retval = select(fd_ap[0]+1, &fds_read, NULL, NULL, &tv);