Details

Reviewers

markj
kib
jhb
pauamma_gundo.com
rpokala

Group Reviewers

manpages

Commits

rGd85a53128792: KASSERT(9): some updates
rG87132d1dce4b: KASSERT(9): some updates

Summary

Correct the needed headers
Make the existing example ANSI C. Don't reference a real kernel function whose implementation has changed, it is simple enough to use foo and bar as identifiers
Add a little bit of introductory text
Document the widely used MPASS macro in this page
Drop the cross-reference to config(8)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 50424
Build 47315: arc lint + arc unit

Event Timeline

mhorne created this revision.Mar 16 2023, 7:35 PM

Herald added a subscriber: imp. · View Herald TranscriptMar 16 2023, 7:35 PM

mhorne requested review of this revision.Mar 16 2023, 7:35 PM

Harbormaster completed remote builds in B50424: Diff 118945.Mar 16 2023, 7:35 PM

mhorne added a parent revision: D39130: critical(9): small updates.Mar 16 2023, 7:35 PM

mhorne added a child revision: D39132: panic(9): some updates.

mhorne added reviewers: markj, kib, jhb.Mar 16 2023, 7:39 PM

I have been wondering about MPASS() forever! :-)

share/man/man9/KASSERT.9
51
68

Handle rpokala's comments.

Harbormaster completed remote builds in B50431: Diff 118953.Mar 16 2023, 8:08 PM

In D39131#890734, @rpokala wrote:

I have been wondering about MPASS() forever! :-)

The definition I gave is not authoritative, just my best guess :)

markj added inline comments.Mar 16 2023, 8:09 PM

share/man/man9/KASSERT.9
52	I think this is a bit unclear: certain non-recoverable errors are handled with an explicit call to panic() since they apply to non-INVARIANTS kernels as well. Maybe it should be more obvious that KASSERT() is optional and only enabled in debugging kernels.
84	Traditionally we use options DIAGNOSTIC for really expensive checks (and in some old code it's used to enable some miscellaneous debug printf()s, but I think that's discouraged now). Maybe it'd be worth mentioning that?

markj added inline comments.Mar 16 2023, 8:11 PM

share/man/man9/KASSERT.9
84	Maybe one way to express this is that assertion conditions should be checkable in constant time. INVARIANTS kernels are allowed to be slow, but they must still be "usable" in some sense.

In D39131#890745, @mhorne wrote:

In D39131#890734, @rpokala wrote:

I have been wondering about MPASS() forever! :-)

The definition I gave is not authoritative, just my best guess :)

I always thought that it was "multi-processor assert" but that doesn't make a ton of sense (maybe it was introduced during the transition to MPSAFE kernels?) and I have no proof. Your name is better.

Minor nits fixable on commit.

share/man/man9/KASSERT.9
34	March (already!)
117

This revision is now accepted and ready to land.Mar 16 2023, 8:48 PM

Handle comments from markj and pauamma.

This revision now requires review to proceed.Mar 16 2023, 8:59 PM

Harbormaster completed remote builds in B50434: Diff 118957.Mar 16 2023, 8:59 PM

mhorne marked 5 inline comments as done.Mar 16 2023, 9:00 PM

mhorne added inline comments.

share/man/man9/KASSERT.9
117	Nice catch!

rpokala accepted this revision.Mar 16 2023, 9:03 PM

This revision is now accepted and ready to land.Mar 16 2023, 9:03 PM

kib added inline comments.Mar 17 2023, 7:01 AM

share/man/man9/KASSERT.9
76	Something should be said that the panic string is formed by sprintf-ing format and args.
95
98	This is in fact the worst example of assert. The normal approach for NULL deref checks is to left it to the MMU. Note that we do not have many a != NULL asserts in kernel. bde@ objected esp. loud against them.