[PATCH v2 0/6] compile out ctx->compat reads

[PATCH v2 0/6] compile out ctx->compat reads

[Pavel Begunkov]

Some code paths read ctx->compat even for !CONFIG_COMPAT, add and use
a helper to optimise that out. Namely cmd and rw.c vector imports
benefit from that, and others are converted for consistency.

rsrc.c is left out to avoid conflicts, it's easier to update it later.
It'd also be a good idea to further clean up compat code on top. 

v2: also use it for net, cmd and waitid
    remove some of the CONFIG_COMPAT guards from rw.c

Pavel Begunkov (6):
  io_uring: introduce io_is_compat()
  io_uring/cmd: optimise !CONFIG_COMPAT flags setting
  io_uring/rw: compile out compat param passing
  io_uring/rw: shrink io_iov_compat_buffer_select_prep
  io_uring/waitid: use io_is_compat()
  io_uring/net: use io_is_compat()

 io_uring/io_uring.h  |  5 +++++
 io_uring/net.c       | 19 ++++++++-----------
 io_uring/rw.c        | 22 ++++++----------------
 io_uring/uring_cmd.c |  2 +-
 io_uring/waitid.c    |  2 +-
 5 files changed, 21 insertions(+), 29 deletions(-)

-- 
2.48.1

[PATCH v2 1/6] io_uring: introduce io_is_compat()

[Pavel Begunkov]

A preparation patch adding a simple helper for gauging the compat state.
It'll help us to optimise and compile out more code in the following
commits.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 650f81dac5d0..daf0e3b740ee 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -147,6 +147,11 @@ static inline void io_lockdep_assert_cq_locked(struct io_ring_ctx *ctx)
 #endif
 }
 
