Re: IORING_CQE_F_COPIED

[Stefan Metzmacher <metze@samba.org>]

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