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

[Pavel Begunkov <asml.silence@gmail.com>]

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@kernel.dk> 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@gmail.com> 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@purestorage.com/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@purestorage.com/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