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
Unknown Object (File)
Nov 23 2024, 7:36 AM
Unknown Object (File)
Nov 6 2024, 1:51 PM
Unknown Object (File)
Oct 31 2024, 4:39 PM
Unknown Object (File)
Oct 29 2024, 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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 23806
Build 22750: arc lint + arc unit

Event Timeline

sys/vm/vm_map.c
673

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

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

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.