All of lore.kernel.org
 help / color / mirror / Atom feed
From: wangtao <tao.wangtao@honor.com>
To: "Christian König" <christian.koenig@amd.com>,
	"T.J. Mercier" <tjmercier@google.com>
Cc: "sumit.semwal@linaro.org" <sumit.semwal@linaro.org>,
	"benjamin.gaignard@collabora.com"
	<benjamin.gaignard@collabora.com>,
	"Brian.Starkey@arm.com" <Brian.Starkey@arm.com>,
	"jstultz@google.com" <jstultz@google.com>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>,
	"linux-kernel@vger.kernel.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>,
	"amir73il@gmail.com" <amir73il@gmail.com>
Subject: RE: [PATCH 2/2] dmabuf/heaps: implement DMA_BUF_IOCTL_RW_FILE for system_heap
Date: Wed, 21 May 2025 10:25:36 +0000	[thread overview]
Message-ID: <e61fcdbf71ba4f9dbfef2f521d1b2fc1@honor.com> (raw)
In-Reply-To: <fdd7a11b-140c-40bd-a1c1-334d69256b92@amd.com>



> -----Original Message-----
> From: Christian König <christian.koenig@amd.com>
> Sent: Wednesday, May 21, 2025 3:36 PM
> To: wangtao <tao.wangtao@honor.com>; T.J. Mercier
> <tjmercier@google.com>
> Cc: sumit.semwal@linaro.org; benjamin.gaignard@collabora.com;
> Brian.Starkey@arm.com; jstultz@google.com; 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/21/25 06:17, wangtao wrote:
> >>> Reducing CPU overhead/power consumption is critical for mobile
> devices.
> >>> We need simpler and more efficient dmabuf direct I/O support.
> >>>
> >>> As Christian evaluated sendfile performance based on your data,
> >>> could you confirm whether the cache was cleared? If not, please
> >>> share the post-cache-clearing test data. Thank you for your support.
> >>
> >> Yes sorry, I was out yesterday riding motorcycles. I did not clear
> >> the cache for the buffered reads, I didn't realize you had. The IO
> >> plus the copy certainly explains the difference.
> >>
> >> Your point about the unlikelihood of any of that data being in the
> >> cache also makes sense.
> > [wangtao] Thank you for testing and clarifying.
> >
> >>
> >> I'm not sure it changes anything about the ioctl approach though.
> >> Another way to do this would be to move the (optional) support for
> >> direct IO into the exporter via dma_buf_fops and dma_buf_ops. Then
> >> normal read() syscalls would just work for buffers that support them.
> >> I know that's more complicated, but at least it doesn't require
> >> inventing new uapi to do it.
> >>
> > [wangtao] Thank you for the discussion. I fully support any method
> > that enables dmabuf direct I/O.
> >
> > I understand using sendfile/splice with regular files for dmabuf adds
> > an extra CPU copy, preventing zero-copy. For example:
> > sendfile path: [DISK] → DMA → [page cache] → CPU copy → [memory
> file].
> 
> Yeah, but why can't you work on improving that?
> 
> > The read() syscall can't pass regular file fd parameters, so I added
> > an ioctl command.
> > While copy_file_range() supports two fds (fd_in/fd_out), it blocks cross-fs
> use.
> > Even without this restriction, file_out->f_op->copy_file_range only
> > enables dmabuf direct reads from regular files, not writes.
> >
> > Since dmabuf's direct I/O limitation comes from its unique
> > attachment/map/fence model and lacks suitable syscalls, adding an
> > ioctl seems necessary.
> 
> I absolutely don't see that. Both splice and sendfile can take two regular file
> descriptors.
> 
> That the underlying fops currently can't do that is not a valid argument for
> adding new uAPI. It just means that you need to work on improving those
> fops.
> 
> As long as nobody proves to me that the existing uAPI isn't sufficient for this
> use case I will systematically reject any approach to adding new one.
> 
[wangtao] I previously explained that read/sendfile/splice/copy_file_range
syscalls can't achieve dmabuf direct IO zero-copy.
1. read() can't pass regular file fd to dmabuf.
2. sendfile() supports regular file <-> regular file/socket zero-copy,
but not regular file <-> memory file.
Example:
sendfile(dst_net, src_disk):
[DISK] --DMA--> [page buffer] --DMA--> [NIC]

