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.
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
