On 9/28/22 17:56, Pavel Begunkov wrote:
On 9/28/22 16:23, Stefan Metzmacher wrote:
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?
Right
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()
Took a closer look, that chunk above looks good and matches
io_req_map_rw() apart from non essential differences. Can you
send a patch?
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?
We can reuse it, we own it and it's immutable from
the iter perspective.
BTW: I tested with 5 vectors with length like this 4, 0, 64, 32, 8388608
and got a short write with about ~ 2000000.
Which is interesting to know. What does 2M here mean? Is it
consistently retries when sending more than 2M bytes?
--
Pavel Begunkov
