I wanted to allocate kernel stacks through UMA so that we can apply NUMA policy to them as well as utilize other allocator features. In general I prefer to add small features to UMA rather than add another custom allocator.
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I think that the patch is functionally fine.
It looks strange that
- For newly created stack, you set the backing object domain policy to prefer domain of the current cpu.
- You do not encode any preference when allocating from cache.
When allocating from cache the preference was recorded when it was inserted into cache in kstack_import(). The object is initialized and pointed to by the pages.
I think the one thing that is questionable here is the PREFER vs FIXED. I should probably fix them both as PREFER. Do you agree?
sys/vm/vm_glue.c | ||
---|---|---|
347 ↗ | (On Diff #59670) | I don't think we need to initialize the iterator anymore? |
sys/vm/vm_glue.c | ||
---|---|---|
347 ↗ | (On Diff #59670) | If we don't initialize it then every ksobj will iterate from the same point. By initializing in this way they each step through the available domains slightly differently. Was there some other change that made this unnecessary? I don't see it. |
So it is UMA_ZONE_NUMA which gives the locality when satisfying the allocation from cache ?
I think the one thing that is questionable here is the PREFER vs FIXED. I should probably fix them both as PREFER. Do you agree?
I think yes.
Yes, stacks will now be allocated according to the UMA policy and the calling thread's domain. This isn't perfect when we're allocating a thread that we want to run on another domain. That is something that requires some deeper support for domain level locality in the scheduler to really be effective anyhow. This will work for threads that do some static binding.
sys/vm/vm_glue.c | ||
---|---|---|
301 ↗ | (On Diff #59727) | This should be flagged CTLFLAG_RWTUN to ensure that it can be set via loader.conf. We should declare CTLFLAG_MPSAFE as well. |
347 ↗ | (On Diff #59670) | That's true, though now the iterator is used only if the preferred domain is depleted. Shouldn't we do this kind of initialization in a more central location, like vm_object_allocate()? |