[PATCH for-next 0/4] io_uring: multishot recv cleanups

[PATCH for-next 0/4] io_uring: multishot recv cleanups

[Dylan Yudaken]

These are some preparatory cleanups that are separate but required for a
later series doing multishot recvmsg (will post this shortly).

Patches:
1: fixes a bug where a socket may receive data before polling
2: makes a similar change to compat logic for providing no iovs
for buffer_select
3/4: move the recycling logic into the io_uring main framework which makes
it a bit easier for recvmsg multishot

Dylan Yudaken (4):
  io_uring: fix multishot ending when not polled
  io_uring: support 0 length iov in buffer select in compat
  io-uring: add recycle_async to ops
  io_uring: move netmsg recycling into io_uring cleanup

 io_uring/io_uring.c |  8 ++++++--
 io_uring/net.c      | 35 ++++++++++++++++++++---------------
 io_uring/net.h      |  1 +
 io_uring/opdef.c    |  2 ++
 io_uring/opdef.h    |  1 +
 5 files changed, 30 insertions(+), 17 deletions(-)


base-commit: 8007202a9a4854eb963f1282953b1c83e91b8253
-- 
2.30.2

[PATCH for-next 1/4] io_uring: fix multishot ending when not polled

[Dylan Yudaken]

If multishot is not actually polling then return IOU_OK rather than the
result.
If the result was > 0 this will confuse things further up the callstack
which expect a return <= 0.

Fixes: 1300ebb20286 ("io_uring: multishot recv")
Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 io_uring/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/net.c b/io_uring/net.c
index ba7e94ff287c..188edbed1eb7 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -508,6 +508,8 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret, unsigned int c
 
 	if (req->flags & REQ_F_POLLED)
 		*ret = IOU_STOP_MULTISHOT;
+	else
+		*ret = IOU_OK;
 	return true;
 }
 
-- 
2.30.2

[PATCH for-next 2/4] io_uring: support 0 length iov in buffer select in compat

[Dylan Yudaken]

Match up work done in "io_uring: allow iov_len = 0 for recvmsg and buffer
select", but for compat code path.

