Page MenuHomeFreeBSD

New manual page for the DTrace lockstat provider
ClosedPublic

Authored by gnn on Jun 10 2017, 3:04 PM.

Details

Summary

Add a manual page for the lockstat provider

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gnn created this revision.Jun 10 2017, 3:04 PM
gnn updated this revision to Diff 29427.Jun 10 2017, 5:27 PM
gnn added a reviewer: DTrace.

"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. ;-)

markj added inline comments.Jun 10 2017, 8:05 PM
share/man/man4/dtrace_lockstat.4
62 ↗(On Diff #29427)

New sentences are supposed to start on new lines.

63 ↗(On Diff #29427)

*locking

This should also reference the FreeBSD names for these primitives instead: mutex(9), rwlock(9), sx(9).

65 ↗(On Diff #29427)

I don't understand the purpose of this sentence - were you intending to include a reference to lockstat(1).

73 ↗(On Diff #29427)

s/tracepoint/probe/

The acquire and release probes will in general have many tracepoints - one for each inlined lock/unlock in the kernel.

75 ↗(On Diff #29427)

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 ↗(On Diff #29427)

s/tracepoint/probe/

89 ↗(On Diff #29427)

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 ↗(On Diff #29427)

The second argument is the number of nanoseconds spent spinning, not the spin count.

96 ↗(On Diff #29427)

s/tracepoint/probe/

97 ↗(On Diff #29427)

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 ↗(On Diff #29427)

It's worth noting that this doesn't include the time spent spinning for the lock.

103 ↗(On Diff #29427)

s/spin lock/spin mutex/

111 ↗(On Diff #29427)

s/tracepoint/probe/

120 ↗(On Diff #29427)

Same comments as for adaptive-spin.

130 ↗(On Diff #29427)

s/tracepoint/probe/

135 ↗(On Diff #29427)

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 ↗(On Diff #29427)

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 ↗(On Diff #29427)

s/tracepoint/probe/

156 ↗(On Diff #29427)

s/tracepoint/probe/

162 ↗(On Diff #29427)

All of the earlier comments for rw locks apply to sx locks too.

205 ↗(On Diff #29427)

... spins on a thread lock.

209 ↗(On Diff #29427)

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 ↗(On Diff #29427)

lockstatus(9) is for lockmgr locks, which don't have lockstat probes. Did you mean locking(9)?

gnn marked 21 inline comments as done.Jun 10 2017, 8:35 PM
gnn added inline comments.
share/man/man4/dtrace_lockstat.4
65 ↗(On Diff #29427)

I'm trying to point out that each type of lock (adaptive, rw, etc.) has similar probe point names.

gnn updated this revision to Diff 29442.Jun 10 2017, 8:39 PM

Address markj's comments.

This revision was automatically updated to reflect the committed changes.
wblock added a subscriber: wblock.Jun 16 2017, 3:27 PM
wblock added inline comments.
head/share/man/man4/dtrace_lockstat.4
56

.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

Should this be "a DTrace probe" or "probes"?

66

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

Suspect this would read better as "has acquire and release *probes which expose*", but it's a judgement call.

80

s/that is//

84

s/whenever/when/

86

s/that is//

98

s/whenever thread/when a thread/

109

s/Whenever/When/

113

s/that is//

119

s/that is//

123

s/waiting/while waiting/

125

s/the time/time/

132

s/that is//

136

s/whenever/when/

140

s/whenever/when/

146

s/were//

148

s/we were/it was/

149

s/we were/it was/

150

s/we were/it was/

153

s/we/it/

Break the sentence instead of using a semicolon:

it started spinning.
In particular, argument 5 is non-zero only
162

s/that were//

166

s/whenever/when/

169

s/that is//

172

s/whenever/when/

175

s/that is//

177

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

s/that is//

185

s/whenever/when/

187

s/that is//

191

s/whenever/when/

197

s/was were/was/

199

s/we were/it was/

200

s/we were/it was/

201

There are two spaces before the zero.
s/we were/it was/

204

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

s/off of/off/

213

s/that were//

220

s/that is//

224

s/whenever/when/

227

s/that is//

231

s/whenever/when/

245

Use .Fx for FreeBSD:

The
.Fx
implementation of the
.Cm lockstat

(but see above, not sure about .Cm)