On Mon, Mar 06, 2017 at 05:02:05PM +0200, Laurent Pinchart wrote:
Hi Daniel,
On Monday 06 Mar 2017 11:38:20 Daniel Vetter wrote:
On Fri, Mar 03, 2017 at 06:45:40PM +0200, Laurent Pinchart wrote:
- I haven't seen any proposal how a heap-based solution could be used in a
generic distribution. This needs to be figured out before committing to any API/ABI.
Two replies from my side:
- Just because a patch doesn't solve world hunger isn't really a good reason to reject it.
As long as it goes in the right direction, sure :-) The points I mentioned were to be interpreted that way, I want to make sure we're not going in a dead-end (or worse, driving full speed into a wall).
- Heap doesn't mean its not resizeable (but I'm not sure that's really your concern).
Not really, no. Heap is another word to mean pool here. It might not be the best term in this context as it has a precise meaning in the context of memory allocation, but that's a detail.
Imo ION is very much part of the picture here to solve this for real. We need to bits:
Be able to allocate memory from specific pools, not going through a specific driver. ION gives us that interface. This is e.g. also needed for "special" memory, like SMA tries to expose.
Some way to figure out how&where to allocate the buffer object. This is purely a userspace problem, and this is the part the unix memory allocator tries to solve. There's no plans in there for big kernel changes, instead userspace does a dance to reconcile all the constraints, and one of the constraints might be "you have to allocate this from this special ION heap". The only thing the kernel needs to expose is which devices use which ION heaps (we kinda do that already), and maybe some hints of how they can be generalized (but I guess stuff like "minimal pagesize of x KB" is also fulfilled by any CMA heap is knowledge userspace needs).
The constraint solver could live in userspace, I'm open to a solution that would go in that direction, but it will require help from the kernel to fetch the constraints from the devices that need to be involved in buffer sharing.
Given a userspace constraint resolver, the interface with the kernel allocator will likely be based on pools. I'm not opposed to that, as long as pool are identified by opaque handles. I don't want userspace to know about the meaning of any particular ION heap. Application must not attempt to "allocate from CMA" for instance, that would lock us to a crazy API that will grow completely out of hands as vendors will start adding all kind of custom heaps, and applications will have to follow (or will be patched out-of-tree by vendors).
Again I think waiting for this to be fully implemented before we merge any part is going to just kill any upstreaming efforts. ION in itself, without the full buffer negotiation dance seems clearly useful (also for stuff like SMA), and having it merged will help with moving the buffer allocation dance forward.
Again I'm not opposed to a kernel allocator based on pools/heaps, as long as
- pools/heaps stay internal to the kernel and are not directly exposed to
userspace
Agreed (and I think ION doesn't have fixed pools afaik, just kinda conventions, at least after Laura's patches). But on a fixed board with a fixed DT (for the cma regions) and fixed .config (for the generic heaps) you can hardcode your heaps. You'll make your code non-portable, but hey that's not our problem imo. E.g. board-specific code can also hard-code how to wire connectors and which one is which in kms (and I've seen this). I don't think the possibility of abusing the uabi should be a good reason to prevent it from merging. Anything that provides something with indirect connections can be abused by hardcoding the names or the indizes.
We do have a TODO entry that talks about exposing the device -> cma heap link in sysfs or somewhere. I'm not versed enough to know whether Laura's patches fixed that, this here mostly seems to tackle the fundamentals of the dma api abuse first.
- a reasonable way to size the different kinds of pools in a generic
distribution kernel can be found
So for the CMA heaps, you can't resize them at runtime, for obvious reasons. For boot-time you can adjust them through DT, and I thought everyone agreed that for different use-cases you might need to adjust your reserved regions.
For all other heaps, they just use the normal allocator functions (e.g. alloc_pages). There's not limit on those except OOM, so nothing to adjust really.
I guess I'm still not entirely clear on your "memory pool" concern ... If it's just the word, we have lots of auto-resizing heaps/pools all around. And if it's just sizing, I think that's already solved as good as possible (assuming there's not a silly limit on the system heap that we should remove ...).
Cheers, Daniel