On Sun, Aug 09, 2020 at 05:33:58PM +0200, Mark Kettenis wrote: > > Date: Sun, 9 Aug 2020 10:02:38 -0500 > > From: Scott Cheloha <scottchel...@gmail.com> > > > > On Sun, Aug 09, 2020 at 04:43:24PM +0200, Mark Kettenis wrote: > > > > 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? > > > > The convention is to use "I" for immutable variables. We do it > > everywhere. I don't think we should buck convention here. > > > > I also proposed using "i" in a prior patch to annotate these > > variables, but mpi@ said it was too close to "I". Also, it's a global > > lock, and we have settled on only annotate global locks with capital > > letters. > > > > If you don't want to rename the mutex I guess we could use "T" for > > "timer". We use "T" for other global locks (tc_lock, timeout_mutex) > > but not in this context. > > > > However, there are only so many letters. Eventually this scheme will > > run afoul of that limitation. An idea I had re. the letter shortage > > was to use two letters where necessary. So instead of "I" you could > > use "It" for "itimer". We annotate locking hierarchies with commas so > > there isn't an ambiguity when reading it. > > > > For example, if the code for writing a hypothetical "ps_foo" process > > struct member was: > > > > KERNEL_LOCK(); > > mtx_enter(&itimer_mtx); > > ps.ps_foo = 10; > > mtx_leave(&itimer_mtx); > > KERNEL_UNLOCK(); > > > > You could annotate it like this: > > > > /* > > * Locks used to protect process struct members: > > * > > * It itimer_mtx > > * K kernel lock > > */ > > struct process { > > /* [...] */ > > int ps_foo; /* [K,It] per-process foobar */ > > /* [...] */ > > }; > > > > anton@, mpi@: is that too radical or easily misread? > > > > Sorry if this all seems fussy, but I'd like to get this right the > > first time. > > 'T' is fine with me. But I'm clearly not an authority here. Anyway, > renaming variables because you don't have a matching letter to > annotate the lock doesn't feel right.
I'm also fine with T.