Hi Pavel,
As I basically use the same logic that's used to generate SO_EE_CODE_ZEROCOPY_COPIED
for the native MSG_ZEROCOPY, I don't see the problem with IORING_CQE_F_COPIED.
Can you be more verbose why you're thinking about something different?
Because it feels like something that should be done roughly once and in
advance. Performance wise, I agree that a bunch of extra instructions in
the (io_uring) IO path won't make difference as the net overhead is
already high, but I still prefer to keep it thin. The complexity is a
good point though, if only we could piggy back it onto MSG_PROBE.
Ok, let's do IORING_CQE_F_COPIED and aim 6.2 + possibly backport.
Thanks!
Experimenting with this stuff lets me wish to have a way to
have a different 'user_data' field for the notif cqe,
maybe based on a IORING_RECVSEND_ flag, it may make my life
easier and would avoid some complexity in userspace...
As I need to handle retry on short writes even with MSG_WAITALL
as EINTR and other errors could cause them.
What do you think?
First, there is no more ubuf_info::zerocopy, see for-next, but you can
grab space in io_kiocb, io_kiocb::iopoll_completed is a good candidate.
Ok I found your "net: introduce struct ubuf_info_msgzc" and
"net: shrink struct ubuf_info" commits.
I think the change would be trivial, the zerocopy field would just move
to struct io_notif_data..., maybe as 'bool copied'.
You would want to take one io_uring patch I'm going to send (will CC
you), with that you won't need to change anything in net/.
The problem is that e.g. tcp_sendmsg_locked() won't ever call
the callback at all if 'zc' is false.
That's why there's the:
if (!zc)
uarg->zerocopy = 0;
Maybe I can inverse the logic and use two variables 'zero_copied'
and 'copied'.
We'd start with both being false and this logic in the callback:
if (success) {
if (unlikely(!nd->zero_copied && !nd->copied))
nd->zero_copied = true;
} else {
if (unlikely(!nd->copied)) {
nd->copied = true;
nd->zero_copied = false;
}
}
And __io_notif_complete_tw still needs:
if (!nd->zero_copied)
notif->cqe.flags |= IORING_CQE_F_COPIED;
instead of if (nd->copied)
And the last bit, let's make the zc probing conditional under IORING_RECVSEND_* flag,
I'll make it zero overhead when not set later by replacing the callback.
And the if statement to select a highspeed callback based on
a IORING_RECVSEND_ flag is less overhead than
the if statements in the slow callback version?
metze