sendfile(dst_disk, src_disk):
[DISK] --DMA--> [page buffer] --DMA--> [DISK]

sendfile(dst_memfile, src_disk):
[DISK] --DMA--> [page buffer] --CPU copy--> [MEMORY file]

3. splice() requires one end to be a pipe, making it unsuitable.
4. copy_file_range() is blocked by cross-FS restrictions (Amir's commit
868f9f2f8e004bfe0d3935b1976f625b2924893b). Even without this,
file_out->f_op->copy_file_range only enables dmabuf read from regular files,
not write.

My focus is enabling dmabuf direct I/O for [regular file] <--DMA--> [dmabuf]
zero-copy. Any API achieving this would work. Are there other uAPIs you think
could help? Could you recommend experts who might offer suggestions?
Thank you.

> Regards,
> Christian.
> 
> > When system exporters return a duplicated sg_table via map_dma_buf
> > (used exclusively like a pages array), they should retain control over
> > it.
> >
> > I welcome all solutions to achieve dmabuf direct I/O! Your feedback is
> > greatly appreciated.
> >
> >> 1G from ext4 on 6.12.20 | read/sendfile (ms) w/ 3 > drop_caches
> >> ------------------------|-------------------
> >> udmabuf buffer read     | 1210
> >> udmabuf direct read     | 671
> >> udmabuf buffer sendfile | 1096
> >> udmabuf direct sendfile | 2340
> >>
> >>
> >>
> >>>
> >>>>>
> >>>>>>> dmabuf buffer read     | 51         | 1068      | 1118
> >>>>>>> dmabuf direct read     | 52         | 297       | 349
> >>>>>>>
> >>>>>>> udmabuf sendfile test steps:
> >>>>>>> 1. Open data file(1024MB), get back_fd 2. Create memfd(32MB) #
> >>>>>>> Loop steps 2-6 3. Allocate udmabuf with memfd 4. Call
> >>>>>>> sendfile(memfd,
> >>>>>>> back_fd) 5. Close memfd after sendfile 6. Close udmabuf 7.
> >>>>>>> Close back_fd
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>> Christian.
> >>>>>>>
> >>>>>>
> >>>


  reply	other threads:[~2025-05-21 10:25 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-13  9:28 [PATCH 2/2] dmabuf/heaps: implement DMA_BUF_IOCTL_RW_FILE for system_heap wangtao
2025-05-13 11:32 ` Christian König
2025-05-13 12:30   ` wangtao
2025-05-13 13:17     ` Christian König
2025-05-14 11:02       ` wangtao
2025-05-14 12:00         ` Christian König
2025-05-15 14:03           ` wangtao
2025-05-15 14:26             ` Christian König
2025-05-16  7:40               ` wangtao
2025-05-16  8:36                 ` Christian König
2025-05-16  9:49                   ` wangtao
2025-05-16 10:29                     ` Christian König
2025-05-19  4:08                       ` wangtao
2025-05-19  7:47                         ` Christian König
2025-05-16 18:37                   ` T.J. Mercier
2025-05-19  4:37                     ` wangtao
2025-05-19 12:03                     ` wangtao
2025-05-20  4:06                       ` wangtao
2025-05-21  2:00                         ` T.J. Mercier
2025-05-21  4:17                           ` wangtao
2025-05-21  7:35                             ` Christian König
2025-05-21 10:25                               ` wangtao [this message]
2025-05-21 11:56                                 ` Christian König
2025-05-22  8:02                                   ` wangtao
2025-05-22 11:57                                     ` Christian König
2025-05-22 12:29                                       ` wangtao
2025-05-27 14:35                                       ` wangtao
2025-05-27 15:10                                         ` Christian König
  -- strict thread matches above, loose matches on Subject: below --
2025-05-14 12:57 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e61fcdbf71ba4f9dbfef2f521d1b2fc1@honor.com \
    --to=tao.wangtao@honor.com \
    --cc=Brian.Starkey@arm.com \
    --cc=amir73il@gmail.com \
    --cc=benjamin.gaignard@collabora.com \
    --cc=bintian.wang@honor.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=feng.han@honor.com \
    --cc=jstultz@google.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=liulu.liu@honor.com \
    --cc=sumit.semwal@linaro.org \
    --cc=tjmercier@google.com \
    --cc=yipengxiang@honor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.