On 5/13/25 14:30, wangtao wrote:
-----Original Message----- From: Christian König christian.koenig@amd.com Sent: Tuesday, May 13, 2025 7:32 PM To: wangtao tao.wangtao@honor.com; sumit.semwal@linaro.org; benjamin.gaignard@collabora.com; Brian.Starkey@arm.com; jstultz@google.com; tjmercier@google.com Cc: linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linaro- mm-sig@lists.linaro.org; linux-kernel@vger.kernel.org; wangbintian(BintianWang) bintian.wang@honor.com; yipengxiang yipengxiang@honor.com; liulu 00013167 liulu.liu@honor.com; hanfeng 00012985 feng.han@honor.com Subject: Re: [PATCH 2/2] dmabuf/heaps: implement DMA_BUF_IOCTL_RW_FILE for system_heap
On 5/13/25 11:28, wangtao wrote:
Support direct file I/O operations for system_heap dma-buf objects. Implementation includes:
- Convert sg_table to bio_vec
That is usually illegal for DMA-bufs.
[wangtao] The term 'convert' is misleading in this context. The appropriate phrasing should be: Construct bio_vec from sg_table.
Well it doesn't matter what you call it. Touching the page inside an sg table of a DMA-buf is illegal, we even have code to actively prevent that.
Once more: This approach was already rejected multiple times! Please use udmabuf instead!
The hack you came up here is simply not necessary.
Regards, Christian.
Appreciate your feedback.
Regards, Christian.
- Set IOCB_DIRECT when O_DIRECT is supported 3. Invoke
vfs_iocb_iter_read()/vfs_iocb_iter_write() for actual I/O
Performance metrics (UFS 4.0 device @4GB/s, Arm64 CPU @1GHz):
| Metric | 1MB | 8MB | 64MB | 1024MB | 3072MB | |--------------------|-------:|-------:|--------:|---------:|--------- |--------------------|:| | Buffer Read (us) | 1658 | 9028 | 69295 | 1019783 | 2978179 | | Direct Read (us) | 707 | 2647 | 18689 | 299627 | 937758 | | Buffer Rate (MB/s) | 603 | 886 | 924 | 1004 | 1032 | | Direct Rate (MB/s) | 1414 | 3022 | 3425 | 3418 | 3276 |
Signed-off-by: wangtao tao.wangtao@honor.com
drivers/dma-buf/heaps/system_heap.c | 118 ++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+)
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 26d5dc89ea16..f7b71b9843aa 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -20,6 +20,8 @@ #include <linux/scatterlist.h> #include <linux/slab.h> #include <linux/vmalloc.h> +#include <linux/bvec.h> +#include <linux/uio.h>
static struct dma_heap *sys_heap;
@@ -281,6 +283,121 @@ static void system_heap_vunmap(struct dma_buf
*dmabuf, struct iosys_map *map)
iosys_map_clear(map); }
+static struct bio_vec *system_heap_init_bvec(struct
system_heap_buffer *buffer,
size_t offset, size_t len, int *nr_segs) {- struct sg_table *sgt = &buffer->sg_table;
- struct scatterlist *sg;
- size_t length = 0;
- unsigned int i, k = 0;
- struct bio_vec *bvec;
- size_t sg_left;
- size_t sg_offset;
- size_t sg_len;
- bvec = kvcalloc(sgt->nents, sizeof(*bvec), GFP_KERNEL);
- if (!bvec)
return NULL;- for_each_sg(sgt->sgl, sg, sgt->nents, i) {
length += sg->length;if (length <= offset)continue;sg_left = length - offset;sg_offset = sg->offset + sg->length - sg_left;sg_len = min(sg_left, len);bvec[k].bv_page = sg_page(sg);bvec[k].bv_len = sg_len;bvec[k].bv_offset = sg_offset;k++;offset += sg_len;len -= sg_len;if (len <= 0)break;- }
- *nr_segs = k;
- return bvec;
+}
+static int system_heap_rw_file(struct system_heap_buffer *buffer, bool
is_read,
bool direct_io, struct file *filp, loff_t file_offset,size_t buf_offset, size_t len)+{
- struct bio_vec *bvec;
- int nr_segs = 0;
- struct iov_iter iter;
- struct kiocb kiocb;
- ssize_t ret = 0;
- if (direct_io) {
if (!(filp->f_mode & FMODE_CAN_ODIRECT))return -EINVAL;- }
- bvec = system_heap_init_bvec(buffer, buf_offset, len, &nr_segs);
- if (!bvec)
return -ENOMEM;- iov_iter_bvec(&iter, is_read ? ITER_DEST : ITER_SOURCE, bvec,
nr_segs, len);
- init_sync_kiocb(&kiocb, filp);
- kiocb.ki_pos = file_offset;
- if (direct_io)
kiocb.ki_flags |= IOCB_DIRECT;- while (kiocb.ki_pos < file_offset + len) {
if (is_read)ret = vfs_iocb_iter_read(filp, &kiocb, &iter);elseret = vfs_iocb_iter_write(filp, &kiocb, &iter);if (ret <= 0)break;- }
- kvfree(bvec);
- return ret < 0 ? ret : 0;
+}
+static int system_heap_dma_buf_rw_file(struct dma_buf *dmabuf,
struct dma_buf_rw_file *back)+{
- struct system_heap_buffer *buffer = dmabuf->priv;
- int ret = 0;
- __u32 op = back->flags & DMA_BUF_RW_FLAGS_OP_MASK;
- bool direct_io = back->flags & DMA_BUF_RW_FLAGS_DIRECT;
- struct file *filp;
- if (op != DMA_BUF_RW_FLAGS_READ && op !=
DMA_BUF_RW_FLAGS_WRITE)
return -EINVAL;- if (direct_io) {
if (!PAGE_ALIGNED(back->file_offset) ||!PAGE_ALIGNED(back->buf_offset) ||!PAGE_ALIGNED(back->buf_len))return -EINVAL;- }
- if (!back->buf_len || back->buf_len > dmabuf->size ||
back->buf_offset >= dmabuf->size ||back->buf_offset + back->buf_len > dmabuf->size)return -EINVAL;- if (back->file_offset + back->buf_len < back->file_offset)
return -EINVAL;- filp = fget(back->fd);
- if (!filp)
return -EBADF;- mutex_lock(&buffer->lock);
- ret = system_heap_rw_file(buffer, op ==
DMA_BUF_RW_FLAGS_READ, direct_io,
filp, back->file_offset, back->buf_offset, back-buf_len);
- mutex_unlock(&buffer->lock);
- fput(filp);
- return ret;
+}
static void system_heap_dma_buf_release(struct dma_buf *dmabuf) { struct system_heap_buffer *buffer = dmabuf->priv; @@ -308,6
+425,7
@@ static const struct dma_buf_ops system_heap_buf_ops = { .mmap = system_heap_mmap, .vmap = system_heap_vmap, .vunmap = system_heap_vunmap,
- .rw_file = system_heap_dma_buf_rw_file, .release = system_heap_dma_buf_release, };