On 3/21/25 21:24, Caleb Sander Mateos wrote:
On Fri, Mar 21, 2025 at 1:23 PM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
On 3/21/25 18:48, Caleb Sander Mateos wrote:
To use ublk zero copy, an application submits a sequence of io_uring
operations:
(1) Register a ublk request's buffer into the fixed buffer table
(2) Use the fixed buffer in some I/O operation
(3) Unregister the buffer from the fixed buffer table
The ordering of these operations is critical; if the fixed buffer lookup
occurs before the register or after the unregister operation, the I/O
will fail with EFAULT or even corrupt a different ublk request's buffer.
It is possible to guarantee the correct order by linking the operations,
but that adds overhead and doesn't allow multiple I/O operations to
execute in parallel using the same ublk request's buffer. Ideally, the
application could just submit the register, I/O, and unregister SQEs in
the desired order without links and io_uring would ensure the ordering.
This mostly works, leveraging the fact that each io_uring SQE is prepped
and issued non-blocking in order (barring link, drain, and force-async
flags). But it requires the fixed buffer lookup to occur during the
initial non-blocking issue.
In other words, leveraging internal details that is not a part
of the uapi, should never be relied upon by the user and is fragile.
Any drain request or IOSQE_ASYNC and it'll break, or for any reason
why it might be desirable to change the behaviour in the future.
Sorry, but no, we absolutely can't have that, it'll be an absolute
nightmare to maintain as basically every request scheduling decision
now becomes a part of the uapi.
I thought we discussed this on the ublk zero copy patchset, but I
can't seem to find the email. My recollection is that Jens thought it
was reasonable for userspace to rely on the sequential prep + issue of
each SQE as long as it's not setting any of these flags that affect
their order. (Please correct me if that's not what you remember.)
Well, my opinions are my own. I think it's reasonable to assume that
for optimisation purposes IFF the user space can sanely handle
errors when the assumption fails.
In your case, the user space should expect that an unregistration
op can happen before a read/write had resolved the buffer (node), in
which case the rw request will find that the buffer slot is empty
and fail. And that should be handled in the user space, e.g.
by reissuing the rw request of failing.
I don't have a strong opinion about whether or not io_uring should
provide this guarantee, but I was under the impression this had
already been decided. I was just trying to fix the few gaps in this
I don't think so, it's a major uapi change, and a huge burden
for many future core io_uring changes.
guarantee, but I'm fine dropping the patches if Jens also feels
userspace shouldn't rely on this io_uring behavior.
There is an api to order requests, if you want to order them you
either have to use that or do it in user space. In your particular
case you can try to opportunistically issue them without ordering
by making sure the reg buffer slot is not reused in the meantime
and handling request failures.
Yes, I am aware of the other options. Unfortunately, io_uring's linked
operation interface isn't rich enough to express an arbitrary
dependency graph. We have multiple I/O operations operating on the
same ublk request's buffer, so we would either need to link the I/O
operations (which would prevent them from executing in parallel), or
use a separate register/unregister operation for every I/O operation
(which has considerable overhead). We can also wait for the completion
of the I/O operations before submitting the unregister operation, but
that adds latency to the ublk request and requires another
io_uring_enter syscall.
We are using separate registered buffer indices for each ublk request
so at least this scenario doesn't lead to data corruption. And we can
certainly handle the EFAULT when the operation goes asynchronous, but
it would be preferable not to need to do that.
--
Pavel Begunkov
