Re: [PATCH v2] io_uring/net: use REQ_F_IMPORT_BUFFER for send_zc

[Pavel Begunkov]

On 3/26/25 17:31, Jens Axboe wrote:
> On 3/26/25 11:23 AM, Caleb Sander Mateos wrote:
> > On Wed, Mar 26, 2025 at 10:05?AM Jens Axboe <axboe@xxxxxxxxx> wrote:
> > > On 3/26/25 11:01 AM, Caleb Sander Mateos wrote:
> > > > On Wed, Mar 26, 2025 at 2:59?AM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
> > > > > On 3/25/25 14:39, Caleb Sander Mateos wrote:
> > > > > > Instead of a bool field in struct io_sr_msg, use REQ_F_IMPORT_BUFFER to
> > > > > > track whether io_send_zc() has already imported the buffer. This flag
> > > > > > already serves a similar purpose for sendmsg_zc and {read,write}v_fixed.
> > > > >
> > > > > It didn't apply cleanly to for-6.15/io_uring-reg-vec, but otherwise
> > > > > looks good.
> > > >
> > > > It looks like Jens dropped my earlier patch "io_uring/net: import
> > > > send_zc fixed buffer before going async":
> > > > https://lore.kernel.org/io-uring/20250321184819.3847386-3-csander@xxxxxxxxxxxxxxx/T/#u
> > > > .
> > > > Not sure why it was dropped. But this change is independent, I can
> > > > rebase it onto the current for-6.15/io_uring-reg-vec if desired.
> > >
> > > Mostly just around the discussion on what we want to guarantee here. I
> > > do think that patch makes sense, fwiw!
> >
> > I hope the approach I took for the revised NVMe passthru patch [1] is
> > an acceptable compromise: the order in which io_uring issues
> > operations isn't guaranteed, but userspace may opportunistically
> > submit operations in parallel with a fallback path in case of failure.
> > Viewed this way, I think it makes sense for the kernel to allow the
> > operation using the fixed buffer to succeed even if it goes async,
> > provided that it doesn't impose any burden on the io_uring
> > implementation. I dropped the "Fixes" tag and added a paragraph to the
> > commit message clarifying that io_uring doesn't guarantee this
> > behavior, it's just an optimization.
> >
> > [1]: https://lore.kernel.org/io-uring/20250324200540.910962-4-csander@xxxxxxxxxxxxxxx/T/#u
>
> It is, I already signed off on that one, I think it's just waiting for
> Keith to get queued up. Always a bit tricky during the merge window,
> particularly when it ends up depending on multiple branches. But should
> go in for 6.15.
>
> When you have time, resending the net one would be useful. I do think
> that one makes sense too.

If that's about "io_uring/net: import send_zc fixed buffer before going
async" please don't, because the next second you'll be arguing that
it's a regression to change it and so it's essentially uapi, and we
end up with 2 step prep with semantics nobody ever will be able to
sanely describe, not without listing all the cases where it can fail.

--
Pavel Begunkov