replacing timeout_add() with timeout_add_msec()

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

replacing timeout_add() with timeout_add_msec()

Amit Kulkarni
Hi,

Referring to the end of mpi's message, and also mlarkin@ later comment https://marc.info/?l=openbsd-tech&m=154577028830964&w=2

I am trying to replace some easy timeout_add() calls with timeout_add_msec().

My current understanding with the occurences of timeout_add() in the tree is that: if there is a hardcoded call like timeout_add(struct timeout, 1), then replace with timeout_add_msec(struct timeout, 10). That is, 1 tick = 10 msec.

So if there's a hardcoded call like timeout_add(struct timeout, 5), then replace with timeout_add_msec(struct timeout, 50).

If there are hz calculations which I don't understand like for example in /sys/arch/alpha/tc/ioasic.c, then I am skipping these for now.
        if (alpha_led_blink != 0) {
                timeout_set(&led_blink_state.tmo, ioasic_led_blink, NULL);
                timeout_add(&led_blink_state.tmo,
                    (((averunnable.ldavg[0] + FSCALE) * hz) >> (FSHIFT + 3)));
        }

A call like timeout_add(struct timeout, 0) is replaced by an equivalent call to timeout_add_msec(struct timeout, 0).

Both the above scenarios are in the following diff and un-tested (not compiled also, for now), no way I can test some of these, as I don't have access to hardware. Mainly looking for critical review and feedback to get this going in the right direction.

Thanks for your time!

diff --git arch/alpha/alpha/promcons.c arch/alpha/alpha/promcons.c
index 9efabd3bf1c..b872f6e3931 100644
--- arch/alpha/alpha/promcons.c
+++ arch/alpha/alpha/promcons.c
@@ -100,7 +100,7 @@ promopen(dev, flag, mode, p)
  error = (*linesw[tp->t_line].l_open)(dev, tp, p);
  if (error == 0 && setuptimeout) {
  timeout_set(&prom_to, promtimeout, tp);
- timeout_add(&prom_to, 1);
+ timeout_add_msec(&prom_to, 10);
  }
  return error;
 }
@@ -220,7 +220,7 @@ promtimeout(v)
  if (tp->t_state & TS_ISOPEN)
  (*linesw[tp->t_line].l_rint)(c, tp);
  }
- timeout_add(&prom_to, 1);
+ timeout_add_msec(&prom_to, 10);
 }
 
 struct tty *
diff --git arch/amd64/isa/clock.c arch/amd64/isa/clock.c
index db516d9ecde..9d5934e6817 100644
--- arch/amd64/isa/clock.c
+++ arch/amd64/isa/clock.c
@@ -326,7 +326,7 @@ rtcstart(void)
  mc146818_write(NULL, MC_REGB, MC_REGB_24HR | MC_REGB_PIE);
 
  /*
- * On a number of i386 systems, the rtc will fail to start when booting
+ * On a number of amd64 systems, the rtc will fail to start when booting
  * the system. This is due to us missing to acknowledge an interrupt
  * during early stages of the boot process. If we do not acknowledge
  * the interrupt, the rtc clock will not generate further interrupts.
@@ -334,7 +334,7 @@ rtcstart(void)
  * to drain any un-acknowledged rtc interrupt(s).
  */
  timeout_set(&rtcdrain_timeout, rtcdrain, (void *)&rtcdrain_timeout);
- timeout_add(&rtcdrain_timeout, 1);
+ timeout_add_msec(&rtcdrain_timeout, 10);
 }
 
 void
diff --git arch/amd64/pci/pchb.c arch/amd64/pci/pchb.c
index 6e599d7be4a..80b5ada1cb4 100644
--- arch/amd64/pci/pchb.c
+++ arch/amd64/pci/pchb.c
@@ -332,7 +332,7 @@ pchb_rnd(void *v)
  }
  }
 
- timeout_add(&sc->sc_rng_to, 1);
+ timeout_add(&sc->sc_rng_to, 10);
 }
 
 void