Fixes: a68caad69ce5 ("io_uring: allow iov_len = 0 for recvmsg and buffer select")
Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 io_uring/net.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 188edbed1eb7..997c17512694 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -384,16 +384,21 @@ static int __io_compat_recvmsg_copy_hdr(struct io_kiocb *req,
 	if (req->flags & REQ_F_BUFFER_SELECT) {
 		compat_ssize_t clen;
 
-		if (len > 1)
-			return -EINVAL;
-		if (!access_ok(uiov, sizeof(*uiov)))
-			return -EFAULT;
-		if (__get_user(clen, &uiov->iov_len))
-			return -EFAULT;
-		if (clen < 0)
+		if (len == 0) {
+			sr->len = 0;
+			iomsg->free_iov = NULL;
+		} else if (len > 1) {
 			return -EINVAL;
-		sr->len = clen;
-		iomsg->free_iov = NULL;
+		} else {
+			if (!access_ok(uiov, sizeof(*uiov)))
+				return -EFAULT;
+			if (__get_user(clen, &uiov->iov_len))
+				return -EFAULT;
+			if (clen < 0)
+				return -EINVAL;
+			sr->len = clen;
+			iomsg->free_iov = NULL;
+		}
 	} else {
 		iomsg->free_iov = iomsg->fast_iov;
 		ret = __import_iovec(READ, (struct iovec __user *)uiov, len,
-- 
2.30.2

[PATCH for-next 3/4] io-uring: add recycle_async to ops

[Dylan Yudaken]

Some ops recycle or cache async data when they are done.
Build this into the framework for async data so that we can rely on
io_uring to know when a request is done (rather than doing it inline in a
handler).

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 io_uring/io_uring.c | 8 ++++++--
 io_uring/opdef.h    | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 32110c5b4059..3e9fdaf9c72c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1578,8 +1578,12 @@ static void io_clean_op(struct io_kiocb *req)
 	if (req->flags & REQ_F_CREDS)
 		put_cred(req->creds);
 	if (req->flags & REQ_F_ASYNC_DATA) {
-		kfree(req->async_data);
-		req->async_data = NULL;
+		const struct io_op_def *def = &io_op_defs[req->opcode];
+
+		if (!(def->recycle_async && def->recycle_async(req))) {
+			kfree(req->async_data);
+			req->async_data = NULL;
+		}
 	}
 	req->flags &= ~IO_REQ_CLEAN_FLAGS;
 }
diff --git a/io_uring/opdef.h b/io_uring/opdef.h
index ece8ed4f96c4..3de64731fc5f 100644
--- a/io_uring/opdef.h
+++ b/io_uring/opdef.h
@@ -34,6 +34,7 @@ struct io_op_def {
 	int (*issue)(struct io_kiocb *, unsigned int);
 	int (*prep_async)(struct io_kiocb *);
 	void (*cleanup)(struct io_kiocb *);
+	bool (*recycle_async)(struct io_kiocb *);
 };
 
 extern const struct io_op_def io_op_defs[];
-- 
2.30.2

[PATCH for-next 4/4] io_uring: move netmsg recycling into io_uring cleanup

[Dylan Yudaken]

Specifically will be useful for multishot as the lifetime of a request is
a bit more complicated, but generally makes things a bit neater.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 io_uring/net.c   | 10 ++++------
 io_uring/net.h   |  1 +
 io_uring/opdef.c |  2 ++
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 997c17512694..c1cbafe96c63 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -98,18 +98,18 @@ static bool io_net_retry(struct socket *sock, int flags)
 	return sock->type == SOCK_STREAM || sock->type == SOCK_SEQPACKET;
 }
 
-static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
+bool io_netmsg_recycle(struct io_kiocb *req)
 {
 	struct io_async_msghdr *hdr = req->async_data;
 
-	if (!hdr || issue_flags & IO_URING_F_UNLOCKED)
-		return;
-
 	if (io_alloc_cache_store(&req->ctx->netmsg_cache)) {
 		hlist_add_head(&hdr->cache_list, &req->ctx->netmsg_cache.list);
 		req->async_data = NULL;
 		req->flags &= ~REQ_F_ASYNC_DATA;
+		return true;
 	}
+
+	return false;
 }
 
 static struct io_async_msghdr *io_recvmsg_alloc_async(struct io_kiocb *req,
@@ -261,7 +261,6 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 	if (kmsg->free_iov)
 		kfree(kmsg->free_iov);
 	req->flags &= ~REQ_F_NEED_CLEANUP;
-	io_netmsg_recycle(req, issue_flags);
 	if (ret >= 0)
 		ret += sr->done_io;
 	else if (sr->done_io)
@@ -583,7 +582,6 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
 	/* fast path, check for non-NULL to avoid function call */
 	if (kmsg->free_iov)
 		kfree(kmsg->free_iov);
-	io_netmsg_recycle(req, issue_flags);
 	req->flags &= ~REQ_F_NEED_CLEANUP;
 	if (ret > 0)
 		ret += sr->done_io;
diff --git a/io_uring/net.h b/io_uring/net.h
index 576efb602c7f..c5a897e61f10 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -44,6 +44,7 @@ int io_connect_prep_async(struct io_kiocb *req);
 int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_connect(struct io_kiocb *req, unsigned int issue_flags);
 
+bool io_netmsg_recycle(struct io_kiocb *req);
 void io_flush_netmsg_cache(struct io_ring_ctx *ctx);
 #else
 static inline void io_flush_netmsg_cache(struct io_ring_ctx *ctx)
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index a7b84b43e6c2..b525e37f397a 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -153,6 +153,7 @@ const struct io_op_def io_op_defs[] = {
 		.issue			= io_sendmsg,
 		.prep_async		= io_sendmsg_prep_async,
 		.cleanup		= io_sendmsg_recvmsg_cleanup,
+		.recycle_async		= io_netmsg_recycle,
 #else
 		.prep			= io_eopnotsupp_prep,
 #endif
@@ -170,6 +171,7 @@ const struct io_op_def io_op_defs[] = {
 		.issue			= io_recvmsg,
 		.prep_async		= io_recvmsg_prep_async,
 		.cleanup		= io_sendmsg_recvmsg_cleanup,
+		.recycle_async		= io_netmsg_recycle,
 #else
 		.prep			= io_eopnotsupp_prep,
 #endif
-- 
2.30.2

Re: (subset) [PATCH for-next 0/4] io_uring: multishot recv cleanups

[Jens Axboe]

On Fri, 8 Jul 2022 11:18:34 -0700, Dylan Yudaken wrote:
> These are some preparatory cleanups that are separate but required for a
> later series doing multishot recvmsg (will post this shortly).
> 
> Patches:
> 1: fixes a bug where a socket may receive data before polling
> 2: makes a similar change to compat logic for providing no iovs
> for buffer_select
> 3/4: move the recycling logic into the io_uring main framework which makes
> it a bit easier for recvmsg multishot
> 
> [...]

Applied, thanks!

[1/4] io_uring: fix multishot ending when not polled
      commit: a8723bb79e40713f6c27564b1d17824d7ff67efb
[2/4] io_uring: support 0 length iov in buffer select in compat
      commit: 20898aeac6b82d8eb6247754494584b2f6cafd53

Best regards,
-- 
Jens Axboe

Re: [PATCH for-next 0/4] io_uring: multishot recv cleanups

[Jens Axboe]

On 7/8/22 12:18 PM, Dylan Yudaken wrote:
> These are some preparatory cleanups that are separate but required for a
> later series doing multishot recvmsg (will post this shortly).
> 
> Patches:
> 1: fixes a bug where a socket may receive data before polling
> 2: makes a similar change to compat logic for providing no iovs
> for buffer_select
> 3/4: move the recycling logic into the io_uring main framework which makes
> it a bit easier for recvmsg multishot

Applied 1-2, because I don't think the cleanup is necessarily correct. Do
we always hold ctx->uring_lock for io_clean_op()? I can see two ways
in there - one we definitely do, but from the __io_req_complete_put() or
io_free_req() path that doesn't seem to be the case. Hmm?

-- 
Jens Axboe

Re: [PATCH for-next 0/4] io_uring: multishot recv cleanups

[Dylan Yudaken]

On Mon, 2022-07-11 at 14:55 -0600, Jens Axboe wrote:
> On 7/8/22 12:18 PM, Dylan Yudaken wrote:
> > These are some preparatory cleanups that are separate but required
> > for a
> > later series doing multishot recvmsg (will post this shortly).
> > 
> > Patches:
> > 1: fixes a bug where a socket may receive data before polling
> > 2: makes a similar change to compat logic for providing no iovs
> > for buffer_select
> > 3/4: move the recycling logic into the io_uring main framework
> > which makes
> > it a bit easier for recvmsg multishot
> 
> Applied 1-2, because I don't think the cleanup is necessarily
> correct. Do
> we always hold ctx->uring_lock for io_clean_op()? I can see two ways
> in there - one we definitely do, but from the __io_req_complete_put()
> or
> io_free_req() path that doesn't seem to be the case. Hmm?
> 

You're right. Sorry about that - i will send out v2 of the recvmsg
patch series without it.