On 7/3/25 15:23, Christoph Hellwig wrote:
[Note: it would be really useful to Cc all relevant maintainers]
Will do next time
On Fri, Jun 27, 2025 at 04:10:27PM +0100, Pavel Begunkov wrote:
This series implements it for read/write io_uring requests. The uAPI
looks similar to normal registered buffers, the user will need to
register a dmabuf in io_uring first and then use it as any other
registered buffer. On registration the user also specifies a file
to map the dmabuf for.
Just commenting from the in-kernel POV here, where the interface
feels wrong.
You can't just expose 'the DMA device' up file operations, because
there can be and often is more than one. Similarly stuffing a
dma_addr_t into an iovec is rather dangerous.
The model that should work much better is to have file operations
to attach to / detach from a dma_buf, and then have an iter that
specifies a dmabuf and offsets into. That way the code behind the
file operations can forward the attachment to all the needed
devices (including more/less while it remains attached to the file)
and can pick the right dma address for each device.
By "iter that specifies a dmabuf" do you mean an opaque file-specific
structure allocated inside the new fop? Akin to what Keith proposed back
then. That sounds good and has more potential for various optimisations.
My concern would be growing struct iov_iter by an extra pointer:
struct dma_seg {
size_t off;
unsigned len;
};
struct iov_iter {
union {
struct iovec *iov;
struct dma_seg *dmav;
...
};
void *dma_token;
};
But maybe that's fine. It's 40B -> 48B, and it'll get back to
40 when / if xarray_start / ITER_XARRAY is removed.
I also remember some discussion that new dma-buf importers should
use the dynamic imported model for long-term imports, but as I'm
everything but an expert in that area I'll let the dma-buf folks
speak.
I'll take a look
--
Pavel Begunkov
