Use a vm_domainset iterator in keg_fetch_slab().
ClosedPublic
Actions

Authored by markj on Oct 4 2018, 8:00 PM.

Details

Reviewers

alc
jeff
kib
gallatin

Commits

rS339686: Use a vm_domainset iterator in keg_fetch_slab().

Summary

Right now, UMA uses a hand-rolled round-robin policy. Aside from the
duplication of logic, this means that we're not following the "minskip"
policy introduced in r338507. We also want to avoid blocking in
individual domains with a WAITOK allocation. Thus, refactor
keg_fetch_slab() to make use of a vm_domainset iterator instead.

I changed keg_alloc_slab() to return with the keg unlocked on failure,
identical to zone_fetch_slab_multi(). I also made changes to ensure
that uk_reserve is only accessed with the keg lock held.

One side effect of this is that UMA allocations are now round-robin by
thread rather than round-robin by keg. I don't believe this will matter
in practice.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 19980
Build 19487: arc lint + arc unit

Event Timeline

markj created this revision.Oct 4 2018, 8:00 PM

Harbormaster completed remote builds in B19980: Diff 48745.Oct 4 2018, 8:00 PM

markj added reviewers: alc, jeff, kib, gallatin.Oct 4 2018, 8:12 PM

emaste added a subscriber: emaste.Oct 4 2018, 8:19 PM

This is surprisingly simple. I am a little uneasy with the prospect of not doing round-robin on the keg's iterator. Can we not pass in an iterator somehow?

sys/vm/uma_core.c
2633	This name makes the code below confusing to read. It looks like we only fetch the first slab if we're allowed to use reserve. Can we name it something like: keg_available_slab() I don't love that name either but anything that indicates that it's a filter or limit check and not dipping into the reserve specifically.

In D17420#373676, @jeff wrote:

This is surprisingly simple. I am a little uneasy with the prospect of not doing round-robin on the keg's iterator. Can we not pass in an iterator somehow?

I tried to find a way to do that; the issue is that a) we want to update the iterator with the keg locked, and b) we want to be able to call vm_wait_doms() in vm_domainset_iter_policy(), but we have to drop the keg lock to do that.

We could pass the lock object in as a separate parameter, but I also want to minimize the number of parameters we pass to vm_domainset_*() since those functions are called frequently. Do you see a different approach?

sys/vm/uma_core.c
2633	My intention was just to prevent unsynchronized access to uk_reserve, but we can do a bit better and define a subroutine which returns the first free slab if we're not dipping into the reserve. That eliminates some code duplication and makes it easier to find a decent name.

Add keg_fetch_free_slab().

Harbormaster completed remote builds in B20132: Diff 48999.Oct 11 2018, 1:22 AM

markj mentioned this in D17419: Use malloc_domainset(9) instead of malloc_domain(M_WAITOK)..Oct 11 2018, 3:38 PM

In D17420#373690, @markj wrote:

In D17420#373676, @jeff wrote:

This is surprisingly simple. I am a little uneasy with the prospect of not doing round-robin on the keg's iterator. Can we not pass in an iterator somehow?

I tried to find a way to do that; the issue is that a) we want to update the iterator with the keg locked, and b) we want to be able to call vm_wait_doms() in vm_domainset_iter_policy(), but we have to drop the keg lock to do that.

We could pass the lock object in as a separate parameter, but I also want to minimize the number of parameters we pass to vm_domainset_*() since those functions are called frequently. Do you see a different approach?

We could pass a domainset_ref to the iterator initialization instead of just a policy. This will give us the right iter pointer. The keg could have a domainset_ref embedded that is set according to initialization flags.

We could always do NOWAIT iterations and put the blocking in the caller to avoid passing in the lock. It's not that much extra code for the caller.

markj mentioned this in D17417: Refactor domainset iterators in preparation for use by malloc() and UMA..Oct 16 2018, 4:01 PM