linux-kernel - Re: [PATCH v2] io_uring/net: use REQ_F_IMPORT_BUFFER for send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADUfDZr0FgW4O3bCtq=Yez2cHz799=Tfud6uA6SHEGT4hdwxiA@mail.gmail.com>
Date: Wed, 26 Mar 2025 10:23:12 -0700
From: Caleb Sander Mateos <csander@...estorage.com>
To: Jens Axboe <axboe@...nel.dk>
Cc: Pavel Begunkov <asml.silence@...il.com>, io-uring@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] io_uring/net: use REQ_F_IMPORT_BUFFER for send_zc

On Wed, Mar 26, 2025 at 10:05 AM Jens Axboe <axboe@...nel.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@...il.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

>
> >> Reviewed-by: Pavel Begunkov <asml.silence@...il.com>
> >
> > Thanks!
> >
> >>
> >>> Signed-off-by: Caleb Sander Mateos <csander@...estorage.com>
> >>> Suggested-by: Pavel Begunkov <asml.silence@...il.com>
> >>
> >> Note for the future, it's a good practice to put your sob last.
> >
> > Okay. Is the preferred order of tags documented anywhere? I ran
> > scripts/checkpatch.pl, but it didn't have any complaints.
>
> I think that one is minor, as it's not reordering with another SOB. Eg
> mine would go below it anyway. But you definitely should always include
> a list of what changed since v1 when posting v2, and so forth. Otherwise
> you need to find the old patch and compare them to see what changed.
> Just put it below the --- line in the email.

Sorry about that, it slipped my mind. Will try to remember next time!

Thanks,
Caleb