> Date: Sat, 8 Aug 2020 19:46:14 -0500
> From: Scott Cheloha <scottchel...@gmail.com>
>
> Hi,
>
> I want to annotate the locking for the per-process interval timers.
>
> In the process struct, the ITIMER_REAL itimerspec and the ps_itimer_to
> timeout are protected by the kernel lock. These should be annotated
> with "K", right?
>
> Also in the process struct, the ITIMER_VIRTUAL and ITIMER_PROF
> itimerspecs are protected by the global itimer_mtx.
>
> However, I don't think "itimer_mtx" isn't the best name for it, as it
> doesn't protect state for *all* per-process interval timers. Just the
> virtual ones.
>
> Could I rename the mutex to "virtual_itimer_mtx"? Then I can annotate
> the state protected by it with "V", as shown here in this patch.
That's quite a long variable name though. And it also protects
ITIMER_PROF. So I'd say the name would be at least as misleading as
the current one and perhaps even more so. You can just use "I" as the
annotation perhaps?
> Preferences? ok?
>
> Index: kern/kern_time.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.134
> diff -u -p -r1.134 kern_time.c
> --- kern/kern_time.c 8 Aug 2020 01:01:26 -0000 1.134
> +++ kern/kern_time.c 9 Aug 2020 00:41:10 -0000
> @@ -488,7 +488,13 @@ out:
> }
>
>
> -struct mutex itimer_mtx = MUTEX_INITIALIZER(IPL_CLOCK);
> +/*
> + * Global virtual interval timer mutex.
> + *
> + * Protects state for the per-process ITIMER_VIRTUAL and ITIMER_PROF
> + * interval timers.
> + */
> +struct mutex virtual_itimer_mtx = MUTEX_INITIALIZER(IPL_CLOCK);
>
> /*
> * Get value of an interval timer. The process virtual and
> @@ -526,10 +532,10 @@ sys_getitimer(struct proc *p, void *v, r
> return (EINVAL);
> itimer = &p->p_p->ps_timer[which];
> memset(&aitv, 0, sizeof(aitv));
> - mtx_enter(&itimer_mtx);
> + mtx_enter(&virtual_itimer_mtx);
> TIMESPEC_TO_TIMEVAL(&aitv.it_interval, &itimer->it_interval);
> TIMESPEC_TO_TIMEVAL(&aitv.it_value, &itimer->it_value);
> - mtx_leave(&itimer_mtx);
> + mtx_leave(&virtual_itimer_mtx);
>
> if (which == ITIMER_REAL) {
> struct timeval now;
> @@ -604,9 +610,9 @@ sys_setitimer(struct proc *p, void *v, r
> }
> pr->ps_timer[ITIMER_REAL] = aits;
> } else {
> - mtx_enter(&itimer_mtx);
> + mtx_enter(&virtual_itimer_mtx);
> pr->ps_timer[which] = aits;
> - mtx_leave(&itimer_mtx);
> + mtx_leave(&virtual_itimer_mtx);
> }
>
> return (0);
> @@ -681,20 +687,20 @@ itimerdecr(struct itimerspec *itp, long
>
> NSEC_TO_TIMESPEC(nsec, &decrement);
>
> - mtx_enter(&itimer_mtx);
> + mtx_enter(&virtual_itimer_mtx);
> timespecsub(&itp->it_value, &decrement, &itp->it_value);
> if (itp->it_value.tv_sec >= 0 && timespecisset(&itp->it_value)) {
> - mtx_leave(&itimer_mtx);
> + mtx_leave(&virtual_itimer_mtx);
> return (1);
> }
> if (!timespecisset(&itp->it_interval)) {
> timespecclear(&itp->it_value);
> - mtx_leave(&itimer_mtx);
> + mtx_leave(&virtual_itimer_mtx);
> return (0);
> }
> while (itp->it_value.tv_sec < 0 || !timespecisset(&itp->it_value))
> timespecadd(&itp->it_value, &itp->it_interval, &itp->it_value);
> - mtx_leave(&itimer_mtx);
> + mtx_leave(&virtual_itimer_mtx);
> return (0);
> }
>
> Index: sys/proc.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/proc.h,v
> retrieving revision 1.297
> diff -u -p -r1.297 proc.h
> --- sys/proc.h 6 Jul 2020 13:33:09 -0000 1.297
> +++ sys/proc.h 9 Aug 2020 00:41:11 -0000
> @@ -150,9 +150,11 @@ struct unveil;
> /*
> * Locks used to protect struct members in this file:
> * a atomic operations
> + * K kernel lock
> * m this process' `ps_mtx'
> * p this process' `ps_lock'
> * R rlimit_lock
> + * V virtual_itimer_mtx
> */
> struct process {
> /*
> @@ -216,7 +218,8 @@ struct process {
> struct rusage *ps_ru; /* sum of stats for dead threads. */
> struct tusage ps_tu; /* accumulated times. */
> struct rusage ps_cru; /* sum of stats for reaped children */
> - struct itimerspec ps_timer[3]; /* timers, indexed by ITIMER_* */
> + struct itimerspec ps_timer[3]; /* [K] ITIMER_REAL timer */
> + /* [V] ITIMER_{PROF,VIRTUAL} timers */
> struct timeout ps_rucheck_to; /* [] resource limit check timer */
> time_t ps_nextxcpu; /* when to send next SIGXCPU, */
> /* in seconds of process runtime */
> @@ -269,7 +272,7 @@ struct process {
> int ps_refcnt; /* Number of references. */
>
> struct timespec ps_start; /* starting uptime. */
> - struct timeout ps_realit_to; /* real-time itimer trampoline. */
> + struct timeout ps_realit_to; /* [K] ITIMER_REAL timeout */
> };
>
> #define ps_session ps_pgrp->pg_session
>
>
