- User Since
- Mar 24 2015, 9:11 PM (261 w, 4 d)
Tue, Mar 3
Mon, Mar 2
Feb 27 2020
OK, I'll mark them as mpsafe in a commit.
Feb 26 2020
Use full context diff.
Commited as rS358333
Sync with rS358330 and split sys/dev/random bits to a separate review for so@ approval.
Feb 25 2020
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).
Feb 24 2020
Committed as rS358286.
Feb 21 2020
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.
Feb 20 2020
Address comments and update to rS358167.
Use correct diff.
Add missed file.
Upload the correct patch.
Feb 19 2020
Although it'd be nice to decode the first argument in ktrace/kdump case just like truss does it here.
Feb 16 2020
Address comments and fix many more obvious cases.
That was committed in rS357972 and should be auto-closed.
This sysctl is MPSAFE.
Feb 15 2020
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.
Feb 14 2020
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.
Feb 13 2020
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.