diff --git dev/ic/vga.c dev/ic/vga.c
index 74cc3e07bf8..2cebb65d2d5 100644
--- dev/ic/vga.c
+++ dev/ic/vga.c
@@ -765,7 +765,7 @@ vga_show_screen(void *v, void *cookie, int waitok, void (*cb)(void *, int, int),
  if (cb) {
  timeout_set(&vc->vc_switch_timeout,
     (void(*)(void *))vga_doswitch, vc);
- timeout_add(&vc->vc_switch_timeout, 0);
+ timeout_add(&vc->vc_switch_timeout, 10);
  return (EAGAIN);
  }
 
diff --git net/if_pfsync.c net/if_pfsync.c
index 8d842e48466..dc8d2f41466 100644
--- net/if_pfsync.c
+++ net/if_pfsync.c
@@ -2318,7 +2318,7 @@ pfsync_bulk_start(void)
  sc->sc_bulk_last = sc->sc_bulk_next;
 
  pfsync_bulk_status(PFSYNC_BUS_START);
- timeout_add(&sc->sc_bulk_tmo, 0);
+ timeout_add_msec(&sc->sc_bulk_tmo, 0);
  }
 }
 



Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Theo de Raadt-2
Amit Kulkarni <[hidden email]> wrote:

> Hi,
>
> Referring to the end of mpi's message, and also mlarkin@ later comment https://marc.info/?l=openbsd-tech&m=154577028830964&w=2
>
> I am trying to replace some easy timeout_add() calls with timeout_add_msec().
>
> My current understanding with the occurences of timeout_add() in the tree is that: if there is a hardcoded call like timeout_add(struct timeout, 1), then replace with timeout_add_msec(struct timeout, 10). That is, 1 tick = 10 msec.
>
> So if there's a hardcoded call like timeout_add(struct timeout, 5), then replace with timeout_add_msec(struct timeout, 50).
>
> If there are hz calculations which I don't understand like for example in /sys/arch/alpha/tc/ioasic.c, then I am skipping these for now.
>         if (alpha_led_blink != 0) {
>                 timeout_set(&led_blink_state.tmo, ioasic_led_blink, NULL);
>                 timeout_add(&led_blink_state.tmo,
>                     (((averunnable.ldavg[0] + FSCALE) * hz) >> (FSHIFT + 3)));
>         }
>
> A call like timeout_add(struct timeout, 0) is replaced by an equivalent call to timeout_add_msec(struct timeout, 0).
>
> Both the above scenarios are in the following diff and un-tested (not compiled also, for now), no way I can test some of these, as I don't have access to hardware. Mainly looking for critical review and feedback to get this going in the right direction.
>
> Thanks for your time!
>
> diff --git arch/alpha/alpha/promcons.c arch/alpha/alpha/promcons.c
> index 9efabd3bf1c..b872f6e3931 100644
> --- arch/alpha/alpha/promcons.c
> +++ arch/alpha/alpha/promcons.c
> @@ -100,7 +100,7 @@ promopen(dev, flag, mode, p)
>   error = (*linesw[tp->t_line].l_open)(dev, tp, p);
>   if (error == 0 && setuptimeout) {
>   timeout_set(&prom_to, promtimeout, tp);
> - timeout_add(&prom_to, 1);
> + timeout_add_msec(&prom_to, 10);
>   }
>   return error;
>  }
> @@ -220,7 +220,7 @@ promtimeout(v)
>   if (tp->t_state & TS_ISOPEN)
>   (*linesw[tp->t_line].l_rint)(c, tp);
>   }
> - timeout_add(&prom_to, 1);
> + timeout_add_msec(&prom_to, 10);
>  }
>  
>  struct tty *

I am glad you have an alpha, and will be able to test your proposed change.

Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Amit Kulkarni
Even on amd64, I won't be able to test, because of missing hardware.
If you think something is wrong, please will you let me have your
feedback?

