Page MenuHomeFreeBSD

Make malloc_domain(9) fall back to other domains.
ClosedPublic

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

Details

Summary

This ensures that malloc_domain(M_WAITOK) won't hang
forever if one domain is depleted (e.g., because most of its
memory has been wired by the ARC).

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20434
Build 19872: arc lint + arc unit

Event Timeline

markj created this revision.Oct 4 2018, 8:00 PM
markj marked an inline comment as done.Oct 4 2018, 8:15 PM
markj added inline comments.
sys/kern/kern_malloc.c
672

This test is inverted.

emaste added a subscriber: emaste.Oct 4 2018, 8:18 PM
markj added a comment.Oct 5 2018, 4:49 PM

As an aside, this same pattern could be used to implement uma_zalloc_domainset() or vm_page_alloc_domainset(). For example: https://reviews.freebsd.org/D17425 , which I posted just as an example.

Do we really need a new malloc variant? Couldn't this be done with a new malloc flag that was specific to malloc_domain(). Something like M_ALTOK or something? (or maybe M_EXACT if you want the default to be the other way around, and have malloc_domain() fall back by default)

I'm just thinking how confusing its going to be for somebody porting code to FreeBSD, trying to choose between malloc_domain, malloc_domainset, malloc, not to mention the zone allocators.

markj added a comment.Oct 5 2018, 5:13 PM

Do we really need a new malloc variant? Couldn't this be done with a new malloc flag that was specific to malloc_domain(). Something like M_ALTOK or something? (or maybe M_EXACT if you want the default to be the other way around, and have malloc_domain() fall back by default)

It could be done that way, but we already have this powerful domainset machinery for expressing NUMA allocation policies; I don't really like the idea of having a second, less general way of expressing those policies. I suspect we'll end up having to add more flags as time goes on, and naming will be tricky: it's not obvious that M_EXACT has anything to do with NUMA.

I'm just thinking how confusing its going to be for somebody porting code to FreeBSD, trying to choose between malloc_domain, malloc_domainset, malloc, not to mention the zone allocators.

That's a fair point. However, I don't think the situation is really that bad - the question of malloc() vs. a slab allocator is orthogonal to this, and it's pretty clear that malloc_domain() and malloc_domainset() are for NUMA-aware allocations. Moreover, the former is just a special case of the latter.

kib accepted this revision.Oct 5 2018, 7:52 PM
This revision is now accepted and ready to land.Oct 5 2018, 7:52 PM
jeff added inline comments.Oct 10 2018, 11:39 PM
sys/sys/malloc.h
239–243

malloc_domain is used in so few places and they should probably all be 'prefer' anyway. We could implement this as a macro/inline wrapper or eliminate it altogether. I'd prefer not to keep the somewhat redundant functions.

markj updated this revision to Diff 49227.Oct 16 2018, 4:08 PM

Remove malloc_domainset() and change malloc_domain() to fall back to
other domains if an allocation from the requested domain is not possible.

This revision now requires review to proceed.Oct 16 2018, 4:08 PM
markj marked an inline comment as done.Oct 16 2018, 4:10 PM
markj added inline comments.
sys/sys/malloc.h
239–243

I ended up removing it entirely, and just changing the behaviour of malloc_domain(). This makes it inconsistent with every other *_domain() KPI.

markj retitled this revision from Add malloc_domainset(9). to Make malloc_domain(9) fall back to other domains..Oct 16 2018, 4:30 PM
markj edited the summary of this revision. (Show Details)
jeff added a comment.Oct 17 2018, 9:11 PM

I still feel it would be nice to make kmem_ and malloc_ take a policy possibly with an inline that converts a number into a preferred policy. There is likely no reason that kmem_ allocations should fail if the requested domain is unavailable.

Making uma_zalloc_domain work this way will be somewhat more complicated but I would like to keep the APIs consistent.

gallatin added inline comments.Oct 19 2018, 12:30 PM
share/man/man9/Makefile
1290 ↗(On Diff #49227)

Looks like you may have forgotten to back this out when you went from malloc_domainset() back to malloc_domain()?

markj updated this revision to Diff 49633.Oct 25 2018, 9:34 PM
markj marked an inline comment as done.

Reintroduce malloc_domainset() per Jeff's comments.

With this change we have _domainset() variants for malloc(), contigmalloc(),
kmem_malloc(), kmem_alloc_contig() and kmem_alloc_attr(). The _domain()
variants of all of those functions call _domainset(DOMAINSET_PREF(domain)).

I didn't implement the _domain variants as inlines since that would require
polluting the namespace with the domainset.h. I suspect there is some
advantage to maintaining the KBI, anyway.

uma_zalloc_domain() is still strict. This is mostly because it is used
by _malloc_domain().

There are still some man pages updates required, but I'm going to hold
off on them for now.

markj accepted this revision.Nov 1 2018, 2:49 PM

A version of this was committed in r339927.

This revision is now accepted and ready to land.Nov 1 2018, 2:49 PM
markj closed this revision.Nov 1 2018, 2:49 PM