+static inline bool io_is_compat(struct io_ring_ctx *ctx)
+{
+	return IS_ENABLED(CONFIG_COMPAT) && unlikely(ctx->compat);
+}
+
 static inline void io_req_task_work_add(struct io_kiocb *req)
 {
 	__io_req_task_work_add(req, 0);
-- 
2.48.1

[PATCH v2 2/6] io_uring/cmd: optimise !CONFIG_COMPAT flags setting

[Pavel Begunkov]

Use io_is_compat() to avoid extra overhead in io_uring_cmd() for flag
setting when compat is compiled out.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/uring_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 8bdf2c9b3fef..14086a266461 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -237,7 +237,7 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
 		issue_flags |= IO_URING_F_SQE128;
 	if (ctx->flags & IORING_SETUP_CQE32)
 		issue_flags |= IO_URING_F_CQE32;
-	if (ctx->compat)
+	if (io_is_compat(ctx))
 		issue_flags |= IO_URING_F_COMPAT;
 	if (ctx->flags & IORING_SETUP_IOPOLL) {
 		if (!file->f_op->uring_cmd_iopoll)
-- 
2.48.1

[PATCH v2 3/6] io_uring/rw: compile out compat param passing

[Pavel Begunkov]

Even when COMPAT is compiled out, we still have to pass
ctx->compat to __import_iovec(). Replace the read with an indirection
with a constant when the kernel doesn't support compat.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/rw.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/io_uring/rw.c b/io_uring/rw.c
index 16f12f94943f..7133029b4396 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -46,7 +46,6 @@ static bool io_file_supports_nowait(struct io_kiocb *req, __poll_t mask)
 	return false;
 }
 
-#ifdef CONFIG_COMPAT
 static int io_iov_compat_buffer_select_prep(struct io_rw *rw)
 {
 	struct compat_iovec __user *uiov;
@@ -63,7 +62,6 @@ static int io_iov_compat_buffer_select_prep(struct io_rw *rw)
 	rw->len = clen;
 	return 0;
 }
-#endif
 
 static int io_iov_buffer_select_prep(struct io_kiocb *req)
 {
@@ -74,10 +72,8 @@ static int io_iov_buffer_select_prep(struct io_kiocb *req)
 	if (rw->len != 1)
 		return -EINVAL;
 
-#ifdef CONFIG_COMPAT
-	if (req->ctx->compat)
+	if (io_is_compat(req->ctx))
 		return io_iov_compat_buffer_select_prep(rw);
-#endif
 
 	uiov = u64_to_user_ptr(rw->addr);
 	if (copy_from_user(&iov, uiov, sizeof(*uiov)))
@@ -120,7 +116,7 @@ static int __io_import_iovec(int ddir, struct io_kiocb *req,
 		nr_segs = 1;
 	}
 	ret = __import_iovec(ddir, buf, sqe_len, nr_segs, &iov, &io->iter,
-				req->ctx->compat);
+				io_is_compat(req->ctx));
 	if (unlikely(ret < 0))
 		return ret;
 	if (iov) {
-- 
2.48.1

[PATCH v2 4/6] io_uring/rw: shrink io_iov_compat_buffer_select_prep

[Pavel Begunkov]

Compat performance is not important and simplicity is more appreciated.
Let's not be smart about it and use simpler copy_from_user() instead of
access + __get_user pair.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/rw.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/io_uring/rw.c b/io_uring/rw.c
index 7133029b4396..22612a956e75 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -48,18 +48,12 @@ static bool io_file_supports_nowait(struct io_kiocb *req, __poll_t mask)
 
 static int io_iov_compat_buffer_select_prep(struct io_rw *rw)
 {
-	struct compat_iovec __user *uiov;
-	compat_ssize_t clen;
+	struct compat_iovec __user *uiov = u64_to_user_ptr(rw->addr);
+	struct compat_iovec iov;
 
-	uiov = u64_to_user_ptr(rw->addr);
-	if (!access_ok(uiov, sizeof(*uiov)))
-		return -EFAULT;
-	if (__get_user(clen, &uiov->iov_len))
+	if (copy_from_user(&iov, uiov, sizeof(iov)))
 		return -EFAULT;
-	if (clen < 0)
-		return -EINVAL;
-
-	rw->len = clen;
+	rw->len = iov.iov_len;
 	return 0;
 }
 
-- 
2.48.1

[PATCH v2 5/6] io_uring/waitid: use io_is_compat()

[Pavel Begunkov]

Use io_is_compat() for consistency.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/waitid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/waitid.c b/io_uring/waitid.c
index 347b8f53efa7..4034b7e3026f 100644
--- a/io_uring/waitid.c
+++ b/io_uring/waitid.c
@@ -78,7 +78,7 @@ static bool io_waitid_copy_si(struct io_kiocb *req, int signo)
 		return true;
 
 #ifdef CONFIG_COMPAT
-	if (req->ctx->compat)
+	if (io_is_compat(req->ctx))
 		return io_waitid_compat_copy_si(iw, signo);
 #endif
 
-- 
2.48.1

[PATCH v2 6/6] io_uring/net: use io_is_compat()

[Pavel Begunkov]

Use io_is_compat() for consistency.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index d22fa61539a3..e795253632d1 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -325,7 +325,7 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
 	iomsg->msg.msg_iter.nr_segs = 0;
 
 #ifdef CONFIG_COMPAT
-	if (unlikely(req->ctx->compat)) {
+	if (io_is_compat(req->ctx)) {
 		struct compat_msghdr cmsg;
 
 		ret = io_compat_msg_copy_hdr(req, iomsg, &cmsg, ITER_SOURCE);
@@ -436,10 +436,9 @@ int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		req->buf_list = NULL;
 	}
 
-#ifdef CONFIG_COMPAT
-	if (req->ctx->compat)
+	if (io_is_compat(req->ctx))
 		sr->msg_flags |= MSG_CMSG_COMPAT;
-#endif
+
 	if (unlikely(!io_msg_alloc_async(req)))
 		return -ENOMEM;
 	if (req->opcode != IORING_OP_SENDMSG)
@@ -725,7 +724,7 @@ static int io_recvmsg_copy_hdr(struct io_kiocb *req,
 	iomsg->msg.msg_iter.nr_segs = 0;
 
 #ifdef CONFIG_COMPAT
-	if (unlikely(req->ctx->compat)) {
+	if (io_is_compat(req->ctx)) {
 		struct compat_msghdr cmsg;
 
 		ret = io_compat_msg_copy_hdr(req, iomsg, &cmsg, ITER_DEST);
@@ -837,10 +836,9 @@ int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 			return -EINVAL;
 	}
 
-#ifdef CONFIG_COMPAT
-	if (req->ctx->compat)
+	if (io_is_compat(req->ctx))
 		sr->msg_flags |= MSG_CMSG_COMPAT;
-#endif
+
 	sr->nr_multishot_loops = 0;
 	return io_recvmsg_prep_setup(req);
 }
@@ -1367,10 +1365,9 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (zc->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
-#ifdef CONFIG_COMPAT
-	if (req->ctx->compat)
+	if (io_is_compat(req->ctx))
 		zc->msg_flags |= MSG_CMSG_COMPAT;
-#endif
+
 	if (unlikely(!io_msg_alloc_async(req)))
 		return -ENOMEM;
 	if (req->opcode != IORING_OP_SENDMSG_ZC)
-- 
2.48.1

Re: [PATCH v2 1/6] io_uring: introduce io_is_compat()

[Anuj gupta]

Looks good:
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH v2 2/6] io_uring/cmd: optimise !CONFIG_COMPAT flags setting

[Anuj gupta]

Looks good:
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH v2 3/6] io_uring/rw: compile out compat param passing

[Anuj gupta]

Looks good:
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH v2 5/6] io_uring/waitid: use io_is_compat()

[Anuj gupta]

Looks good:
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH v2 6/6] io_uring/net: use io_is_compat()

[Anuj gupta]

Looks good:
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH v2 0/6] compile out ctx->compat reads