Thanks


On Sun, Jan 6, 2019 at 4:56 PM Theo de Raadt <[hidden email]> wrote:

>
> Amit Kulkarni <[hidden email]> wrote:
>
> > Hi,
> >
> > Referring to the end of mpi's message, and also mlarkin@ later comment https://marc.info/?l=openbsd-tech&m=154577028830964&w=2
> >
> > I am trying to replace some easy timeout_add() calls with timeout_add_msec().
> >
> > My current understanding with the occurences of timeout_add() in the tree is that: if there is a hardcoded call like timeout_add(struct timeout, 1), then replace with timeout_add_msec(struct timeout, 10). That is, 1 tick = 10 msec.
> >
> > So if there's a hardcoded call like timeout_add(struct timeout, 5), then replace with timeout_add_msec(struct timeout, 50).
> >
> > If there are hz calculations which I don't understand like for example in /sys/arch/alpha/tc/ioasic.c, then I am skipping these for now.
> >         if (alpha_led_blink != 0) {
> >                 timeout_set(&led_blink_state.tmo, ioasic_led_blink, NULL);
> >                 timeout_add(&led_blink_state.tmo,
> >                     (((averunnable.ldavg[0] + FSCALE) * hz) >> (FSHIFT + 3)));
> >         }
> >
> > A call like timeout_add(struct timeout, 0) is replaced by an equivalent call to timeout_add_msec(struct timeout, 0).
> >
> > Both the above scenarios are in the following diff and un-tested (not compiled also, for now), no way I can test some of these, as I don't have access to hardware. Mainly looking for critical review and feedback to get this going in the right direction.
> >
> > Thanks for your time!
> >
> > diff --git arch/alpha/alpha/promcons.c arch/alpha/alpha/promcons.c
> > index 9efabd3bf1c..b872f6e3931 100644
> > --- arch/alpha/alpha/promcons.c
> > +++ arch/alpha/alpha/promcons.c
> > @@ -100,7 +100,7 @@ promopen(dev, flag, mode, p)
> >       error = (*linesw[tp->t_line].l_open)(dev, tp, p);
> >       if (error == 0 && setuptimeout) {
> >               timeout_set(&prom_to, promtimeout, tp);
> > -             timeout_add(&prom_to, 1);
> > +             timeout_add_msec(&prom_to, 10);
> >       }
> >       return error;
> >  }
> > @@ -220,7 +220,7 @@ promtimeout(v)
> >               if (tp->t_state & TS_ISOPEN)
> >                       (*linesw[tp->t_line].l_rint)(c, tp);
> >       }
> > -     timeout_add(&prom_to, 1);
> > +     timeout_add_msec(&prom_to, 10);
> >  }
> >
> >  struct tty *
>
> I am glad you have an alpha, and will be able to test your proposed change.

Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Theo de Raadt-2
Amit Kulkarni <[hidden email]> wrote:

> Even on amd64, I won't be able to test, because of missing hardware.
> If you think something is wrong, please will you let me have your
> feedback?

I'm a bit stunned at the zeal to push untested diffs into the tree

