Page MenuHomeFreeBSD

Move vm map consistency checking to under DIAGNOSTIC.
ClosedPublic

Authored by markj on Apr 22 2019, 2:23 AM.
Tags
None
Referenced Files
F103303947: D19999.diff
Sat, Nov 23, 7:36 AM
Unknown Object (File)
Wed, Nov 6, 1:51 PM
Unknown Object (File)
Thu, Oct 31, 4:39 PM
Unknown Object (File)
Tue, Oct 29, 1:46 AM
Unknown Object (File)
Sep 25 2024, 8:25 AM
Unknown Object (File)
Sep 24 2024, 11:32 PM
Unknown Object (File)
Sep 24 2024, 10:59 PM
Unknown Object (File)
Sep 24 2024, 2:53 AM
Subscribers

Details

Summary

The overhead is too high for INVARIANTS. Add a sysctl to optionally
disable the checks.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/vm/vm_map.c
673 ↗(On Diff #56460)

This seems to imply that if INVARIANTS is not set, DIAGNOSTIC is irrelevant. Picking a random file with DIAGNOSTIC in it (kern_clock.c) suggest that this isn't the usual practice. So the #endif for INVARIANTS ought to be before the #ifdef for DIAGNOSTIC, among other rearrangements.

sys/vm/vm_map.c
673 ↗(On Diff #56460)

So now I've read your related email. I yield. I won't pretend to know what exactly INVARIANTS and DIAGNOSTICS are supposed to mean.

Wouldn't you want to include the consistent() function if *either* INVARIANTS or DIAGNOSTICS were defined, and then default vmmap_check to 0 or 1 depending on the settings of INVARIANTS and DIAGNOSTICS?

sys/vm/vm_map.c
673 ↗(On Diff #56460)

I think that's true in practice: DIAGNOSTIC is basically meant to be an extension of INVARIANTS. The difference is that we want INVARIANTS kernels to be performant enough to be usable as a general-purpose OS, whereas DIAGNOSTIC is mainly useful for running tests or for debugging a specific issue. I can't imagine a scenario where one would configure DIAGNOSTIC without INVARIANTS. It might be more correct to compile the consistency checking under INVARIANT_SUPPORT rather than INVARIANTS, though. (See sys/conf/NOTES for a definition.)

I'll note also that the pattern of

#ifdef DIAGNOSTIC
KASSERT(...);
#endif

is not uncommon, but KASSERT is a no-op unless INVARIANTS is configured.

This revision is now accepted and ready to land.Apr 22 2019, 9:55 AM
This revision was automatically updated to reflect the committed changes.