On Wed, Feb 12, 2025 at 04:06:58PM +0000, Pavel Begunkov wrote: > On 2/12/25 15:28, Keith Busch wrote: > > 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@xxxxxxxxx/ > > > > > > - 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. > > > > > - 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@xxxxxxxxx/ > > > > > > 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
