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