[Jens Axboe]

On Mon, 24 Feb 2025 12:42:18 +0000, Pavel Begunkov wrote:
> Some code paths read ctx->compat even for !CONFIG_COMPAT, add and use
> a helper to optimise that out. Namely cmd and rw.c vector imports
> benefit from that, and others are converted for consistency.
> 
> rsrc.c is left out to avoid conflicts, it's easier to update it later.
> It'd also be a good idea to further clean up compat code on top.
> 
> [...]

Applied, thanks!

[1/6] io_uring: introduce io_is_compat()
      commit: 3035deac0cd5bd9c8cacdcf5a1c488cbc87abc2d
[2/6] io_uring/cmd: optimise !CONFIG_COMPAT flags setting
      commit: 0bba6fccbdcb28d284debc31150f84ef14f7e252
[3/6] io_uring/rw: compile out compat param passing
      commit: 82d187d356dcc257ecaa659e57e6c0546ec1cd2d
[4/6] io_uring/rw: shrink io_iov_compat_buffer_select_prep
      commit: 52524b281d5746cf9dbd53a7dffce9576e8ddd30
[5/6] io_uring/waitid: use io_is_compat()
      commit: d8cc800bac886fac69cc672545eee89dbeb50bef
[6/6] io_uring/net: use io_is_compat()
      commit: 5e646339944c10d615b1b0b5a2fa8a4f6734f21d

Best regards,
-- 
Jens Axboe

Re: [PATCH v2 5/6] io_uring/waitid: use io_is_compat()

[Caleb Sander Mateos]

On Mon, Feb 24, 2025 at 4:48 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> Use io_is_compat() for consistency.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  io_uring/waitid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/io_uring/waitid.c b/io_uring/waitid.c
> index 347b8f53efa7..4034b7e3026f 100644
> --- a/io_uring/waitid.c
> +++ b/io_uring/waitid.c
> @@ -78,7 +78,7 @@ static bool io_waitid_copy_si(struct io_kiocb *req, int signo)
>                 return true;
>
>  #ifdef CONFIG_COMPAT
> -       if (req->ctx->compat)
> +       if (io_is_compat(req->ctx))
>                 return io_waitid_compat_copy_si(iw, signo);
>  #endif

Would it be possible to remove the #ifdef CONFIG_COMPAT here (and
around io_waitid_compat_copy_si()), like you did in rw.c? The compiler
should be able to optimize out the if (false) and the unused static
function. Same comment for the remaining uses of #ifdef CONFIG_COMPAT
in net.c.

Best,
Caleb


>
> --
> 2.48.1
>
>

Re: [PATCH v2 5/6] io_uring/waitid: use io_is_compat()

[Pavel Begunkov]

On 2/24/25 15:33, Caleb Sander Mateos wrote:
> On Mon, Feb 24, 2025 at 4:48 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> Use io_is_compat() for consistency.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   io_uring/waitid.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/io_uring/waitid.c b/io_uring/waitid.c
>> index 347b8f53efa7..4034b7e3026f 100644
>> --- a/io_uring/waitid.c
>> +++ b/io_uring/waitid.c
>> @@ -78,7 +78,7 @@ static bool io_waitid_copy_si(struct io_kiocb *req, int signo)
>>                  return true;
>>
>>   #ifdef CONFIG_COMPAT
>> -       if (req->ctx->compat)
>> +       if (io_is_compat(req->ctx))
>>                  return io_waitid_compat_copy_si(iw, signo);
>>   #endif
> 
> Would it be possible to remove the #ifdef CONFIG_COMPAT here (and
> around io_waitid_compat_copy_si()), like you did in rw.c? The compiler
> should be able to optimize out the if (false) and the unused static
> function. Same comment for the remaining uses of #ifdef CONFIG_COMPAT
> in net.c.

Likely so, I hinted on the same in the cv as well, but it doesn't
have to happen in a single set. If anything, I'd prefer to flush this
now, so that the dependency is merged and everything else can
be done independently.

-- 
Pavel Begunkov