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

Keith Busch
2025-02-12T15:28:54Z

On Wed, Feb 12, 2025 at 10:29:32AM +0800, Ming Lei wrote:
> It is explained in the following links:
>
> https://lore.kernel.org/linux-block/b6211101-3f74-4dea-a880-81bb75575dbd@gmail.com/
>
> - 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.

> - 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/
>
> 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.