Re: [PATCH v1 12/15] io_uring/zcrx: add io_recvzc request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/9/24 19:28, Jens Axboe wrote:
diff --git a/io_uring/net.c b/io_uring/net.c
index d08abcca89cc..482e138d2994 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1193,6 +1201,76 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
  	return ret;
  }
+int io_recvzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_recvzc *zc = io_kiocb_to_cmd(req, struct io_recvzc);
+	unsigned ifq_idx;
+
+	if (unlikely(sqe->file_index || sqe->addr2 || sqe->addr ||
+		     sqe->len || sqe->addr3))
+		return -EINVAL;
+
+	ifq_idx = READ_ONCE(sqe->zcrx_ifq_idx);
+	if (ifq_idx != 0)
+		return -EINVAL;
+	zc->ifq = req->ctx->ifq;
+	if (!zc->ifq)
+		return -EINVAL;

This is read and assigned to 'zc' here, but then the issue handler does
it again? I'm assuming that at some point we'll have ifq selection here,
and then the issue handler will just use zc->ifq. So this part should
probably remain, and the issue side just use zc->ifq?

Yep, fairly overlooked. It's not a real problem, but should
only be fetched and checked here.

+	/* All data completions are posted as aux CQEs. */
+	req->flags |= REQ_F_APOLL_MULTISHOT;

This puzzles me a bit...

Well, it's a multishot request. And that flag protects from cq
locking rules violations, i.e. avoiding multishot reqs from
posting from io-wq.

+	zc->flags = READ_ONCE(sqe->ioprio);
+	zc->msg_flags = READ_ONCE(sqe->msg_flags);
+	if (zc->msg_flags)
+		return -EINVAL;

Maybe allow MSG_DONTWAIT at least? You already pass that in anyway.

What would the semantics be? The io_uring nowait has always
been a pure mess because it's not even clear what it supposed
to mean for async requests.


+	if (zc->flags & ~(IORING_RECVSEND_POLL_FIRST | IORING_RECV_MULTISHOT))
+		return -EINVAL;
+
+
+#ifdef CONFIG_COMPAT
+	if (req->ctx->compat)
+		zc->msg_flags |= MSG_CMSG_COMPAT;
+#endif
+	return 0;
+}

Heh, we could probably just return -EINVAL for that case, but since this
is all we need, fine.

Well, there is no msghdr, cmsg nor iovec there, so doesn't even
make sense to set it. Can fail as well, I don't anyone would care.

--
Pavel Begunkov




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux