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

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

[Caleb Sander Mateos]

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.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Suggested-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/io_uring_types.h |  5 ++++-
 io_uring/net.c                 | 10 +++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index c17d2eedf478..699e2c0895ae 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -583,11 +583,14 @@ enum {
 	REQ_F_BUFFERS_COMMIT	= IO_REQ_FLAG(REQ_F_BUFFERS_COMMIT_BIT),
 	/* buf node is valid */
 	REQ_F_BUF_NODE		= IO_REQ_FLAG(REQ_F_BUF_NODE_BIT),
 	/* request has read/write metadata assigned */
 	REQ_F_HAS_METADATA	= IO_REQ_FLAG(REQ_F_HAS_METADATA_BIT),
-	/* resolve padded iovec to registered buffers */
+	/*
+	 * For vectored fixed buffers, resolve iovec to registered buffers.
+	 * For SEND_ZC, whether to import buffers (i.e. the first issue).
+	 */
 	REQ_F_IMPORT_BUFFER	= IO_REQ_FLAG(REQ_F_IMPORT_BUFFER_BIT),
 };
 
 typedef void (*io_req_tw_func_t)(struct io_kiocb *req, io_tw_token_t tw);
 
diff --git a/io_uring/net.c b/io_uring/net.c
index c87af980b98e..a7b3e2688689 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -75,11 +75,10 @@ struct io_sr_msg {
 	unsigned			nr_multishot_loops;
 	u16				flags;
 	/* initialised and used only by !msg send variants */
 	u16				buf_group;
 	bool				retry;
-	bool				imported; /* only for io_send_zc */
 	void __user			*msg_control;
 	/* used only for send zerocopy */
 	struct io_kiocb 		*notif;
 };
 
@@ -1305,11 +1304,10 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	struct io_ring_ctx *ctx = req->ctx;
 	struct io_kiocb *notif;
 
 	zc->done_io = 0;
 	zc->retry = false;
-	zc->imported = false;
 	req->flags |= REQ_F_POLL_NO_LAZY;
 
 	if (unlikely(READ_ONCE(sqe->__pad2[0]) || READ_ONCE(sqe->addr3)))
 		return -EINVAL;
 	/* we don't support IOSQE_CQE_SKIP_SUCCESS just yet */
@@ -1351,12 +1349,14 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (io_is_compat(req->ctx))
 		zc->msg_flags |= MSG_CMSG_COMPAT;
 
 	if (unlikely(!io_msg_alloc_async(req)))
 		return -ENOMEM;
-	if (req->opcode != IORING_OP_SENDMSG_ZC)
+	if (req->opcode == IORING_OP_SEND_ZC) {
+		req->flags |= REQ_F_IMPORT_BUFFER;
 		return io_send_setup(req, sqe);
+	}
 	return io_sendmsg_zc_setup(req, sqe);
 }
 
 static int io_sg_from_iter_iovec(struct sk_buff *skb,
 				 struct iov_iter *from, size_t length)
@@ -1447,12 +1447,12 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(!sock))
 		return -ENOTSOCK;
 	if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
 		return -EOPNOTSUPP;
 
-	if (!zc->imported) {
-		zc->imported = true;
+	if (req->flags & REQ_F_IMPORT_BUFFER) {
+		req->flags &= ~REQ_F_IMPORT_BUFFER;
 		ret = io_send_zc_import(req, issue_flags);
 		if (unlikely(ret))
 			return ret;
 	}
 
-- 
2.45.2

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

[Pavel Begunkov]

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.

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> Suggested-by: Pavel Begunkov <asml.silence@gmail.com>

Note for the future, it's a good practice to put your sob last.

-- 
Pavel Begunkov

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

[Caleb Sander Mateos]

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.

>
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

Thanks!

>
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > Suggested-by: Pavel Begunkov <asml.silence@gmail.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.

Best,
Caleb

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

[Jens Axboe]

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!

>> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> Thanks!
> 
>>
>>> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
>>> Suggested-by: Pavel Begunkov <asml.silence@gmail.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.

-- 
Jens Axboe

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

[Caleb Sander Mateos]

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

>
> >> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> >
> > Thanks!
> >
> >>
> >>> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> >>> Suggested-by: Pavel Begunkov <asml.silence@gmail.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

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

[Jens Axboe]

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.

-- 
Jens Axboe

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@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

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

[Pavel Begunkov]

On 3/26/25 17:05, Jens Axboe 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!
> 
>>> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
>>
>> Thanks!
>>
>>>
>>>> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
>>>> Suggested-by: Pavel Begunkov <asml.silence@gmail.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.

Minor, yes. But to answer why, because it's normally chronological.
By default I read it as Suggested-by was added later and not by the
patch author, which nobody cares too much about, but that's why Jens
mentions ordering b/w sob of other people.

-- 
Pavel Begunkov

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

[Jens Axboe]

On Tue, 25 Mar 2025 08:39:42 -0600, 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.
> 
> 

Applied, thanks!

[1/1] io_uring/net: use REQ_F_IMPORT_BUFFER for send_zc
      commit: 73b6dacb1c6feae8ca4a6ff120848430aeb57fbd

Best regards,
-- 
Jens Axboe