Add a manual page for the lockstat provider
Details
- Reviewers
markj sson - Group Reviewers
manpages DTrace - Commits
- rS319803: Manual page for the DTrace lockstat provider
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 9790 Build 10227: arc lint + arc unit
Event Timeline
"Tracepoints exist for several types of kernel lockng primitives" -> locking primitives?
Other than that didn't see any problems. Thanks for writing this!
Note: my commit bit is currently not active, so take this review for what it's worth. ;-)
share/man/man4/dtrace_lockstat.4 | ||
---|---|---|
62 | New sentences are supposed to start on new lines. | |
63 | *locking This should also reference the FreeBSD names for these primitives instead: mutex(9), rwlock(9), sx(9). | |
65 | I don't understand the purpose of this sentence - were you intending to include a reference to lockstat(1). | |
73 | s/tracepoint/probe/ The acquire and release probes will in general have many tracepoints - one for each inlined lock/unlock in the kernel. | |
75 | This should be referred to as a MTX_DEF mutex. Most of the locking primitives will do adaptive spinning, so "adaptive lock" is confusing. | |
83 | s/tracepoint/probe/ | |
89 | This isn't true - the probe fires after the mutex is acquired. It indicates that the thread _had_ spinned while waiting for the mutex. | |
92 | The second argument is the number of nanoseconds spent spinning, not the spin count. | |
96 | s/tracepoint/probe/ | |
97 | Not true - it fires if the thread took itself off the CPU while waiting for a lock to become free. In particular, adaptive-block will not fire if the thread spinned for a short period for the lock and then successfully acquired it. The adaptive-block probe fires only after the lock has been successfully acquired, and in particular, after the adaptive-acquire probe fires. It would be good to note this as well. | |
101 | It's worth noting that this doesn't include the time spent spinning for the lock. | |
103 | s/spin lock/spin mutex/ | |
111 | s/tracepoint/probe/ | |
120 | Same comments as for adaptive-spin. | |
130 | s/tracepoint/probe/ | |
135 | Same comment as for adaptive-block. The -block probes only fire when the thread takes itself off the CPU by blocking on a turnstile. | |
146 | Same comments as for adaptive-spin and spin-spin. Also, this probe has more than two arguments. Argument 3 is 1 if we were spinning for a read lock, else 0 if we were spinning for the write lock. Argument 4 is 1 if we were waiting for a reader to release the lock, else 0 if we were waiting for a writer to release the lock. Argument 5 is the number of readers that held the lock when we started spinning; in particular, argument 5 is non-zero only if argument 4 is 1. | |
150 | s/tracepoint/probe/ | |
156 | s/tracepoint/probe/ | |
162 | All of the earlier comments for rw locks apply to sx locks too. | |
205 | ... spins on a thread lock. | |
209 | It's the number of nanoseconds spent spinning. Threads never block (in the sense of taking themselves off the CPU) while waiting for a thread lock. | |
213 | lockstatus(9) is for lockmgr locks, which don't have lockstat probes. Did you mean locking(9)? |
share/man/man4/dtrace_lockstat.4 | ||
---|---|---|
65 | I'm trying to point out that each type of lock (adaptive, rw, etc.) has similar probe point names. |
head/share/man/man4/dtrace_lockstat.4 | ||
---|---|---|
56 ↗ | (On Diff #29443) | .Nm is for setting the name of the thing documented by the man page. Normally, it is only set once at the beginning and then used without a value later. Here and later in this man page, it appears to be used for markup, but that's not really what it does. Depends on what you are trying to do, but probably should be a different macro. Maybe .Cm ? |
61 ↗ | (On Diff #29443) | Should this be "a DTrace probe" or "probes"? |
66 ↗ | (On Diff #29443) | This is a bit passive and implies that the attempt was not entirely successful. It's kind of implied that there was an attempt anyway. Also, not clear on what "regular" means in this context. How about just: The interface to the .Cm lockstat provider is simple and easy to understand. (See above about the .Cm, not sure if that's the right thing to use.) |
74 ↗ | (On Diff #29443) | Suspect this would read better as "has acquire and release *probes which expose*", but it's a judgement call. |
80 ↗ | (On Diff #29443) | s/that is// |
84 ↗ | (On Diff #29443) | s/whenever/when/ |
86 ↗ | (On Diff #29443) | s/that is// |
98 ↗ | (On Diff #29443) | s/whenever thread/when a thread/ |
109 ↗ | (On Diff #29443) | s/Whenever/When/ |
113 ↗ | (On Diff #29443) | s/that is// |
119 ↗ | (On Diff #29443) | s/that is// |
123 ↗ | (On Diff #29443) | s/waiting/while waiting/ |
125 ↗ | (On Diff #29443) | s/the time/time/ |
132 ↗ | (On Diff #29443) | s/that is// |
136 ↗ | (On Diff #29443) | s/whenever/when/ |
140 ↗ | (On Diff #29443) | s/whenever/when/ |
146 ↗ | (On Diff #29443) | s/were// |
148 ↗ | (On Diff #29443) | s/we were/it was/ |
149 ↗ | (On Diff #29443) | s/we were/it was/ |
150 ↗ | (On Diff #29443) | s/we were/it was/ |
153 ↗ | (On Diff #29443) | s/we/it/ Break the sentence instead of using a semicolon: it started spinning. In particular, argument 5 is non-zero only |
162 ↗ | (On Diff #29443) | s/that were// |
166 ↗ | (On Diff #29443) | s/whenever/when/ |
169 ↗ | (On Diff #29443) | s/that is// |
172 ↗ | (On Diff #29443) | s/whenever/when/ |
175 ↗ | (On Diff #29443) | s/that is// |
177 ↗ | (On Diff #29443) | s/Whenever/When/ Needs a comma, too: When a shared-exclusive lock is acquired, the That could be avoided by flipping the sentence: The .Fn lockstat:::sx-acquire probe fires when a shared-exclusive lock is acquired. |
181 ↗ | (On Diff #29443) | s/that is// |
185 ↗ | (On Diff #29443) | s/whenever/when/ |
187 ↗ | (On Diff #29443) | s/that is// |
191 ↗ | (On Diff #29443) | s/whenever/when/ |
197 ↗ | (On Diff #29443) | s/was were/was/ |
199 ↗ | (On Diff #29443) | s/we were/it was/ |
200 ↗ | (On Diff #29443) | s/we were/it was/ |
201 ↗ | (On Diff #29443) | There are two spaces before the zero. |
204 ↗ | (On Diff #29443) | s/we/it/ Copypasta for the semicolon, which are usually best reserved for types of writing other than documentation. Short, declarative sentences without pauses are best. |
209 ↗ | (On Diff #29443) | s/off of/off/ |
213 ↗ | (On Diff #29443) | s/that were// |
220 ↗ | (On Diff #29443) | s/that is// |
224 ↗ | (On Diff #29443) | s/whenever/when/ |
227 ↗ | (On Diff #29443) | s/that is// |
231 ↗ | (On Diff #29443) | s/whenever/when/ |
245 ↗ | (On Diff #29443) | Use .Fx for FreeBSD: The .Fx implementation of the .Cm lockstat (but see above, not sure about .Cm) |