(you didn't ask anyone to test it for you)

Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Amit Kulkarni
> > Even on amd64, I won't be able to test, because of missing hardware.
> > If you think something is wrong, please will you let me have your
> > feedback?
>
> I'm a bit stunned at the zeal to push untested diffs into the tree
>
> (you didn't ask anyone to test it for you)

I requested for critical review or feedback in the initial email, to know if I am going down the right path. If the approach is ok, then I was planning to sit down to try converting all the straightforward of timeout_add() to timeout_add_msec() calls in the tree, then a review again, before finally requesting a test run.

Requesting a test, when I am unsure of the approach in code, would lead to low or zero confidence from others when asked in future.

Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Claudio Jeker
In reply to this post by Amit Kulkarni
On Sun, Jan 06, 2019 at 01:43:19PM -0600, Amit Kulkarni wrote:

> Hi,
>
> Referring to the end of mpi's message, and also mlarkin@ later comment https://marc.info/?l=openbsd-tech&m=154577028830964&w=2
>
> I am trying to replace some easy timeout_add() calls with timeout_add_msec().
>
> My current understanding with the occurences of timeout_add() in the tree is that: if there is a hardcoded call like timeout_add(struct timeout, 1), then replace with timeout_add_msec(struct timeout, 10). That is, 1 tick = 10 msec.
>
> So if there's a hardcoded call like timeout_add(struct timeout, 5), then replace with timeout_add_msec(struct timeout, 50).
>
> If there are hz calculations which I don't understand like for example in /sys/arch/alpha/tc/ioasic.c, then I am skipping these for now.
>         if (alpha_led_blink != 0) {
>                 timeout_set(&led_blink_state.tmo, ioasic_led_blink, NULL);
>                 timeout_add(&led_blink_state.tmo,
>                     (((averunnable.ldavg[0] + FSCALE) * hz) >> (FSHIFT + 3)));
>         }
>
> A call like timeout_add(struct timeout, 0) is replaced by an equivalent call to timeout_add_msec(struct timeout, 0).
>
> Both the above scenarios are in the following diff and un-tested (not compiled also, for now), no way I can test some of these, as I don't have access to hardware. Mainly looking for critical review and feedback to get this going in the right direction.
>
> Thanks for your time!

You started to convert the wrong timeout_add() calls. Doing a blind
timeout_add() to timeout_add_msec() conversion especially on calls with 0
or 1 tick sleep time is the wrong approach.
The right approach is to identify calls that do sleep for a well defined time
(1sec, 50ms or whatever) and convert those. Most of the timeouts you
changes are not in that category. First I would look at timeouts that have
a multiplication with hz in them since those wait for a specific time.

> diff --git arch/alpha/alpha/promcons.c arch/alpha/alpha/promcons.c
> index 9efabd3bf1c..b872f6e3931 100644
> --- arch/alpha/alpha/promcons.c
> +++ arch/alpha/alpha/promcons.c
> @@ -100,7 +100,7 @@ promopen(dev, flag, mode, p)
>   error = (*linesw[tp->t_line].l_open)(dev, tp, p);
>   if (error == 0 && setuptimeout) {
>   timeout_set(&prom_to, promtimeout, tp);
> - timeout_add(&prom_to, 1);
> + timeout_add_msec(&prom_to, 10);
>   }
>   return error;
>  }
> @@ -220,7 +220,7 @@ promtimeout(v)
>   if (tp->t_state & TS_ISOPEN)
>   (*linesw[tp->t_line].l_rint)(c, tp);
>   }
> - timeout_add(&prom_to, 1);
> + timeout_add_msec(&prom_to, 10);
>  }
>  
>  struct tty *
> diff --git arch/amd64/isa/clock.c arch/amd64/isa/clock.c
> index db516d9ecde..9d5934e6817 100644
> --- arch/amd64/isa/clock.c
> +++ arch/amd64/isa/clock.c
> @@ -326,7 +326,7 @@ rtcstart(void)
>   mc146818_write(NULL, MC_REGB, MC_REGB_24HR | MC_REGB_PIE);
>  
>   /*
> - * On a number of i386 systems, the rtc will fail to start when booting
> + * On a number of amd64 systems, the rtc will fail to start when booting
>   * the system. This is due to us missing to acknowledge an interrupt
>   * during early stages of the boot process. If we do not acknowledge
>   * the interrupt, the rtc clock will not generate further interrupts.
> @@ -334,7 +334,7 @@ rtcstart(void)
>   * to drain any un-acknowledged rtc interrupt(s).
>   */
>   timeout_set(&rtcdrain_timeout, rtcdrain, (void *)&rtcdrain_timeout);
> - timeout_add(&rtcdrain_timeout, 1);
> + timeout_add_msec(&rtcdrain_timeout, 10);
>  }
>  
>  void
> diff --git arch/amd64/pci/pchb.c arch/amd64/pci/pchb.c
> index 6e599d7be4a..80b5ada1cb4 100644
> --- arch/amd64/pci/pchb.c
> +++ arch/amd64/pci/pchb.c
> @@ -332,7 +332,7 @@ pchb_rnd(void *v)
>   }
>   }
>  
> - timeout_add(&sc->sc_rng_to, 1);
> + timeout_add(&sc->sc_rng_to, 10);
>  }
>  
>  void
> diff --git dev/ic/vga.c dev/ic/vga.c
> index 74cc3e07bf8..2cebb65d2d5 100644
> --- dev/ic/vga.c
> +++ dev/ic/vga.c
> @@ -765,7 +765,7 @@ vga_show_screen(void *v, void *cookie, int waitok, void (*cb)(void *, int, int),
>   if (cb) {
>   timeout_set(&vc->vc_switch_timeout,
>      (void(*)(void *))vga_doswitch, vc);
> - timeout_add(&vc->vc_switch_timeout, 0);
> + timeout_add(&vc->vc_switch_timeout, 10);
>   return (EAGAIN);
>   }
>  
> diff --git net/if_pfsync.c net/if_pfsync.c
> index 8d842e48466..dc8d2f41466 100644
> --- net/if_pfsync.c
> +++ net/if_pfsync.c
> @@ -2318,7 +2318,7 @@ pfsync_bulk_start(void)
>   sc->sc_bulk_last = sc->sc_bulk_next;
>  
>   pfsync_bulk_status(PFSYNC_BUS_START);
> - timeout_add(&sc->sc_bulk_tmo, 0);
> + timeout_add_msec(&sc->sc_bulk_tmo, 0);
>   }
>  }
>  
>
>
>

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Mark Kettenis
In reply to this post by Amit Kulkarni
> Date: Sun, 6 Jan 2019 21:46:01 -0600
> From: Amit Kulkarni <[hidden email]>
>
> > > Even on amd64, I won't be able to test, because of missing hardware.
> > > If you think something is wrong, please will you let me have your
> > > feedback?
> >
> > I'm a bit stunned at the zeal to push untested diffs into the tree
> >
> > (you didn't ask anyone to test it for you)
>
> I requested for critical review or feedback in the initial email, to
> know if I am going down the right path. If the approach is ok, then
> I was planning to sit down to try converting all the straightforward
> of timeout_add() to timeout_add_msec() calls in the tree, then a
> review again, before finally requesting a test run.

You can't do these conversions without understanding the code.  In
particular, timeout_add(t, 1) could mean sleep for 1/HZ seconds *or*
sleep as short as possible.  It is actually most likely the latter, in
which case coverting to timeout_add_msec() makes no sense.

Reply | Threaded
Open this post in threaded view
|

Re: replacing timeout_add() with timeout_add_msec()

Amit Kulkarni
In reply to this post by Claudio Jeker
> You started to convert the wrong timeout_add() calls. Doing a blind
> timeout_add() to timeout_add_msec() conversion especially on calls with 0
> or 1 tick sleep time is the wrong approach.
> The right approach is to identify calls that do sleep for a well defined time
> (1sec, 50ms or whatever) and convert those. Most of the timeouts you
> changes are not in that category. First I would look at timeouts that have
> a multiplication with hz in them since those wait for a specific time.

Thanks for the feedback Claudio and Mark!

Here is a new diff based on your suggestions, looking for feedback, no tests requested yet. I assumed in below diff that 1hz == 1sec (confirmed in timeout_add_sec() in /sys/kern/kern_timeout.c), and converted some multiplication and some divisions, all related to hz. Did some conversions in comments also, because in future they might be used.
-- amit


diff --git sys/arch/armv7/exynos/exuart.c sys/arch/armv7/exynos/exuart.c
index 4b0588750ea..15086bc5976 100644
--- sys/arch/armv7/exynos/exuart.c
+++ sys/arch/armv7/exynos/exuart.c
@@ -283,7 +283,7 @@ exuart_intr(void *arg)
  if (p >= sc->sc_ibufend) {
  sc->sc_floods++;
  if (sc->sc_errors++ == 0)
- timeout_add(&sc->sc_diag_tmo, 60 * hz);
+ timeout_add_sec(&sc->sc_diag_tmo, 60);
  } else {
  *p++ = c;
  if (p == sc->sc_ibufhigh && ISSET(tp->t_cflag, CRTSCTS))
@@ -710,7 +710,7 @@ exuartclose(dev_t dev, int flag, int mode, struct proc *p)
  /* tty device is waiting for carrier; drop dtr then re-raise */
  //CLR(sc->sc_ucr3, EXUART_CR3_DSR);
  //bus_space_write_2(iot, ioh, EXUART_UCR3, sc->sc_ucr3);
- timeout_add(&sc->sc_dtr_tmo, hz * 2);
+ timeout_add_sec(&sc->sc_dtr_tmo, 2);
  } else {
  /* no one else waiting; turn off the uart */
  exuart_pwroff(sc);
diff --git sys/arch/sparc64/dev/fd.c sys/arch/sparc64/dev/fd.c
index 8d548062f83..654d8c95524 100644
--- sys/arch/sparc64/dev/fd.c
+++ sys/arch/sparc64/dev/fd.c
@@ -1632,7 +1632,7 @@ loop:
  fdc->sc_state = RECALCOMPLETE;
  if (fdc->sc_flags & FDC_NEEDHEADSETTLE) {
  /* allow 1/30 second for heads to settle */
- timeout_add(&fdc->fdcpseudointr_to, hz / 30);
+ timeout_add_msec(&fdc->fdcpseudointr_to, 33);
  return (1); /* will return later */
  }
 
diff --git sys/dev/fdt/imxuart.c sys/dev/fdt/imxuart.c
index 84c7eb5aee6..c2fd7e4a6d3 100644
--- sys/dev/fdt/imxuart.c
+++ sys/dev/fdt/imxuart.c
@@ -228,7 +228,7 @@ imxuart_intr(void *arg)
  if (p >= sc->sc_ibufend) {
  sc->sc_floods++;
  if (sc->sc_errors++ == 0)
- timeout_add(&sc->sc_diag_tmo, 60 * hz);
+ timeout_add_sec(&sc->sc_diag_tmo, 60);
  } else {
  *p++ = c;
  if (p == sc->sc_ibufhigh &&
@@ -457,7 +457,7 @@ imxuart_softint(void *arg)
  if (ISSET(c, IMXUART_RX_OVERRUN)) {
  sc->sc_overflows++;
  if (sc->sc_errors++ == 0)
- timeout_add(&sc->sc_diag_tmo, 60 * hz);
+ timeout_add_sec(&sc->sc_diag_tmo, 60);
  }
  /* This is ugly, but fast. */
 
@@ -629,7 +629,7 @@ imxuartclose(dev_t dev, int flag, int mode, struct proc *p)
  /* tty device is waiting for carrier; drop dtr then re-raise */
  CLR(sc->sc_ucr3, IMXUART_CR3_DSR);
  bus_space_write_2(iot, ioh, IMXUART_UCR3, sc->sc_ucr3);
- timeout_add(&sc->sc_dtr_tmo, hz * 2);
+ timeout_add_sec(&sc->sc_dtr_tmo, 2);
  } else {
  /* no one else waiting; turn off the uart */
  imxuart_pwroff(sc);
diff --git sys/dev/ic/if_wi_hostap.c sys/dev/ic/if_wi_hostap.c
index 64e3c10f3f5..155a391e7f9 100644
--- sys/dev/ic/if_wi_hostap.c
+++ sys/dev/ic/if_wi_hostap.c
@@ -410,7 +410,7 @@ wihap_sta_timeout(void *v)
 
  /* Add wihap timeout if we have not already done so. */
  if (!timeout_pending(&whi->tmo))
- timeout_add(&whi->tmo, hz / 10);
+ timeout_add_msec(&whi->tmo, 100);
 
  splx(s);
 }
diff --git sys/dev/ic/pluart.c sys/dev/ic/pluart.c
index 0f024c0ad34..19bbb76f4a6 100644
--- sys/dev/ic/pluart.c
+++ sys/dev/ic/pluart.c
@@ -225,7 +225,7 @@ pluart_intr(void *arg)
  if (p >= sc->sc_ibufend) {
  sc->sc_floods++;
  if (sc->sc_errors++ == 0)
- timeout_add(&sc->sc_diag_tmo, 60 * hz);
+ timeout_add_sec(&sc->sc_diag_tmo, 60);
  } else {
  *p++ = c;
  if (p == sc->sc_ibufhigh && ISSET(tp->t_cflag, CRTSCTS)) {
@@ -441,7 +441,7 @@ pluart_softint(void *arg)
  if (ISSET(c, IMXUART_RX_OVERRUN)) {
  sc->sc_overflows++;
  if (sc->sc_errors++ == 0)
- timeout_add(&sc->sc_diag_tmo, 60 * hz);
+ timeout_add_sec(&sc->sc_diag_tmo, 60);
  }
  */
  /* This is ugly, but fast. */
@@ -618,7 +618,7 @@ pluartclose(dev_t dev, int flag, int mode, struct proc *p)
  /* tty device is waiting for carrier; drop dtr then re-raise */
  //CLR(sc->sc_ucr3, IMXUART_CR3_DSR);
  //bus_space_write_4(iot, ioh, IMXUART_UCR3, sc->sc_ucr3);
- timeout_add(&sc->sc_dtr_tmo, hz * 2);
+ timeout_add_sec(&sc->sc_dtr_tmo, 2);
  } else {
  /* no one else waiting; turn off the uart */
  pluart_pwroff(sc);
diff --git sys/dev/ic/w83l518d_sdmmc.c sys/dev/ic/w83l518d_sdmmc.c
index 76c7a10c8d6..7c83510bed3 100644
--- sys/dev/ic/w83l518d_sdmmc.c
+++ sys/dev/ic/w83l518d_sdmmc.c
@@ -578,7 +578,7 @@ wb_sdmmc_intr(struct wb_softc *wb)
     "\5CRC\6FIFO\7CARD\010PENDING");
 
  if (val & WB_INT_CARD)
- timeout_add(&wb->wb_sdmmc_to, hz / 4);
+ timeout_add_msec(&wb->wb_sdmmc_to, 250);
 
  return 1;
 }
diff --git sys/dev/pci/pccbb.c sys/dev/pci/pccbb.c
index 642e7879b7f..b4980347f0b 100644
--- sys/dev/pci/pccbb.c
+++ sys/dev/pci/pccbb.c
@@ -1234,7 +1234,7 @@ cb_pcmcia_poll(void *arg)
  u_int32_t spsr;       /* socket present-state reg */
 
  timeout_set(&cb_poll_timeout, cb_pcmcia_poll, arg);
- timeout_add(&cb_poll_timeout, hz / 10);
+ timeout_add_msec(&cb_poll_timeout, 100);
  switch (poll->level) {
  case IPL_NET:
  s = splnet();
diff --git sys/net/if_pppoe.c sys/net/if_pppoe.c
index e55316351fd..9e03310714b 100644
--- sys/net/if_pppoe.c
+++ sys/net/if_pppoe.c
@@ -1346,7 +1346,7 @@ pppoe_tlf(struct sppp *sp)
  * function and defer disconnecting to the timeout handler.
  */
  sc->sc_state = PPPOE_STATE_CLOSING;
- timeout_add(&sc->sc_timeout, hz / 50);
+ timeout_add_msec(&sc->sc_timeout, 20);
 }
 
 static void