witness: actually set read-only tunables in time for witness_startup
ClosedPublic
Actions

Authored by rlibby on Tue, Jun 16, 11:19 PM.

Details

Reviewers

markj
kib
glebius

Commits

rG08180f1b613b: witness: actually set read-only tunables in time for witness_startup

Summary

SYSCTL_XXX with CTLFLAG_RDTUN and without CTLFLAG_NOFETCH should not be
used for values that are needed before SI_SUB_KLD. Otherwise they are
tuned after they are needed. Set CTLFLAG_RDTUN | CTLFLAG_NOFETCH for
the debug.witness.witness_count and debug.witness.skipspin sysctls and
add separate tunables for them, which run at SI_SUB_TUNABLES time, i.e.,
in time for witness_startup.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 73917
Build 70800: arc lint + arc unit

Event Timeline

rlibby created this revision.Tue, Jun 16, 11:19 PM

Herald added subscribers: olce, imp. · View Herald TranscriptTue, Jun 16, 11:19 PM

rlibby requested review of this revision.Tue, Jun 16, 11:19 PM

Harbormaster completed remote builds in B73917: Diff 179887.Tue, Jun 16, 11:19 PM

rlibby mentioned this in D57600: witness: add tunables debug.witness.lock_order_{data_count,hash_size}.Tue, Jun 16, 11:24 PM

rlibby added reviewers: markj, kib, glebius.Tue, Jun 16, 11:25 PM

So, why mess with the types (int -> bool and int -> u_long)? Strictly speaking it is not necessary and I can revert it if you prefer. I note though that there is no TUNABLE_UINT() and a comment for TUNABLE_INT() suggests not to use it. I don't know why this is, but just converting to bool and u_long seems fine anyway.

In D57613#1320890, @rlibby wrote:

So, why mess with the types (int -> bool and int -> u_long)? Strictly speaking it is not necessary and I can revert it if you prefer. I note though that there is no TUNABLE_UINT() and a comment for TUNABLE_INT() suggests not to use it. I don't know why this is, but just converting to bool and u_long seems fine anyway.

Converting to bool is fine IMO. sysctl_handle_bool() provides some backward compatibility for code which assumes the old type. The int -> u_long conversion is not backwards compatible but I can't imagine that it's a problem in this case.

This revision is now accepted and ready to land.Wed, Jun 17, 1:27 PM

kib accepted this revision.Wed, Jun 17, 4:37 PM

Closed by commit rG08180f1b613b: witness: actually set read-only tunables in time for witness_startup (authored by rlibby). · Explain WhyThu, Jun 18, 4:17 AM

This revision was automatically updated to reflect the committed changes.

rlibby added a commit: rG08180f1b613b: witness: actually set read-only tunables in time for witness_startup.