OK, I'll mark them as mpsafe in a commit.

Use full context diff.

Commited as rS358333

Sync with rS358330 and split sys/dev/random bits to a separate review for so@ approval.

That it'll not be run more than one at a time (from userspace).

No, because the sysctl handler was running under Giant implicitly (as it wasn't marked as MPSAFE).

Committed as rS358286.

That is the plan - document current state now, review and fix what have to be fixed later. The flag itself doesn't change anything, it's just a more explicit way of showing that the handler may be not-MPSAFE.

Address comments and update to rS358167.

Use correct diff.

Update diff.

Add missed file.

Address comments.

Address comments.

Upload the correct patch.

Address comments.

Although it'd be nice to decode the first argument in ktrace/kdump case just like truss does it here.

Address comments and fix many more obvious cases.

In D23718#520881, @melifaro wrote:

Generic comment: maybe it would be better to commit all non-leaf nodes CTLFLAG_MPSAFE changes and let the reviews look into the real cases where CTLFLAG_NEEDGIANT is necessary?

This work has been split for several smaller batches: D23378, D23625 D23626 D23627 D23628 D23629 D23630 D23631 D23632 D23633 D23634 D23635 D23636 D23637 D23638 D23639 D23640, D23678 and D23718

That was committed in rS357972 and should be auto-closed.

This sysctl is MPSAFE.

The amount of work to change the SYSCTL_NODEs with handlers into SYSCTL_PROCs and remove the handler from SYSCTL_NODEs will cause a lot more code changes.
Yes it'll limit the amount of CTLFLAG_MPSAFE being added (and later removed) to SYSCTL_NODEs without handlers.
But the question of should SYSCTL_NODEs be allowed to have handlers is a very different one, and one may suggest it's a bug or just a rarely used feature.
I would want to concentrate on the current approach: mark SYSCTL_NODES without handlers as MPSAFE, nad masrk SYSCTL_NODES and SYSCT_PROCS with handlers that are not marked as MPSAFE now as NEEDGIANT and continue the work pushing the Giant off the sysctl. Fixing semantics of SYSCTL_NODE can be done later and has nothing to do with the MPSAFE/NEEDGIANT work.

For documentation only, because committing this breaks the other patches and this review was created for brainstorming, but the whole discussion moved to IRC.

Marking them as NEEDGIANT is default to document the current state - all handlers not marked as MPSAFE are implicitly run under Giant. The new flag makes that a bit more explicit and allows for reviews like this to actually assess if that's really needed.
I'll take a look if we have any precedent on how to deal with sysctls operating on devices that can disappear. All other can probably by marked as MPSAFE, I'll update the patch.

SYSCTL_*_NODE is MPSAFE if the handler is NULL (what is true for most of them) or if the handler takes care of locking internally.

Mark the nodes as MPSAFE.

Mark the nodes as MPSAFE.

These nodes are MPSAFE.

Because they weren't marked as MPSAFE before :-) but after checking I think we can assume they are MPSAFE.

That is exactly the reason: sysctl's that weren't before marked as MPSAFE are marked as NEEDGIANT, but if you're happy with current locking (or lack of it if unneeded) I can mark them as MPSAFE.