uma: multipage chicken switch
ClosedPublic
Actions

Authored by rlibby on Feb 3 2020, 5:59 PM.

Details

Reviewers

jeff
markj

Commits

rS357550: uma: multipage chicken switch

Test Plan

OK set vm.multipage_slabs=0

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 29168
Build 27101: arc lint + arc unit

sys/vm/uma_core.c
367	Do you actually need the TUNABLE_INT()? I thought that was for fetching tunables inline. CTLFLAG_RDTUN ought to be sufficient. I'd suggest putting this under debug, assuming your intent is that this only be used for debugging regressions.

sys/vm/uma_core.c
367	My understanding is that the sysctl method for tunables doesn't work until SI_SUB_KMEM, which is after SI_SUB_VM where we do uma_startup1 with the startup zones. (I don't think there's a particularly good reason for this, it seems in principle like it could be fixed.) Yes, I expect for debugging regression. Okay, "debug.uma_multipage_slabs"?

markj added inline comments.

sys/vm/uma_core.c
367	I see, I think you're right. I also can't see a reason it couldn't be done earlier. Maybe "debug.vm.uma_multipage_slabs", though the debug OID is already kind of a mess. Either is fine with me.

This revision is now accepted and ready to land.Feb 3 2020, 6:31 PM

sys/vm/uma_core.c
367	Adding a debug.vm node sounds good to me. I can move the sysctls in D20714 under there too.

sys/vm/uma_core.c
367	Ah, I see we have vm.debug (not debug.vm) already, but under INVARIANTS. I can raise that out of invariants, and move it to uma_dbg.c. Do you think debug.vm is the better location in the sysctl tree?

sys/vm/uma_core.c
367	I think debug.vm or even debug.vm.uma is slightly more consistent with most other subsystems. I also don't like that the sysctls under vm.debug are UMA-specific but don't contain UMA in the name. That said, I don't want to bikeshed, so please feel free to put them wherever you think it makes sense. I just think having "debug" somewhere in the name would be a good idea.

rlibby added inline comments.

sys/vm/uma_core.c
367	No worries about bikeshedding, I don't mind cleaning this up and it's good to have a consistent idea of where we're headed. That said, I propose: For now, commit this as "vm.debug.uma_multipage_slabs", i.e. under the existing node. "Soon", rename "vm.debug" to "debug.vm". (Or "debug.vm.uma"? TBD.) And also move the various uma_dbg stuff in uma_core.c to uma_dbg.c.