public inbox for [email protected]
 help / color / mirror / Atom feed
From: Ming Lei <[email protected]>
To: Pavel Begunkov <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: [RFC 7/7] io_uring,fs: introduce IORING_OP_GET_BUF
Date: Tue, 2 May 2023 22:57:30 +0800	[thread overview]
Message-ID: <ZFEk2rQv2//[email protected]> (raw)
In-Reply-To: <fc43826d510dc75de83d81161ca03e2688515686.1682701588.git.asml.silence@gmail.com>

On Sun, Apr 30, 2023 at 10:35:29AM +0100, Pavel Begunkov wrote:
> There are several problems with splice requests, aka IORING_OP_SPLICE:
> 1) They are always executed by a worker thread, which is a slow path,
>    as we don't have any reliable way to execute it NOWAIT.
> 2) It can't easily poll for data, as there are 2 files it operates on.
>    It would either need to track what file to poll or poll both of them,
>    in both cases it'll be a mess and add lot of overhead.
> 3) It has to have pipes in the middle, which adds overhead and is not
>    great from the uapi design perspective when it goes for io_uring
>    requests.
> 4) We want to operate with spliced data as with a normal buffer, i.e.
>    write / send / etc. data as normally while it's zerocopy.
> 
> It can partially be solved, but the root cause is a suboptimal for
> io_uring design of IORING_OP_SPLICE. Introduce a new request type
> called IORING_OP_GET_BUF, inspired by splice(2) as well as other
> proposals like fused requests. The main idea is to use io_uring's
> registered buffers as the middle man instead of pipes. Once a buffer
> is fetched / spliced from a file using a new fops callback
> ->iou_get_buf, it's installed as a registered buffers and can be used
> by all operations supporting the feature.
> 
> Once the userspace releases the buffer, io_uring will wait for all
> requests using the buffer to complete and then use a file provided
> callback ->release() to return the buffer back. It operates on the

In the commit of "io_uring: add an example for buf-get op", I don't see
any code to release the buffer, can you explain it in details about how
to release the buffer in userspace? And add it in your example?

Here I guess the ->release() is called in the following code path:

io_buffer_unmap
    io_rsrc_buf_put
        io_rsrc_put_work
            io_rsrc_node_ref_zero
                io_put_rsrc_node

If it is true, what is counter-pair code for io_put_rsrc_node()?
So far, only see io_req_set_rsrc_node() is called from
io_file_get_fixed(), is it needed for consumer OP of the buffer?

Also io_buffer_unmap() is called after io_rsrc_node's reference drops
to zero, which means ->release() isn't called after all its consumer(s)
are done given io_rsrc_node is shared by in-flight requests. If it is
true, this way will increase buffer lifetime a lot.

ublk zero copy needs to call ->release() immediately after all
consumers are done, because the ublk disk request won't be completed
until the buffer is released(the buffer actually belongs to ublk block request).

Also the usage in liburing example needs two extra syscall(io_uring_enter) for
handling one IO, not take into account the "release OP". IMO, this way makes
application more complicated, also might perform worse:

1) for ublk zero copy, the original IO just needs one OP, but now it
takes three OPs, so application has to take coroutine for applying
3 stages batch submission(GET_BUF, IO, release buffer) since IO_LINK can't
or not suggested to be used. In case of low QD, batch size is reduced much,
and performance may hurt because IOs/syscall is 1/3 of fused command.

2) GET_BUF OP is separated from the consumer OP, this way may cause
more cache miss, and I know this way is for avoiding IO_LINK.

I'd understand the approach first before using it to implement ublk zero copy
and comparing its performance with fused command.


Thanks, 
Ming


  reply	other threads:[~2023-05-02 14:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-30  9:35 [RFC 0/7] Rethinking splice Pavel Begunkov
2023-04-30  9:35 ` [RFC 1/7] io_uring: add io_mapped_ubuf caches Pavel Begunkov
2023-04-30  9:35 ` [RFC 2/7] io_uring: add reg-buffer data directions Pavel Begunkov
2023-04-30  9:35 ` [RFC 3/7] io_uring: fail loop_rw_iter with pure bvec bufs Pavel Begunkov
2023-04-30  9:35 ` [RFC 4/7] io_uring/rsrc: introduce struct iou_buf_desc Pavel Begunkov
2023-04-30  9:35 ` [RFC 5/7] io_uring/rsrc: add buffer release callbacks Pavel Begunkov
2023-04-30  9:35 ` [RFC 6/7] io_uring/rsrc: introduce helper installing one buffer Pavel Begunkov
2023-04-30  9:35 ` [RFC 7/7] io_uring,fs: introduce IORING_OP_GET_BUF Pavel Begunkov
2023-05-02 14:57   ` Ming Lei [this message]
2023-05-02 15:20     ` Ming Lei
2023-05-03 14:54     ` Pavel Begunkov
2023-05-04  2:06       ` Ming Lei
2023-05-08  2:30         ` Pavel Begunkov
2023-05-17  4:05           ` Ming Lei

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=ZFEk2rQv2//[email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox