kern_time.c: use timeout_add_tv(9)

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

kern_time.c: use timeout_add_tv(9)

Klemens Nanni-2
Simply omit conversions from timevals to hz and use the proper API.

First one is trivial, the second one truncates the converted number of
ticks to at leat one, which timeout_add_*(9) does automatically, so stop
doing it manually.

Tested by running it on my X230.
OK?

Index: kern_time.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.120
diff -u -p -r1.120 kern_time.c
--- kern_time.c 2 Jul 2019 14:54:36 -0000 1.120
+++ kern_time.c 21 Jul 2019 01:43:34 -0000
@@ -557,7 +557,6 @@ sys_setitimer(struct proc *p, void *v, r
  struct itimerval *oitv;
  struct process *pr = p->p_p;
  int error;
- int timo;
  int which;
 
  which = SCARG(uap, which);
@@ -585,8 +584,7 @@ sys_setitimer(struct proc *p, void *v, r
  timeout_del(&pr->ps_realit_to);
  getmicrouptime(&ctv);
  if (timerisset(&aitv.it_value)) {
- timo = tvtohz(&aitv.it_value);
- timeout_add(&pr->ps_realit_to, timo);
+ timeout_add_tv(&pr->ps_realit_to, &aitv.it_value);
  timeradd(&aitv.it_value, &ctv, &aitv.it_value);
  }
  pr->ps_timer[ITIMER_REAL] = aitv;
@@ -621,18 +619,14 @@ realitexpire(void *arg)
  }
  for (;;) {
  struct timeval ctv, ntv;
- int timo;
 
  timeradd(&tp->it_value, &tp->it_interval, &tp->it_value);
  getmicrouptime(&ctv);
  if (timercmp(&tp->it_value, &ctv, >)) {
  ntv = tp->it_value;
  timersub(&ntv, &ctv, &ntv);
- timo = tvtohz(&ntv) - 1;
- if (timo <= 0)
- timo = 1;
  if ((pr->ps_flags & PS_EXITING) == 0)
- timeout_add(&pr->ps_realit_to, timo);
+ timeout_add_tv(&pr->ps_realit_to, &ntv);
  return;
  }
  }

Reply | Threaded
Open this post in threaded view
|

Re: kern_time.c: use timeout_add_tv(9)

Scott Cheloha
On Sun, Jul 21, 2019 at 04:36:00AM +0200, Klemens Nanni wrote:
> Simply omit conversions from timevals to hz and use the proper API.
>
> First one is trivial, the second one truncates the converted number of
> ticks to at leat one, which timeout_add_*(9) does automatically, so stop
> doing it manually.

Both of these are incorrect.  The first conversion doesn't account for
the ongoing tick so the initial interrupt always fires up to one tick
early.  The second conversion truncates the interval, so it will fire
a tick early until it has accumulated enough error to fire correctly.

This isn't something you can really fix out at the syscall level.  The
timeout(9) backend needs to preserve the granularity of the caller's
request or you'll always be running into these sorts of issues.  I'd
focus on converting stuff outside of sys/kern in the meantime.  When
we change the backend these issues will go away on their own.

-Scott