Re: [PATCHv2 0/6] ublk zero-copy support

On Wed, Feb 12, 2025 at 04:06:58PM +0000, Pavel Begunkov wrote:

On 2/12/25 15:28, Keith Busch wrote:

quoted

On Wed, Feb 12, 2025 at 10:29:32AM +0800, Ming Lei wrote:

quoted

It is explained in the following links:

https://lore.kernel.org/linux-block/b6211101-3f74-4dea-a880-81bb75575dbd@gmail.com/ (local)

- node kbuffer is registered in ublk uring_cmd's ->issue(), but lookup
   in RW_FIXED OP's ->prep(), and ->prep() is always called before calling
   ->issue() when the two are submitted in same io_uring_enter(), so you
   need to move io_rsrc_node_lookup() & buffer importing from RW_FIXED's ->prep()
   to ->issue() first.

I don't think that's accurate, at least in practice. In a normal flow,
we'll have this sequence:

  io_submit_sqes
    io_submit_sqe (uring_cmd ublk register)
      io_init_req
        ->prep()
      io_queue_sqe
        ->issue()
    io_submit_sqe (read/write_fixed)
      io_init_req
        ->prep()
      io_queue_sqe
       ->issue()

The first SQE is handled in its entirety before even looking at the
subsequent SQE. Since the register is first, then the read/write_fixed's
prep will have a valid index. Testing this patch series appears to show
this reliably works.

Ming describes how it works for links. This one is indeed how
non links are normally executed. Though I'd repeat it's an
implementation detail and not a part of the uapi. Interestingly,
Keith, you sent some patches changing the ordering here quite a
while ago, just as an example of how it can change.

My fault, I should have provided the link or async background.

quoted

quoted

- secondly, ->issue() order is only respected by IO_LINK, and io_uring
   can't provide such guarantee without using IO_LINK:

   Pavel explained it in the following link:

   https://lore.kernel.org/linux-block/68256da6-bb13-4498-a0e0-dce88bb32242@gmail.com/ (local)

   There are also other examples, such as, register buffer stays in one
   link chain, and the consumer OP isn't in this chain, the consumer OP
   can still be issued before issuing register_buffer.

Yep, I got that. Linking is just something I was hoping to avoid. I
understand there are conditions that can break the normal flow I'm
relying on regarding  the ordering. This hasn't appeared to be a problem
in practice, but I agree this needs to be handled.

LINK/ASYNC needs to be supported, and sometimes they are useful.

- IO_LINK is the only way for respecting IO order

  io_uring only supports non-link or link all in one batch

- ASYNC sometimes can avoid to call two ->issue() unnecessarily if you
  know that the OP can't be dealt with async way in advance, maybe not
  one problem for ublk uring_cmd, but it is helpful for some FS write
  (un-allocated write)


Thanks,
Ming