On 9/28/22 16:23, Stefan Metzmacher wrote:
Hi Pavel,
We currently only add a notification CQE when the send succeded, i.e.
cqe.res >= 0. However, it'd be more robust to do buffer notifications
for failed requests as well in case drivers decide do something fanky.
Always return a buffer notification after initial prep, don't hide it.
This behaviour is better aligned with documentation and the patch also
helps the userspace to respect it.
Just as reference, this was the version I was testing with:
https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=7ffb896cdb8ccd55065f7ffae9fb8050e39211c7
void io_sendrecv_fail(struct io_kiocb *req)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- int res = req->cqe.res;
if (req->flags & REQ_F_PARTIAL_IO)
- res = sr->done_io;
+ req->cqe.res = sr->done_io;
+
if ((req->flags & REQ_F_NEED_CLEANUP) &&
- (req->opcode == IORING_OP_SEND_ZC || req->opcode == IORING_OP_SENDMSG_ZC)) {
- /* preserve notification for partial I/O */
- if (res < 0)
- sr->notif->flags |= REQ_F_CQE_SKIP;
- io_notif_flush(sr->notif);
- sr->notif = NULL;
Here we rely on io_send_zc_cleanup(), correct?
Note that I hit a very bad problem during my tests of SENDMSG_ZC.
BUG(); in first_iovec_segment() triggered very easily.
The problem is io_setup_async_msg() in the partial retry case,
which seems to happen more often with _ZC.
if (!async_msg->free_iov)
async_msg->msg.msg_iter.iov = async_msg->fast_iov;
Is wrong it needs to be something like this:
+ if (!kmsg->free_iov) {
+ size_t fast_idx = kmsg->msg.msg_iter.iov - kmsg->fast_iov;
+ async_msg->msg.msg_iter.iov = &async_msg->fast_iov[fast_idx];
+ }
I agree, it doesn't look right. It indeed needs sth like
io_uring/rw.c:io_req_map_rw()
As iov_iter_iovec_advance() may change i->iov in order to have i->iov_offset
being only relative to the first element.
I'm not sure about the 'kmsg->free_iov' case, do we reuse the
callers memory or should we make a copy?
I initially used this
https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=e1d3a9f5c7708a37172d258753ed7377eaac9e33
But I didn't test with the non-fast_iov case.
BTW: I tested with 5 vectors with length like this 4, 0, 64, 32, 8388608
and got a short write with about ~ 2000000.
I'm not sure if it was already a problem before:
commit 257e84a5377fbbc336ff563833a8712619acce56
io_uring: refactor sendmsg/recvmsg iov managing
But I guess it was a potential problem before starting with
7ba89d2af17aa879dda30f5d5d3f152e587fc551 where io_net_retry()
was introduced.
metze
--
Pavel Begunkov
