On 10/10/2014 1:07 PM, Sumit Semwal wrote:
Devices sharing buffers using dma-buf could benefit from sharing their constraints via struct device, and dma-buf framework would manage the common constraints for all attached devices per buffer.
With that information, we could have a 'generic' allocator helper in the form of a central dma-buf exporter, which can create dma-bufs, and allocate backing storage at the time of first call to dma_buf_map_attachment.
This allocation would utilise the constraint-mask by matching it to the right allocator from a pool of allocators, and then allocating buffer backing storage from this allocator.
The pool of allocators could be platform-dependent, allowing for platforms to hide the specifics of these allocators from the devices that access the dma-buf buffers.
A sample sequence could be:
- get handle to cenalloc_device,
- create a dmabuf using cenalloc_buffer_create;
- use this dmabuf to attach each device, which has its constraints set in the constraints mask (dev->dma_params->access_constraints_mask)
- at each dma_buf_attach() call, dma-buf will check to see if the constraint mask for the device requesting attachment is compatible with the constraints of devices already attached to the dma-buf; returns an error if it isn't.
- after all devices have attached, the first call to dma_buf_map_attachment() will allocate the backing storage for the buffer.
- follow the dma-buf api for map / unmap etc usage.
- detach all attachments,
- call cenalloc_buffer_free to free the buffer if refcount reaches zero;
** IMPORTANT** This mechanism of delayed allocation based on constraint-enablement will work *ONLY IF* the first map_attachment() call is made AFTER all attach() calls are done.
My first instinct is 'I wonder which drivers will call map_attachment at the wrong time and screw things up'. Are there any plans for synchronization and/or debugging output to catch drivers violating this requirement?
[...]
+int cenalloc_phys(struct dma_buf *dmabuf,
phys_addr_t *addr, size_t *len)+{
- struct cenalloc_buffer *buffer;
- int ret;
- if (is_cenalloc_buffer(dmabuf))
buffer = (struct cenalloc_buffer *)dmabuf->priv;- else
return -EINVAL;- if (!buffer->allocator->ops->phys) {
pr_err("%s: cenalloc_phys is not implemented by this allocator.\n",__func__);return -ENODEV;- }
- mutex_lock(&buffer->lock);
- ret = buffer->allocator->ops->phys(buffer->allocator, buffer, addr,
len);- mutex_lock(&buffer->lock);
- return ret;
+} +EXPORT_SYMBOL_GPL(cenalloc_phys);
The .phys operation makes it difficult to have drivers which can handle both contiguous and non contiguous memory (too much special casing). Any chance we could drop this API and just have drivers treat an sg_table with 1 entry as contiguous memory?
Thanks, Laura