Hello,
On 10/01/20(Fri) 20:47, [email protected] wrote:
> this diff changes timeout_add(9) to timeout_add_msec(9).
> Since the changes are fairly short, I took the liberty to put
> all diffs of sys/kern/ into this mail.
> If you want me to send indiviual mails please say so.
I would be delighted if you could come up with an explanation of why
the calls you changed where using a 1 tick idiom. Those changes are
the most complicated, that's why I left them for the end.
> While there I zapped two whitespaces.
I'd suggest to leave that for another diff.
> There are a couple multiplications by 10 in the diffs,
> would it be of interest to have a function like NetBSDs
>
> #define mstohz(ms) ((unsigned int)((ms + 0ul) * hz / 1000ul))
> or
> #define hztoms(t) ((unsigned int)(((t) + 0ul) * 1000ul / hz))
That wouldn't help as our underlying goal is to stop using ticks as a
time unit which involves stop referencing `hz'.
So any function that use `hz, even in an abstracted form, isn't the way
to go. That's why the question "What is the purpose of the timeouts?"
interesting :o)
Comments below ;)
> diff --git sys/kern/kern_event.c sys/kern/kern_event.c
> index f0224b6ea2b..9a5009e54ee 100644
> --- sys/kern/kern_event.c
> +++ sys/kern/kern_event.c
> @@ -359,7 +359,7 @@ filt_timer_timeout_add(struct knote *kn)
> /* Remove extra tick from tvtohz() if timeout has fired before. */
> if (timeout_triggered(to))
> tticks--;
> - timeout_add(to, (tticks > 0) ? tticks : 1);
> + timeout_add_msec(to, (tticks > 0) ? tticks * 10 : 10);
Here the code is still dealing with ticks, so the API change isn't that
much important.
> --- sys/kern/kern_sig.c
> +++ sys/kern/kern_sig.c
> @@ -1327,7 +1327,7 @@ proc_stop(struct proc *p, int sw)
> atomic_setbits_int(&pr->ps_flags, PS_STOPPED);
> atomic_setbits_int(&p->p_flag, P_SUSPSIG);
> if (!timeout_pending(&proc_stop_to)) {
> - timeout_add(&proc_stop_to, 0);
> + timeout_add_msec(&proc_stop_to, 0);
What is the intend of calling timeout_add() with a 0 argument? Does this
conversion reflect that intend?
> /*
> * We need this soft interrupt to be handled fast.
> * Extra calls to softclock don't hurt.
> diff --git sys/kern/kern_srp.c sys/kern/kern_srp.c
> index 52fb4490047..4e60ccf01f4 100644
> --- sys/kern/kern_srp.c
> +++ sys/kern/kern_srp.c
> @@ -161,7 +161,7 @@ srp_v_gc_start(struct srp_gc *srp_gc, struct srp *srp,
> void *v)
> ctx->hzrd.sh_v = v;
>
> timeout_set(&ctx->tick, srp_v_gc, ctx);
> - timeout_add(&ctx->tick, 1);
> + timeout_add_msec(&ctx->tick, 10);
The code still deals with ticks internally, why? How can we change it?
> diff --git sys/kern/kern_synch.c sys/kern/kern_synch.c
> index ae6ca786eb7..b051b86d22a 100644
> --- sys/kern/kern_synch.c
> +++ sys/kern/kern_synch.c
> @@ -407,7 +407,7 @@ sleep_setup_timeout(struct sleep_state *sls, int timo)
> if (timo) {
> KASSERT((p->p_flag & P_TIMEOUT) == 0);
> sls->sls_timeout = 1;
> - timeout_add(&p->p_sleep_to, timo);
> + timeout_add_msec(&p->p_sleep_to, timo * 10);
The calling function still uses tick, so why try to obfuscate that?
> }
> }
>
> diff --git sys/kern/kern_time.c sys/kern/kern_time.c
> index 06dabb1fdfb..da4b8c335eb 100644
> --- sys/kern/kern_time.c
> +++ sys/kern/kern_time.c
> @@ -598,7 +598,7 @@ sys_setitimer(struct proc *p, void *v, register_t *retval)
> getnanouptime(&cts);
> if (timespecisset(&aits.it_value)) {
> timo = tstohz(&aits.it_value);
> - timeout_add(&pr->ps_realit_to, timo);
> + timeout_add_msec(&pr->ps_realit_to, timo * 10);
Look at my futex diff, do we want something like TIMESPEC_TO_NSEC(9)
here?
> timespecadd(&aits.it_value, &cts, &aits.it_value);
> }
> pr->ps_timer[ITIMER_REAL] = aits;
> diff --git sys/kern/uipc_domain.c sys/kern/uipc_domain.c
> index 8cb2c77dc60..71f8c13785a 100644
> --- sys/kern/uipc_domain.c
> +++ sys/kern/uipc_domain.c
> @@ -101,8 +101,8 @@ domaininit(void)
> max_hdr = max_linkhdr + max_protohdr;
> timeout_set_proc(&pffast_timeout, pffasttimo, &pffast_timeout);
> timeout_set_proc(&pfslow_timeout, pfslowtimo, &pfslow_timeout);
> - timeout_add(&pffast_timeout, 1);
> - timeout_add(&pfslow_timeout, 1);
> + timeout_add_msec(&pffast_timeout, 10);
> + timeout_add_msec(&pfslow_timeout, 10);
Why? What was the original intend?
> }
>
> struct domain *
> diff --git sys/kern/uipc_socket.c sys/kern/uipc_socket.c
> index 58d78fbd5d4..88eaa468a56 100644
> --- sys/kern/uipc_socket.c
> +++ sys/kern/uipc_socket.c
> @@ -257,7 +257,7 @@ sofree(struct socket *so, int s)
> if (so->so_sp) {
> /* Reuse splice idle, sounsplice() has been called before. */
> timeout_set_proc(&so->so_sp->ssp_idleto, soreaper, so);
> - timeout_add(&so->so_sp->ssp_idleto, 0);
> + timeout_add_msec(&so->so_sp->ssp_idleto, 0);
What is the unit of ssp_idleto? Should that change?
> } else
> #endif /* SOCKET_SPLICE */
> {
>
