Re: [PATCH v7 1/4] io_uring: Add io_uring04 test for vectored I/O operations
Sebastian Chlad <[email protected]>
| Newsgroups | gmane.linux.ltp |
|---|---|
| Message-ID | <CAJCsO8fQOQfa08bN92zkbqhU_B0B0Ojx2xRm0+TA6jP6WSkZ5g@mail.gmail.com> |
Hi Sachin On Tue, 19 May 2026 at 10:45, Sachin Sant <[email protected]> wrote: > A gentle reminder. > > Let me know if there are any additional review comments > on this patch series. > I see some fundamental problems here which admittedly started before this series. I believe these need to be fixed before we apply more tests on top of it. In io_uring_submit_sqe() and io_uring_submit_sqe_vec(), opcode is passed as user_data: io_uring_submit_sqe_internal(s, fd, opcode, (unsigned long)buf, len, offset, opcode); user_data is supposed to be a unique per-request identifier for correlating CQEs back to their SQEs. Using opcode there is meaningless as a tag. The existing tests happen to pass because io_uring_do_io_op() and io_uring_do_vec_io_op() always submit exactly one SQE and immediately block waiting for its CQE, so correlation is never actually needed. However, any test with multiple in-flight SQEs of the same opcode would pass; the user_data check in io_uring_wait-cqe() would match regardless of which CQE corresponds to which request. Please hold off on applying this series until the underlying issue is fixed. I will send a patch to correct the user_data handling. I also think io_uring_do_io_op() and io_uring_do_vec_io_op() should not tightly couple SQE submission with waiting for a CQE. Keeping them as a single helper makes the tests effectively synchronous and does not model real io_uring usage. It would be cleaner to have separate helpers for submitting SQEs and waiting/reaping CQEs, and let each test handle CQE validation and result reporting in its own body. Having basic smoke tests, sending single SQE and checking CQE makes sense in LTP, but I doubt we shall have this as a helper. > -- > Thanks > - Sachin > > > Thanks and let me know if I misunderstand something here. It's the first time I look at this code really. Sebastian > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > -- Mailing list info: https://lists.linux.it/listinfo/ltp