Skip to contentSkip to conversation

io_uring: thread identity handoff for blocking inline issue

RFC PATCH In review Jens Axboe 5 participants 29 messages Sep 11 – Sep 27

Subsystems (7)
x86 MM core arm64 Tracing +3 more subsystems
lore
Share & tools
lore

Jens Axboe

author maintainer
#0 lore raw

cover io_uring: thread identity handoff for blocking inline issue

Hi,

io_uring issues requests inline with IO_URING_F_NONBLOCK and punts to io-wq when that isn't possible. For a range of opcodes it isn't possible at all, as there's no nonblocking path in the kernel for them: fsync, statx, openat, the *at family, xattr, fadvise, splice, etc. Those are punted unconditionally, and the punt costs a thread wakeup, a context switch and a task_work completion round trip per request. io_uring HAS to be cautious to prevent accidental blocking in the kernel, even if the operations predominantly never block. Sad story. Examples of that are things like an fdatasync that doesn't block, statx that hits dcache, openat for O_TMPFILE, etc. All of those would've completed inline just fine, but io_uring just cannot rely on that.

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct. Folks that have been around a while may remember earlier attempts at this about 20 years ago.

We catch the blocking through a scheduler hook. A task in a blocking inline issue carries PF_IO_HANDOFF, and sched_submit_work() calls into io_uring for it, next to the existing io-wq and workqueue hooks. This is where the identity is handed off, with the uring_lock still held by the blocking task and released for the promoted worker on its behalf.

Structure of the series:

1 kernel: the thread identity handoff itself, in
kernel/thread_handoff.c. Independent of io_uring.
2 sched: the PF_IO_HANDOFF hook.
3-4 arm64 and x86 support. The arch hooks sync live register state
before the source blocks and load it on the destination.
5-9 io_uring prep: helper cleanups, a tctx node list, tracking of
uring_lock sections in the issue path so the hook knows when the lock may be dropped, splitting io_uring_enter() so it can be resumed by another task, and keeping the block plug on the submitter's stack.
10 io-wq: claiming an idle worker for a handoff, and keeping a
couple of idle spares around as targets.
11-12 io_uring: the handoff itself, and deferring the identity
migration to the end of the submission so a batch of blocking SQEs costs one migration rather than one per SQE.
13 io_uring: issue blockable requests inline in blocking mode.
This is where behavior changes.
14 tracepoints.
15 treat IOSQE_ASYNC the same way. Separate as it's a userspace
visible policy change, and I'm not sure yet it should be done.

Some things are refused for handoff up front: traced tasks, per-task perf contexts, PI futexes, audit contexts, armed per-thread CPU timers, core scheduling cookies, vfork parents. Per-thread accounting moves with the handoff, so the counters userspace sees for a tid stay monotonic. Known gaps are LSM state kept in the task rather than the cred, and PR_SET_IO_FLUSHER. Neither moves, and not moving them can only ever restrict.

Reads and writes on files with FMODE_NOWAIT are excluded. They have a working nonblocking path and poll retry, and a handoff per op would be worse than that. The handoff covers requests that previously would always have been handed to io-wq upfront. uring_cmd is excluded too, as drivers like ublk bind state to the submitting task.

Some test results:

Measured in a virtme-ng guest, 8 vcpu, non-debug x86 config, same kernel with a sysctl toggle for turning the feature on and off. CPU is the usage of the whole process including io-wq workers, as a percentage of one CPU. Mean of two runs.

ops/s cpu
=====================================================
fsync, tmpfs, qd 1
baseline 28.2k 96%
handoff 221k 100%
change +681% +5%
fsync, tmpfs, qd 8

   baseline                                195k   141%
   handoff                                 526k   100%
   change                                 +170%   -29%
 fsync, tmpfs, qd 32
   baseline                                357k   219%
   handoff                                 611k   100%
   change                                  +71%   -54%

fsync, ext4 (flushes, always blocks), qd 1

   baseline                                9.6k    66%
   handoff                                 7.1k    91%
   change                                  -26%   +37%
 fsync, ext4 (flushes, always blocks), qd 8
   baseline                               29.8k   212%
   handoff                                14.7k   146%
   change                                  -51%   -31%
 fsync, ext4 (flushes, always blocks), qd 32
   baseline                               42.2k   270%
   handoff                                14.6k   144%
   change                                  -65%   -47%

statx, ext4, qd 1
baseline 23.6k 96%
handoff 121k 100%
change +414% +5%
statx, ext4, qd 8

   baseline                                129k   234%
   handoff                                 179k   100%
   change                                  +38%   -57%
 statx, ext4, qd 32
   baseline                                197k   276%
   handoff                                 140k   100%
   change                                  -29%   -64%

statx, tmpfs, qd 1
baseline 24.4k 96%
handoff 117k 100%
change +378% +4%
statx, tmpfs, qd 8

   baseline                                131k   232%
   handoff                                 180k   100%
   change                                  +37%   -57%
 statx, tmpfs, qd 32
   baseline                                219k   295%
   handoff                                 190k   100%
   change                                  -13%   -66%

fadvise DONTNEED, ext4, qd 1
baseline 26.1k 95%
handoff 150k 100%
change +478% +5%
fadvise DONTNEED, ext4, qd 8

   baseline                                127k   170%
   handoff                                 252k   100%
   change                                  +99%   -41%
 fadvise DONTNEED, ext4, qd 32
   baseline                                270k   234%
   handoff                                 273k   100%
   change                                   +1%   -57%

renameat, ext4, qd 1
baseline 7.4k 98%
handoff 13.4k 100%
change +81% +2%
renameat, ext4, qd 8

   baseline                               12.0k   544%
   handoff                                14.6k   100%
   change                                  +21%   -82%
 renameat, ext4, qd 32
   baseline                               11.2k   500%
   handoff                                14.3k   100%
   change                                  +28%   -80%

renameat, tmpfs, qd 1

   baseline                               15.0k    97%
   handoff                                48.5k    99%
   change                                 +223%    +2%

renameat, tmpfs, qd 8
baseline 65.6k 514%
handoff 55.4k 98%
change -15% -81%
renameat, tmpfs, qd 32
baseline 39.4k 462%
handoff 38.9k 98%
change -1% -79%

openat O_TMPFILE, ext4, qd 1
baseline 6.3k 100%
handoff 11.3k 100%
change +81% +1%
openat O_TMPFILE, ext4, qd 8
baseline 23.0k 258%
handoff 12.9k 99%
change -44% -62%
openat O_TMPFILE, ext4, qd 32
baseline 26.2k 248%
handoff 13.4k 99%
change -49% -60%

openat O_TMPFILE, tmpfs, qd 1

   baseline                               13.4k    95%
   handoff                                43.0k    98%
   change                                 +222%    +3%

openat O_TMPFILE, tmpfs, qd 8
baseline 57.8k 220%
handoff 51.7k 96%
change -11% -56%
openat O_TMPFILE, tmpfs, qd 32
baseline 69.3k 218%
handoff 56.8k 96%
change -18% -56%

splice to pipe, ext4, qd 1

   baseline                               19.4k    96%
   handoff                                21.1k    98%
   change                                   +8%    +2%
 splice to pipe, ext4, qd 8
   baseline                               48.2k   176%
   handoff                                48.4k   176%
   change                                   +0%    +0%
 splice to pipe, ext4, qd 32
   baseline                               53.3k   188%
   handoff                                48.1k   182%

change -10% -3%

As you can tell, normal QD=1 type issues see big wins. Conversely, for higher queue depth, there are losses. The losses are generally from one of two reasons:

1) The syscall part is fairly expensive, and previously we farmed all of this work across a bunch of io-wq workers, and the parallelization there helps performance. For QD=1 that obviously isn't the case. The more expensive the lower level kernel parts are, the lower the QD required to see a perf loss. openat is the obvious worse case for this.

2) The syscall part ALWAYS blocks. For this case, io-wq is going to be quicker, just punt the opcode upfront. fsync on ext4, as shown in the table above, is indeed that case. Each of those block.

I've got some ideas for how to mitigate the losses for higher queue depths, but a) I didn't think they were THAT interesting for an RFC, as low QD is generally what people do with these kinds of ops, and b) the main concept behind this handoff is really the interesting part right now.

Passes the full liburing test suite, on both x86-64 and arm64. Other archs don't support this yet.

This is obviously an RFC, in terms of what I'd love people to take a closer look at:

- The scheduler hook and the identity move itself, kernel/thread_handoff.c. Is the set of refused states complete enough, and is moving thread group leadership this way (the leader must stay first on ->thread_head, like de_thread() keeps it) acceptable / kosher.
- The x86 and arm64 register state handling. x86 refuses AMX users, and arm64 refuses SME.
- Whether anyone relies on a task's user identity staying on one task_struct in ways not covered above in the series.

Patches are against 7.3-rc2. Also available at:

git://git.kernel.dk/linux.git io_uring-thread-handoff.3

 arch/Kconfig                    |   7 +
 arch/arm64/Kconfig              |   1 +
 arch/arm64/kernel/process.c     | 109 +++++++
 arch/x86/Kconfig                |   1 +
 arch/x86/kernel/process.c       |  10 +-
 arch/x86/kernel/process_64.c    | 139 +++++++++
 include/linux/io_uring.h        |   9 +
 include/linux/io_uring_types.h  |  48 +++-
 include/linux/sched.h           |   2 +-
 include/linux/thread_handoff.h  |  76 +++++
 include/trace/events/io_uring.h | 114 ++++++++
 init/Kconfig                    |  11 +
 io_uring/Makefile               |   1 +
 io_uring/handoff.c              | 395 +++++++++++++++++++++++++
 io_uring/handoff.h              | 103 +++++++
 io_uring/io-wq.c                | 270 +++++++++++++++++-
 io_uring/io-wq.h                |  15 +
 io_uring/io_uring.c             | 310 ++++++++++++++------
 io_uring/io_uring.h             |  46 ++-
 io_uring/kbuf.c                 |   5 +-
 io_uring/msg_ring.c             |   2 +-
 io_uring/opdef.c                |  29 ++
 io_uring/opdef.h                |   2 +
 io_uring/rw.c                   |   2 +-
 io_uring/splice.c               |   6 +
 io_uring/tctx.c                 |  16 +-
 io_uring/tctx.h                 |   2 +
 io_uring/tw.c                   |  14 +-
 io_uring/uring_cmd.c            |   5 +-
 kernel/Makefile                 |   1 +
 kernel/fork.c                   |   6 +-
 kernel/sched/core.c             |  36 +++
 kernel/thread_handoff.c         | 490 ++++++++++++++++++++++++++++++++
 33 files changed, 2167 insertions(+), 116 deletions(-)
Signature · 1 line
Jens Axboe
author maintainer
#1 lore raw

PATCH 1/15 kernel: add thread identity handoff

Add infrastructure for moving the user visible identity of a thread that is about to block in the kernel to another thread in the same process, which then returns to userspace on its behalf.

The blocked thread keeps its task_struct and kernel stack, only what userspace observes moves: tid, signal state, rseq and robust list, credentials, scheduling attributes, mempolicy, comm and the user register state. Thread group leadership follows the tid.

thread_handoff_allowed() vets the source and thread_handoff_compatible() the destination. thread_handoff_prepare() runs on the source right before it blocks, thread_handoff_finish() on the destination moves the identity over. Architectures provide the arch_thread_handoff_*() hooks and select ARCH_HAS_THREAD_HANDOFF.

---
 arch/Kconfig                   |   7 +
 include/linux/thread_handoff.h |  72 +++++
 init/Kconfig                   |  11 +
 kernel/Makefile                |   1 +
 kernel/sched/core.c            |  33 +++
 kernel/thread_handoff.c        | 484 +++++++++++++++++++++++++++++++++
 6 files changed, 608 insertions(+)
 create mode 100644 include/linux/thread_handoff.h
 create mode 100644 kernel/thread_handoff.c
arch/Kconfig +7 -0
index 45c657772362..5ce7e1713f59 100644
@@ -604,6 +604,13 @@ config ARCH_HAVE_EXTRA_ELF_NOTES
 config ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
 	bool
 
+config ARCH_HAS_THREAD_HANDOFF
+	bool
+	help
+	  Architecture provides the arch_thread_handoff_*() hooks for moving
+	  the user visible register state of a thread that is blocked in the
+	  kernel to another thread of the same process.
+
 config HAVE_ALIGNED_STRUCT_PAGE
 	bool
 	help
include/linux/thread_handoff.h new file +72 -0

72 lines not on this page · raw message

init/Kconfig +11 -0
index 8583d9f06c52..b376f802a77a 100644
@@ -1957,9 +1957,20 @@ config AIO
 	  by some high performance threaded applications. Disabling
 	  this option saves about 7k.
 
+config THREAD_HANDOFF
+	bool
+	depends on ARCH_HAS_THREAD_HANDOFF
+	help
+	  Support for handing the user visible identity of a thread that is
+	  about to block in the kernel to another thread of the same process,
+	  which then returns to userspace on its behalf. Used by io_uring to
+	  issue requests inline and only offload them to a worker thread if
+	  they actually block.
+
 config IO_URING
 	bool "Enable IO uring support" if EXPERT
 	select IO_WQ
+	select THREAD_HANDOFF if ARCH_HAS_THREAD_HANDOFF
 	default y
 	help
 	  This option enables support for the io_uring interface, enabling
kernel/Makefile +1 -0
index 1e1a31673577..5843d4866db7 100644
@@ -14,6 +14,7 @@ obj-y     = fork.o exec_domain.o exec_state.o panic.o \
 
 obj-$(CONFIG_MULTIUSER) += groups.o
 obj-$(CONFIG_VHOST_TASK) += vhost_task.o
+obj-$(CONFIG_THREAD_HANDOFF) += thread_handoff.o
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace internal ftrace files
kernel/sched/core.c +33 -0
index b998ef6b87af..eeb55367c0c6 100644
@@ -96,6 +96,7 @@
 
 #include "../workqueue_internal.h"
 #include "../../io_uring/io-wq.h"
+#include <linux/thread_handoff.h>
 #include "../smpboot.h"
 #include "../locking/mutex.h"
 
@@ -5684,6 +5685,38 @@ static inline void prefetch_curr_exec_start(struct task_struct *p)
  * In case the task is currently running, return the runtime plus current's
  * pending runtime that have not been accounted yet.
  */
+#ifdef CONFIG_THREAD_HANDOFF
+/* keep the prev_sum_exec_runtime delta intact, slice accounting uses it */
+u64 sched_exec_runtime_take(struct task_struct *p)
+{
+	struct rq_flags rf;
+	struct rq *rq;
+	u64 ns;
+
+	rq = task_rq_lock(p, &rf);
+	if (task_current_donor(rq, p) && task_on_rq_queued(p)) {
+		update_rq_clock(rq);
+		p->sched_class->update_curr(rq);
+	}
+	ns = p->se.sum_exec_runtime;
+	p->se.sum_exec_runtime = 0;
+	p->se.prev_sum_exec_runtime -= ns;
+	task_rq_unlock(rq, p, &rf);
+	return ns;
+}
+
+void sched_exec_runtime_add(struct task_struct *p, u64 ns)
+{
+	struct rq_flags rf;
+	struct rq *rq;
+
+	rq = task_rq_lock(p, &rf);
+	p->se.sum_exec_runtime += ns;
+	p->se.prev_sum_exec_runtime += ns;
+	task_rq_unlock(rq, p, &rf);
+}
+#endif
+
 unsigned long long task_sched_runtime(struct task_struct *p)
 {
 	struct rq_flags rf;
kernel/thread_handoff.c new file +484 -0

484 lines not on this page · raw message

Signature · 1 line
2.55.0
author maintainer
#2 lore raw

PATCH 2/15 sched: call into io_uring when a PF_IO_HANDOFF task blocks

In reply to Jens Axboe

Add PF_IO_HANDOFF, set by io_uring on a task for the duration of an inline request issue that may block, and have sched_submit_work() call io_uring_task_sleeping() when such a task blocks. Placeholder for now.

---
 include/linux/io_uring.h | 5 +++++
 include/linux/sched.h    | 2 +-
 kernel/fork.c            | 3 ++-
 kernel/sched/core.c      | 3 +++
 4 files changed, 11 insertions(+), 2 deletions(-)
include/linux/io_uring.h +5 -0
index d1aa4edfc2a5..969de22c3d0f 100644
@@ -60,4 +60,9 @@ static inline int io_uring_fork(struct task_struct *tsk)
 }
 #endif
 
+/* called from sched_submit_work() when a PF_IO_HANDOFF task blocks */
+static inline void io_uring_task_sleeping(struct task_struct *tsk)
+{
+}
+
 #endif
include/linux/sched.h +1 -1
index 8b3d47a325cc..310310865029 100644
@@ -1816,7 +1816,7 @@ extern struct pid *cad_pid;
 						 * I am cleaning dirty pages from some other bdi. */
 #define PF_KTHREAD		0x00200000	/* I am a kernel thread */
 #define PF_RANDOMIZE		0x00400000	/* Randomize virtual address space */
-#define PF__HOLE__00800000	0x00800000
+#define PF_IO_HANDOFF		0x00800000	/* io_uring: hand identity off if the task blocks */
 #define PF__HOLE__01000000	0x01000000
 #define PF__HOLE__02000000	0x02000000
 #define PF_NO_SETAFFINITY	0x04000000	/* Userland is not allowed to meddle with cpus_mask */
kernel/fork.c +2 -1
index 416758c8a3d4..510c8a9aa870 100644
@@ -2190,7 +2190,8 @@ __latent_entropy struct task_struct *copy_process(
 		goto bad_fork_cleanup_count;
 
 	delayacct_tsk_init(p);	/* Must remain after dup_task_struct() */
-	p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER | PF_IDLE | PF_NO_SETAFFINITY);
+	p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER | PF_IDLE |
+		      PF_NO_SETAFFINITY | PF_IO_HANDOFF);
 	p->flags |= PF_FORKNOEXEC;
 	INIT_LIST_HEAD(&p->children);
 	INIT_LIST_HEAD(&p->sibling);
kernel/sched/core.c +3 -0
index eeb55367c0c6..bba1c3b26b7e 100644
@@ -96,6 +96,7 @@
 
 #include "../workqueue_internal.h"
 #include "../../io_uring/io-wq.h"
+#include <linux/io_uring.h>
 #include <linux/thread_handoff.h>
 #include "../smpboot.h"
 #include "../locking/mutex.h"
@@ -7352,6 +7353,8 @@ static inline void sched_submit_work(struct task_struct *tsk)
 		wq_worker_sleeping(tsk);
 	else if (task_flags & PF_IO_WORKER)
 		io_wq_worker_sleeping(tsk);
+	else if (task_flags & PF_IO_HANDOFF)
+		io_uring_task_sleeping(tsk);
 
 	/*
 	 * spinlock and rwlock must not flush block requests.  This will
Signature · 1 line
2.55.0
author maintainer
#3 lore raw

PATCH 3/15 arm64: implement thread identity handoff

In reply to Jens Axboe

Implement the arch_thread_handoff_*() hooks and select ARCH_HAS_THREAD_HANDOFF.

The source is inside a syscall, so only the FPSIMD view of the vector registers needs to move. Tasks with SME streaming mode or ZA enabled, compat tasks, tasks with a guarded control stack and tasks trapping counter-timer access are refused.

---
 arch/arm64/Kconfig          |   1 +
 arch/arm64/kernel/process.c | 109 ++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)
arch/arm64/Kconfig +1 -0
index b5a51b0ef944..a919570c80cd 100644
@@ -46,6 +46,7 @@ config ARM64
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_HW_PTE_YOUNG
 	select ARCH_HAS_SETUP_DMA_OPS
+	select ARCH_HAS_THREAD_HANDOFF
 	select ARCH_HAS_SET_DIRECT_MAP
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_FORCE_DMA_UNENCRYPTED
arch/arm64/kernel/process.c +109 -0

118 lines not on this page · raw message

Signature · 1 line
2.55.0
author maintainer
#4 lore raw

PATCH 4/15 x86: implement thread identity handoff

In reply to Jens Axboe

Implement the arch_thread_handoff_*() hooks for 64-bit x86 and select ARCH_HAS_THREAD_HANDOFF.

Prepare saves FS/GS, PKRU and the FPU state. Finish copies the syscall pt_regs, fault info and FPU image over, and loads what __switch_to() would have. Refused are 32-bit tasks, I/O bitmaps and emulated iopl, per-thread speculation and CPUID/TSC controls, user shadow stacks and non-default sized fpstates.

ret_from_fork() now returns what the thread function returns in regs->ax rather than 0. A kernel thread returning from kernel_execve() returns 0 anyway, an io-wq worker that got handed a user identity returns the result of the syscall it took over.

---
 arch/x86/Kconfig             |   1 +
 arch/x86/kernel/process.c    |  10 +--
 arch/x86/kernel/process_64.c | 139 +++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 5 deletions(-)
arch/x86/Kconfig +1 -0
index 15fd9ec5ecac..4f53859d0228 100644
@@ -109,6 +109,7 @@ config X86
 	select ARCH_HAS_STRICT_MODULE_RWX
 	select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
 	select ARCH_HAS_SYSCALL_WRAPPER
+	select ARCH_HAS_THREAD_HANDOFF		if X86_64
 	select ARCH_HAS_UBSAN
 	select ARCH_HAS_DEBUG_WX
 	select ARCH_HAS_ZONE_DMA_SET if EXPERT
arch/x86/kernel/process.c +5 -5
index 346c438ac880..ed52af862392 100644
@@ -155,13 +155,13 @@ __visible void ret_from_fork(struct task_struct *prev, struct pt_regs *regs,
 
 	/* Is this a kernel thread? */
 	if (unlikely(fn)) {
-		fn(fn_arg);
+		long ret = fn(fn_arg);
+
 		/*
-		 * A kernel thread is allowed to return here after successfully
-		 * calling kernel_execve().  Exit to userspace to complete the
-		 * execve() syscall.
+		 * A kernel thread returning from kernel_execve(), or an io-wq
+		 * worker returning the result of a syscall it took over.
 		 */
-		regs->ax = 0;
+		regs->ax = ret;
 	}
 
 	syscall_exit_to_user_mode(regs);
arch/x86/kernel/process_64.c +139 -0

152 lines not on this page · raw message

Signature · 1 line
2.55.0
maintainer
#5 lore raw

Re: [PATCH 04/15] x86: implement thread identity handoff

arch/x86/kernel/process_64.c long do_arch_prctl_64(struct task_struct *task, int option, unsigned long arg2) · line 1003

Quoted 11 lines · Jens Axboe
arch/x86/kernel/process_64.c +11 -0
+/* state bound to the task that neither side may have */
+static bool thread_handoff_task_ok(struct task_struct *tsk)
+{
+	/* I/O permissions, the bitmap hangs off the task */
+	if (test_tsk_thread_flag(tsk, TIF_IO_BITMAP) || tsk->thread.iopl_emul)
+		return false;
+#ifdef CONFIG_X86_USER_SHADOW_STACK
+	/* the shadow stack is per-thread and would have to move along */
+	if (tsk->thread.features & ARCH_SHSTK_SHSTK)
+		return false;
+#endif

Distributions are having this enabled, this means nobody will pass this check.

Quoted 5 lines
arch/x86/kernel/process_64.c +5 -0
+	/* only the default sized FPU state gets copied over, no AMX */
+	if (x86_task_fpu(tsk)->fpstate->is_valloc)
+		return false;
+	return true;
+}
author maintainer
#6 lore raw

Re: [PATCH 04/15] x86: implement thread identity handoff

Quoted 16 lines · Peter Zijlstra

arch/x86/kernel/process_64.c long do_arch_prctl_64(struct task_struct *task, int option, unsigned long arg2) · line 1003

Quoted 11 lines · Jens Axboe
arch/x86/kernel/process_64.c +11 -0
+/* state bound to the task that neither side may have */
+static bool thread_handoff_task_ok(struct task_struct *tsk)
+{
+	/* I/O permissions, the bitmap hangs off the task */
+	if (test_tsk_thread_flag(tsk, TIF_IO_BITMAP) || tsk->thread.iopl_emul)
+		return false;
+#ifdef CONFIG_X86_USER_SHADOW_STACK
+	/* the shadow stack is per-thread and would have to move along */
+	if (tsk->thread.features & ARCH_SHSTK_SHSTK)
+		return false;
+#endif

Distributions are having this enabled, this means nobody will pass this check.

Ah good point, should be feasible just by swapping s->shstk, s->features, and s->features_locked. Traveling right now, but I'll test that for the next version.

Signature · 1 line
Jens Axboe
author maintainer
#7 lore raw

PATCH 5/15 io_uring/kbuf: use io_ring_submit_unlock() helper

In reply to Jens Axboe

io_buffers_select() open-codes how it unlocks the ring based on the issue_flags, use the generic helper instead.

---
 io_uring/kbuf.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
io_uring/kbuf.c +2 -3
index 7c309173dd19..8f04e6cc773b 100644
@@ -371,10 +371,9 @@ int io_buffers_select(struct io_kiocb *req, struct buf_sel_arg *arg,
 		ret = io_provided_buffers_select(req, &arg->out_len, sel->buf_list, arg->iovs);
 	}
 out_unlock:
-	if (issue_flags & IO_URING_F_UNLOCKED) {
+	if (issue_flags & IO_URING_F_UNLOCKED)
 		sel->buf_list = NULL;
-		mutex_unlock(&ctx->uring_lock);
-	}
+	io_ring_submit_unlock(ctx, issue_flags);
 	return ret;
 }
 
Signature · 1 line
2.55.0
author maintainer
#8 lore raw

PATCH 6/15 io_uring: keep the tctx nodes on a list

In reply to Jens Axboe

tctx->xa is indexed by the ring pointer, which makes it a deep and sparse xarray. Iterating it with xa_for_each() is expensive, 12 usec for a single entry in testing. Keep the nodes on a list as well and walk that instead, lookups by ring stay in the xarray.

---
 include/linux/io_uring_types.h |  2 ++
 io_uring/tctx.c                | 10 ++++++----
 io_uring/tctx.h                |  2 ++
 3 files changed, 10 insertions(+), 4 deletions(-)
include/linux/io_uring_types.h +2 -0
index 39629ee77b91..4af3d579ead6 100644
@@ -153,6 +153,8 @@ struct io_uring_task {
 	struct file			*registered_rings[IO_RINGFD_REG_MAX];
 
 	struct xarray			xa;
+	/* the nodes in ->xa, for walking without the xarray lookup cost */
+	struct list_head		node_list;
 	struct wait_queue_head		wait;
 	atomic_t			in_cancel;
 	atomic_t			inflight_tracked;
io_uring/tctx.c +6 -4
index 466b7300e208..737dfad4a976 100644
@@ -105,6 +105,7 @@ __cold struct io_uring_task *io_uring_alloc_task_context(struct task_struct *tas
 
 	tctx->task = task;
 	xa_init(&tctx->xa);
+	INIT_LIST_HEAD(&tctx->node_list);
 	init_waitqueue_head(&tctx->wait);
 	atomic_set(&tctx->in_cancel, 0);
 	atomic_set(&tctx->inflight_tracked, 0);
@@ -135,6 +136,7 @@ static int io_tctx_install_node(struct io_ring_ctx *ctx,
 		kfree(node);
 		return ret;
 	}
+	list_add(&node->tctx_link, &tctx->node_list);
 
 	mutex_lock(&ctx->tctx_lock);
 	list_add(&node->ctx_node, &ctx->tctx_list);
@@ -227,6 +229,7 @@ __cold void io_uring_del_tctx_node(unsigned long index)
 
 	WARN_ON_ONCE(current != node->task);
 	WARN_ON_ONCE(list_empty(&node->ctx_node));
+	list_del(&node->tctx_link);
 
 	mutex_lock(&node->ctx->tctx_lock);
 	list_del(&node->ctx_node);
@@ -243,11 +246,10 @@ __cold void io_uring_del_tctx_node(unsigned long index)
 __cold void io_uring_clean_tctx(struct io_uring_task *tctx)
 {
 	struct io_wq *wq = tctx->io_wq;
-	struct io_tctx_node *node;
-	unsigned long index;
+	struct io_tctx_node *node, *tmp;
 
-	xa_for_each(&tctx->xa, index, node) {
-		io_uring_del_tctx_node(index);
+	list_for_each_entry_safe(node, tmp, &tctx->node_list, tctx_link) {
+		io_uring_del_tctx_node((unsigned long)node->ctx);
 		cond_resched();
 	}
 	if (wq) {
io_uring/tctx.h +2 -0
index 76ad1ad4594e..9a139816ce4f 100644
@@ -2,6 +2,8 @@
 
 struct io_tctx_node {
 	struct list_head	ctx_node;
+	/* on tctx->node_list, only ever touched by the owning task */
+	struct list_head	tctx_link;
 	struct task_struct	*task;
 	struct io_ring_ctx	*ctx;
 };
Signature · 1 line
2.55.0
author maintainer
#9 lore raw

PATCH 7/15 io_uring: add uring_lock section depth tracking and blockable opdef flag

In reply to Jens Axboe

Prep patch for issuing requests inline in blocking mode and catching the sleep when it happens, rather than punting to io-wq upfront because an operation may block.

If a request blocks inline, uring_lock must be dropped on behalf of the sleeping task, which is only safe outside the sections that rely on the lock being held. Track those with a depth counter in io_ring_submit_lock() and io_ring_submit_unlock().

Add a "blockable" flag to io_issue_def for opcodes whose issue path can cope with blocking inline: read/write, the forced async fs ops, open, close and splice/tee. uring_cmd is excluded for now, drivers may bind state to the submitting task.

No functional changes in this patch.

---
 include/linux/io_uring_types.h |  5 +++++
 io_uring/io_uring.h            |  3 +++
 io_uring/opdef.c               | 29 +++++++++++++++++++++++++++++
 io_uring/opdef.h               |  2 ++
 4 files changed, 39 insertions(+)
include/linux/io_uring_types.h +5 -0
index 4af3d579ead6..50a4a0ad222f 100644
@@ -352,6 +352,11 @@ struct io_ring_ctx {
 	/* submission data */
 	struct {
 		struct mutex		uring_lock;
+		/*
+		 * io_ring_submit_lock() nesting depth, non-zero means the
+		 * issue path relies on the lock being held.
+		 */
+		unsigned int		submit_lock_depth;
 
 		/*
 		 * Ring buffer of indices into array of io_uring_sqe, which is
io_uring/io_uring.h +3 -0
index 896aab1ed026..870bb4dcc415 100644
@@ -393,6 +393,8 @@ static inline void io_ring_submit_unlock(struct io_ring_ctx *ctx,
 					 unsigned issue_flags)
 {
 	lockdep_assert_held(&ctx->uring_lock);
+	lockdep_assert(ctx->submit_lock_depth > 0);
+	ctx->submit_lock_depth--;
 	if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
 		mutex_unlock(&ctx->uring_lock);
 }
@@ -409,6 +411,7 @@ static inline void io_ring_submit_lock(struct io_ring_ctx *ctx,
 	if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
 		mutex_lock(&ctx->uring_lock);
 	lockdep_assert_held(&ctx->uring_lock);
+	ctx->submit_lock_depth++;
 }
 
 static inline void io_commit_cqring(struct io_ring_ctx *ctx)
io_uring/opdef.c +29 -0
index cf3aa2242cd7..fa07a2b94536 100644
@@ -69,6 +69,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.iopoll			= 1,
 		.vectored		= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_readv,
 		.issue			= io_read,
 	},
@@ -83,12 +84,14 @@ const struct io_issue_def io_issue_defs[] = {
 		.iopoll			= 1,
 		.vectored		= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_writev,
 		.issue			= io_write,
 	},
 	[IORING_OP_FSYNC] = {
 		.needs_file		= 1,
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_fsync_prep,
 		.issue			= io_fsync,
 	},
@@ -101,6 +104,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.ioprio			= 1,
 		.iopoll			= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_read_fixed,
 		.issue			= io_read_fixed,
 	},
@@ -114,6 +118,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.ioprio			= 1,
 		.iopoll			= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_write_fixed,
 		.issue			= io_write_fixed,
 	},
@@ -132,6 +137,7 @@ const struct io_issue_def io_issue_defs[] = {
 	[IORING_OP_SYNC_FILE_RANGE] = {
 		.needs_file		= 1,
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_sfr_prep,
 		.issue			= io_sync_file_range,
 	},
@@ -215,16 +221,19 @@ const struct io_issue_def io_issue_defs[] = {
 	[IORING_OP_FALLOCATE] = {
 		.needs_file		= 1,
 		.hash_reg_file          = 1,
+		.blockable		= 1,
 		.prep			= io_fallocate_prep,
 		.issue			= io_fallocate,
 	},
 	[IORING_OP_OPENAT] = {
 		.filter_pdu_size	= sizeof_field(struct io_uring_bpf_ctx, open),
+		.blockable		= 1,
 		.prep			= io_openat_prep,
 		.issue			= io_openat,
 		.filter_populate	= io_openat_bpf_populate,
 	},
 	[IORING_OP_CLOSE] = {
+		.blockable		= 1,
 		.prep			= io_close_prep,
 		.issue			= io_close,
 	},
@@ -236,6 +245,7 @@ const struct io_issue_def io_issue_defs[] = {
 	},
 	[IORING_OP_STATX] = {
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_statx_prep,
 		.issue			= io_statx,
 	},
@@ -249,6 +259,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.ioprio			= 1,
 		.iopoll			= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_read,
 		.issue			= io_read,
 	},
@@ -262,17 +273,20 @@ const struct io_issue_def io_issue_defs[] = {
 		.ioprio			= 1,
 		.iopoll			= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_write,
 		.issue			= io_write,
 	},
 	[IORING_OP_FADVISE] = {
 		.needs_file		= 1,
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_fadvise_prep,
 		.issue			= io_fadvise,
 	},
 	[IORING_OP_MADVISE] = {
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_madvise_prep,
 		.issue			= io_madvise,
 	},
@@ -308,6 +322,7 @@ const struct io_issue_def io_issue_defs[] = {
 	},
 	[IORING_OP_OPENAT2] = {
 		.filter_pdu_size	= sizeof_field(struct io_uring_bpf_ctx, open),
+		.blockable		= 1,
 		.prep			= io_openat2_prep,
 		.issue			= io_openat2,
 		.filter_populate	= io_openat_bpf_populate,
@@ -327,6 +342,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.hash_reg_file		= 1,
 		.unbound_nonreg_file	= 1,
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_splice_prep,
 		.issue			= io_splice,
 	},
@@ -347,6 +363,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.hash_reg_file		= 1,
 		.unbound_nonreg_file	= 1,
 		.audit_skip		= 1,
+		.blockable		= 1,
 		.prep			= io_tee_prep,
 		.issue			= io_tee,
 	},
@@ -360,22 +377,27 @@ const struct io_issue_def io_issue_defs[] = {
 #endif
 	},
 	[IORING_OP_RENAMEAT] = {
+		.blockable		= 1,
 		.prep			= io_renameat_prep,
 		.issue			= io_renameat,
 	},
 	[IORING_OP_UNLINKAT] = {
+		.blockable		= 1,
 		.prep			= io_unlinkat_prep,
 		.issue			= io_unlinkat,
 	},
 	[IORING_OP_MKDIRAT] = {
+		.blockable		= 1,
 		.prep			= io_mkdirat_prep,
 		.issue			= io_mkdirat,
 	},
 	[IORING_OP_SYMLINKAT] = {
+		.blockable		= 1,
 		.prep			= io_symlinkat_prep,
 		.issue			= io_symlinkat,
 	},
 	[IORING_OP_LINKAT] = {
+		.blockable		= 1,
 		.prep			= io_linkat_prep,
 		.issue			= io_linkat,
 	},
@@ -387,19 +409,23 @@ const struct io_issue_def io_issue_defs[] = {
 	},
 	[IORING_OP_FSETXATTR] = {
 		.needs_file = 1,
+		.blockable		= 1,
 		.prep			= io_fsetxattr_prep,
 		.issue			= io_fsetxattr,
 	},
 	[IORING_OP_SETXATTR] = {
+		.blockable		= 1,
 		.prep			= io_setxattr_prep,
 		.issue			= io_setxattr,
 	},
 	[IORING_OP_FGETXATTR] = {
 		.needs_file = 1,
+		.blockable		= 1,
 		.prep			= io_fgetxattr_prep,
 		.issue			= io_fgetxattr,
 	},
 	[IORING_OP_GETXATTR] = {
+		.blockable		= 1,
 		.prep			= io_getxattr_prep,
 		.issue			= io_getxattr,
 	},
@@ -497,6 +523,7 @@ const struct io_issue_def io_issue_defs[] = {
 	[IORING_OP_FTRUNCATE] = {
 		.needs_file		= 1,
 		.hash_reg_file		= 1,
+		.blockable		= 1,
 		.prep			= io_ftruncate_prep,
 		.issue			= io_ftruncate,
 	},
@@ -553,6 +580,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.iopoll			= 1,
 		.vectored		= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_readv_fixed,
 		.issue			= io_read,
 	},
@@ -567,6 +595,7 @@ const struct io_issue_def io_issue_defs[] = {
 		.iopoll			= 1,
 		.vectored		= 1,
 		.async_size		= sizeof(struct io_async_rw),
+		.blockable		= 1,
 		.prep			= io_prep_writev_fixed,
 		.issue			= io_write,
 	},
io_uring/opdef.h +2 -0
index 667f981e63b0..45c2f77cf782 100644
@@ -29,6 +29,8 @@ struct io_issue_def {
 	unsigned		vectored : 1;
 	/* set to 1 if this opcode uses 128b sqes in a mixed sq */
 	unsigned		is_128 : 1;
+	/* issue path is safe to run inline in blocking mode */
+	unsigned		blockable : 1;
 
 	/* size of async data needed, if any */
 	unsigned short		async_size;
Signature · 1 line
2.55.0
author maintainer
#10 lore raw

PATCH 8/15 io_uring: split io_uring_enter() and io_submit_sqes() into helpers

In reply to Jens Axboe

Move the tail of io_submit_sqes() into io_submit_sqes_end(), and split the IORING_ENTER_GETEVENTS handling out of io_uring_enter() into io_uring_enter_finish() and io_uring_getevents(). A later patch needs to resume io_uring_enter() from a different task than the one that started the syscall.

No functional changes in this patch.

---
 io_uring/io_uring.c | 179 ++++++++++++++++++++++++++++----------------
 1 file changed, 116 insertions(+), 63 deletions(-)
io_uring/io_uring.c +116 -63
index 61053421d809..b09221e239c0 100644
@@ -2010,12 +2010,31 @@ static bool io_get_sqe(struct io_ring_ctx *ctx, const struct io_uring_sqe **sqe)
 	return true;
 }
 
+static int io_submit_sqes_end(struct io_ring_ctx *ctx, unsigned int entries,
+			      unsigned int left)
+	__must_hold(&ctx->uring_lock)
+{
+	int ret = entries;
+
+	if (unlikely(left)) {
+		ret -= left;
+		/* try again if it submitted nothing and can't allocate a req */
+		if (!ret && io_req_cache_empty(ctx))
+			ret = -EAGAIN;
+		current->io_uring->cached_refs += left;
+	}
+
+	io_submit_state_end(ctx);
+	 /* Commit SQ ring head once we've consumed and submitted all SQEs */
+	io_commit_sqring(ctx);
+	return ret;
+}
+
 int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
 	__must_hold(&ctx->uring_lock)
 {
 	unsigned int entries;
 	unsigned int left;
-	int ret;
 
 	if (ctx->flags & IORING_SETUP_SQ_REWIND)
 		entries = ctx->sq_entries;
@@ -2026,7 +2045,7 @@ int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
 	if (unlikely(!entries))
 		return 0;
 
-	ret = left = entries;
+	left = entries;
 	io_get_task_refs(left);
 	io_submit_state_start(&ctx->submit_state, left);
 
@@ -2052,18 +2071,7 @@ int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
 		}
 	} while (--left);
 
-	if (unlikely(left)) {
-		ret -= left;
-		/* try again if it submitted nothing and can't allocate a req */
-		if (!ret && io_req_cache_empty(ctx))
-			ret = -EAGAIN;
-		current->io_uring->cached_refs += left;
-	}
-
-	io_submit_state_end(ctx);
-	 /* Commit SQ ring head once we've consumed and submitted all SQEs */
-	io_commit_sqring(ctx);
-	return ret;
+	return io_submit_sqes_end(ctx, entries, left);
 }
 
 static void io_rings_free(struct io_ring_ctx *ctx)
@@ -2583,6 +2591,94 @@ struct file *io_uring_ctx_get_file(unsigned int fd, bool registered)
 }
 
 
+static int io_iopoll_getevents(struct io_ring_ctx *ctx, u32 min_complete,
+			       u32 flags, const void __user *argp, size_t argsz)
+	__must_hold(&ctx->uring_lock)
+{
+	int ret;
+
+	ret = io_validate_ext_arg(ctx, flags, argp, argsz);
+	if (likely(!ret))
+		return io_iopoll_check(ctx, min_complete);
+	return ret;
+}
+
+static int io_wait_getevents(struct io_ring_ctx *ctx, u32 min_complete,
+			     u32 flags, const void __user *argp, size_t argsz)
+{
+	struct ext_arg ext_arg = { .argsz = argsz };
+	int ret;
+
+	ret = io_get_ext_arg(ctx, flags, argp, &ext_arg);
+	if (likely(!ret))
+		return io_cqring_wait(ctx, min_complete, flags, &ext_arg);
+	return ret;
+}
+
+static int io_getevents_ret(struct io_ring_ctx *ctx, int ret, int ret2)
+{
+	if (ret)
+		return ret;
+	/*
+	 * EBADR indicates that one or more CQE were dropped. Once the user has
+	 * been informed we can clear the bit as they are obviously ok with
+	 * those drops.
+	 */
+	if (unlikely(ret2 == -EBADR))
+		clear_bit(IO_CHECK_CQ_DROPPED_BIT, &ctx->check_cq);
+	return ret2;
+}
+
+static int io_uring_getevents(struct io_ring_ctx *ctx, int ret,
+			      u32 min_complete, u32 flags,
+			      const void __user *argp, size_t argsz)
+{
+	int ret2;
+
+	if (ctx->int_flags & IO_RING_F_SYSCALL_IOPOLL) {
+		/*
+		 * We disallow the app entering submit/complete with polling,
+		 * but we still need to lock the ring to prevent racing with
+		 * polled issue that got punted to a workqueue.
+		 */
+		mutex_lock(&ctx->uring_lock);
+		ret2 = io_iopoll_getevents(ctx, min_complete, flags, argp,
+					   argsz);
+		mutex_unlock(&ctx->uring_lock);
+	} else {
+		ret2 = io_wait_getevents(ctx, min_complete, flags, argp, argsz);
+	}
+	return io_getevents_ret(ctx, ret, ret2);
+}
+
+/* Finish an io_uring_enter() call that submitted and holds the uring_lock */
+static int io_uring_enter_finish(struct io_ring_ctx *ctx, int ret, u32 min_complete,
+			  u32 flags, const void __user *argp, size_t argsz)
+{
+	int ret2;
+
+	if (!(flags & IORING_ENTER_GETEVENTS)) {
+		mutex_unlock(&ctx->uring_lock);
+		return ret;
+	}
+
+	if (ctx->int_flags & IO_RING_F_SYSCALL_IOPOLL) {
+		ret2 = io_iopoll_getevents(ctx, min_complete, flags, argp,
+					   argsz);
+		mutex_unlock(&ctx->uring_lock);
+	} else {
+		/*
+		 * Ignore errors, we'll soon call io_cqring_wait() and it
+		 * should handle ownership problems if any.
+		 */
+		if (ctx->flags & IORING_SETUP_DEFER_TASKRUN)
+			(void)io_run_local_work_locked(ctx, min_complete);
+		mutex_unlock(&ctx->uring_lock);
+		ret2 = io_wait_getevents(ctx, min_complete, flags, argp, argsz);
+	}
+	return io_getevents_ret(ctx, ret, ret2);
+}
+
 SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 		u32, min_complete, u32, flags, const void __user *, argp,
 		size_t, argsz)
@@ -2639,57 +2735,14 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 			mutex_unlock(&ctx->uring_lock);
 			goto out;
 		}
-		if (flags & IORING_ENTER_GETEVENTS) {
-			if (ctx->int_flags & IO_RING_F_SYSCALL_IOPOLL)
-				goto iopoll_locked;
-			/*
-			 * Ignore errors, we'll soon call io_cqring_wait() and
-			 * it should handle ownership problems if any.
-			 */
-			if (ctx->flags & IORING_SETUP_DEFER_TASKRUN)
-				(void)io_run_local_work_locked(ctx, min_complete);
-		}
-		mutex_unlock(&ctx->uring_lock);
+		ret = io_uring_enter_finish(ctx, ret, min_complete, flags,
+					    argp, argsz);
+		goto out;
 	}
 
-	if (flags & IORING_ENTER_GETEVENTS) {
-		int ret2;
-
-		if (ctx->int_flags & IO_RING_F_SYSCALL_IOPOLL) {
-			/*
-			 * We disallow the app entering submit/complete with
-			 * polling, but we still need to lock the ring to
-			 * prevent racing with polled issue that got punted to
-			 * a workqueue.
-			 */
-			mutex_lock(&ctx->uring_lock);
-iopoll_locked:
-			ret2 = io_validate_ext_arg(ctx, flags, argp, argsz);
-			if (likely(!ret2))
-				ret2 = io_iopoll_check(ctx, min_complete);
-			mutex_unlock(&ctx->uring_lock);
-		} else {
-			struct ext_arg ext_arg = { .argsz = argsz };
-
-			ret2 = io_get_ext_arg(ctx, flags, argp, &ext_arg);
-			if (likely(!ret2))
-				ret2 = io_cqring_wait(ctx, min_complete, flags,
-						      &ext_arg);
-		}
-
-		if (!ret) {
-			ret = ret2;
-
-			/*
-			 * EBADR indicates that one or more CQE were dropped.
-			 * Once the user has been informed we can clear the bit
-			 * as they are obviously ok with those drops.
-			 */
-			if (unlikely(ret2 == -EBADR))
-				clear_bit(IO_CHECK_CQ_DROPPED_BIT,
-					  &ctx->check_cq);
-		}
-	}
+	if (flags & IORING_ENTER_GETEVENTS)
+		ret = io_uring_getevents(ctx, ret, min_complete, flags, argp,
+					 argsz);
 out:
 	if (!(flags & IORING_ENTER_REGISTERED_RING))
 		fput(file);
Signature · 1 line
2.55.0
author maintainer
#11 lore raw

PATCH 9/15 io_uring: keep the submission plug on the io_submit_sqes() stack

In reply to Jens Axboe

The submission plug is embedded in the ring's submit state, which only works while a single task runs the whole batch. With a submitter identity handoff another task finishes the batch, and the block layer caches current->plug across sleeps. Put the plug on the stack of io_submit_sqes() instead, it stays with the task that started it.

No functional changes in this patch.

---
 include/linux/io_uring_types.h |  3 ++-
 io_uring/io_uring.c            | 10 +++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)
include/linux/io_uring_types.h +2 -1
index 50a4a0ad222f..0b0d73688b8c 100644
@@ -298,7 +298,8 @@ struct io_submit_state {
 	bool			need_plug;
 	bool			cq_flush;
 	unsigned short		submit_nr;
-	struct blk_plug		plug;
+	/* the submitting task's plug, lives on its stack */
+	struct blk_plug		*plug;
 };
 
 struct io_alloc_cache {
io_uring/io_uring.c +7 -3
index b09221e239c0..100ade1eee3e 100644
@@ -1810,7 +1810,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
 		if (state->need_plug && def->plug) {
 			state->plug_started = true;
 			state->need_plug = false;
-			blk_start_plug_nr_ios(&state->plug, state->submit_nr);
+			blk_start_plug_nr_ios(state->plug, state->submit_nr);
 		}
 	}
 
@@ -1938,13 +1938,14 @@ static void io_submit_state_end(struct io_ring_ctx *ctx)
 	/* flush only after queuing links as they can generate completions */
 	io_submit_flush_completions(ctx);
 	if (state->plug_started)
-		blk_finish_plug(&state->plug);
+		blk_finish_plug(state->plug);
 }
 
 /*
  * Start submission side cache.
  */
 static void io_submit_state_start(struct io_submit_state *state,
+				  struct blk_plug *plug,
 				  unsigned int max_ios)
 {
 	state->plug_started = false;
@@ -1952,6 +1953,8 @@ static void io_submit_state_start(struct io_submit_state *state,
 	state->submit_nr = max_ios;
 	/* set only head, no need to init link_last in advance */
 	state->link.head = NULL;
+	/* on the submitter's stack, the block layer caches current->plug */
+	state->plug = plug;
 }
 
 static void io_commit_sqring(struct io_ring_ctx *ctx)
@@ -2035,6 +2038,7 @@ int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
 {
 	unsigned int entries;
 	unsigned int left;
+	struct blk_plug plug;
 
 	if (ctx->flags & IORING_SETUP_SQ_REWIND)
 		entries = ctx->sq_entries;
@@ -2047,7 +2051,7 @@ int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
 
 	left = entries;
 	io_get_task_refs(left);
-	io_submit_state_start(&ctx->submit_state, left);
+	io_submit_state_start(&ctx->submit_state, &plug, left);
 
 	do {
 		const struct io_uring_sqe *sqe;
Signature · 1 line
2.55.0
author maintainer
#12 lore raw

PATCH 10/15 io-wq: support handing a task identity to an idle worker

In reply to Jens Axboe

Add the io-wq side of handing a blocked submitter's identity to an idle worker. io_wq_handoff_claim() picks an idle worker that can take over the identity of the current task, and arranges for it to run a caller supplied function when it wakes instead of continuing as a worker.

Only a worker inside the idle sleep of the worker loop is claimable, being on the free list isn't enough as it may be sleeping elsewhere. This is tracked in acct->nr_iosleep. Redundant workers exit through the normal idle timeout.

---
 io_uring/io-wq.c | 263 ++++++++++++++++++++++++++++++++++++++++++++---
 io_uring/io-wq.h |  14 +++
 kernel/fork.c    |   3 +-
 3 files changed, 267 insertions(+), 13 deletions(-)
io_uring/io-wq.c +251 -12
index 2ca223e47d41..3d4eb4992d5b 100644
@@ -20,6 +20,7 @@
 #include <linux/sched/sysctl.h>
 #include <uapi/linux/io_uring.h>
 #include <linux/kcov.h>
+#include <linux/thread_handoff.h>
 
 #include "io-wq.h"
 #include "slist.h"
@@ -32,6 +33,15 @@ enum {
 	IO_WORKER_F_UP		= 0,	/* up and active */
 	IO_WORKER_F_RUNNING	= 1,	/* account as running */
 	IO_WORKER_F_FREE	= 2,	/* worker on free list */
+	IO_WORKER_F_IDLE_SLEEP	= 3,	/* in the idle sleep of the worker loop */
+};
+
+/* worker->handoff handshake */
+enum {
+	IO_WORKER_HANDOFF_NONE		= 0,
+	IO_WORKER_HANDOFF_PROMOTE,	/* claimed, set under ->workers_lock */
+	IO_WORKER_HANDOFF_DONE,		/* claimer took over the worker */
+	IO_WORKER_HANDOFF_FINISHED,	/* identity moved, demoted released */
 };
 
 enum {
@@ -64,6 +74,10 @@ struct io_worker {
 	struct callback_head create_work;
 	int init_retries;
 
+	atomic_t handoff;
+	io_wq_handoff_fn *handoff_fn;
+	struct task_struct *handoff_task;
+
 	union {
 		struct rcu_head rcu;
 		struct delayed_work work;
@@ -88,6 +102,9 @@ struct io_wq_acct {
 	unsigned max_workers;
 	atomic_t nr_running;
 
+	/* workers in the idle sleep, claimable for a handoff */
+	atomic_t nr_iosleep;
+
 	/**
 	 * The list of free workers.  Protected by #workers_lock
 	 * (write) and RCU (read).
@@ -151,6 +168,8 @@ static bool io_acct_cancel_pending_work(struct io_wq *wq,
 					struct io_wq_acct *acct,
 					struct io_cb_cancel_data *match);
 static void create_worker_cb(struct callback_head *cb);
+static void create_worker_cont(struct callback_head *cb);
+static bool io_task_work_match(struct callback_head *cb, void *data);
 static void io_wq_cancel_tw_create(struct io_wq *wq);
 
 static inline unsigned int __io_get_work_hash(unsigned int work_flags)
@@ -233,7 +252,7 @@ static bool io_task_worker_match(struct callback_head *cb, void *data)
 	return worker == data;
 }
 
-static void io_worker_exit(struct io_worker *worker)
+static void __noreturn io_worker_exit(struct io_worker *worker)
 {
 	struct io_wq *wq = worker->wq;
 	struct io_wq_acct *acct = io_wq_get_acct(worker);
@@ -411,7 +430,7 @@ static bool io_queue_worker_create(struct io_worker *worker,
 
 	atomic_inc(&wq->worker_refs);
 	init_task_work(&worker->create_work, func);
-	if (!task_work_add(wq->task, &worker->create_work, TWA_SIGNAL)) {
+	if (!io_wq_task_work_add(wq->task, &worker->create_work, TWA_SIGNAL)) {
 		/*
 		 * EXIT may have been set after checking it above, check after
 		 * adding the task_work and remove any creation item if it is
@@ -687,19 +706,48 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
 	} while (1);
 }
 
-static int io_wq_worker(void *data)
+/*
+ * Leave the idle sleep, check if we got claimed while in it. Serialized with
+ * the claimer by ->workers_lock, so a claim can't be missed or raced.
+ */
+static bool io_wq_worker_idle_done(struct io_wq_acct *acct,
+				   struct io_worker *worker)
 {
-	struct io_worker *worker = data;
-	struct io_wq_acct *acct = io_wq_get_acct(worker);
-	struct io_wq *wq = worker->wq;
-	bool exit_mask = false, last_timeout = false;
-	char buf[TASK_COMM_LEN] = {};
+	bool promoted;
 
-	set_mask_bits(&worker->flags, 0,
-		      BIT(IO_WORKER_F_UP) | BIT(IO_WORKER_F_RUNNING));
+	atomic_dec(&acct->nr_iosleep);
+	raw_spin_lock(&acct->workers_lock);
+	clear_bit(IO_WORKER_F_IDLE_SLEEP, &worker->flags);
+	/* the claimer may have committed already, DONE rather than PROMOTE */
+	promoted = atomic_read(&worker->handoff) != IO_WORKER_HANDOFF_NONE;
+	raw_spin_unlock(&acct->workers_lock);
 
-	snprintf(buf, sizeof(buf), "iou-wrk-%d", wq->task->pid);
-	set_task_comm(current, buf);
+	WARN_ON_ONCE(promoted && worker->handoff_task != current);
+	return promoted;
+}
+
+/* claimed, wait for the claimer to finish taking over the worker struct */
+static io_wq_handoff_fn *io_wq_worker_promoted(struct io_worker *worker)
+{
+	io_wq_handoff_fn *fn = worker->handoff_fn;
+
+	/* stop the scheduler from treating us as a worker while we wait */
+	current->flags &= ~(PF_IO_WORKER | PF_USER_WORKER);
+
+	wait_var_event(&worker->handoff,
+		atomic_read_acquire(&worker->handoff) == IO_WORKER_HANDOFF_DONE);
+
+	WARN_ON_ONCE(current->worker_private);
+	atomic_set(&worker->handoff, IO_WORKER_HANDOFF_NONE);
+	return fn;
+}
+
+/* the worker loop, only returns if claimed for a handoff */
+static io_wq_handoff_fn *io_wq_worker_run(struct io_worker *worker)
+{
+	struct io_wq_acct *acct = io_wq_get_acct(worker);
+	bool exit_mask = false, last_timeout = false;
+	struct io_wq *wq = worker->wq;
 
 	while (!test_bit(IO_WQ_BIT_EXIT, &wq->state)) {
 		long ret;
@@ -724,6 +772,11 @@ static int io_wq_worker(void *data)
 		if ((last_timeout && (exit_mask || acct->nr_workers > 1)) ||
 		    test_bit(IO_WQ_BIT_EXIT_ON_IDLE, &wq->state)) {
 			acct->nr_workers--;
+			/* a handoff can't claim an exiting worker */
+			if (test_bit(IO_WORKER_F_FREE, &worker->flags)) {
+				clear_bit(IO_WORKER_F_FREE, &worker->flags);
+				hlist_nulls_del_rcu(&worker->nulls_node);
+			}
 			raw_spin_unlock(&acct->workers_lock);
 			__set_current_state(TASK_RUNNING);
 			break;
@@ -733,7 +786,12 @@ static int io_wq_worker(void *data)
 		raw_spin_unlock(&acct->workers_lock);
 		if (io_run_task_work())
 			continue;
+		/* claimable only in this sleep, the free list isn't enough */
+		set_bit(IO_WORKER_F_IDLE_SLEEP, &worker->flags);
+		atomic_inc(&acct->nr_iosleep);
 		ret = schedule_timeout(WORKER_IDLE_TIMEOUT);
+		if (unlikely(io_wq_worker_idle_done(acct, worker)))
+			return io_wq_worker_promoted(worker);
 		if (signal_pending(current)) {
 			struct ksignal ksig;
 
@@ -752,9 +810,190 @@ static int io_wq_worker(void *data)
 		io_worker_handle_work(acct, worker);
 
 	io_worker_exit(worker);
+}
+
+static int io_wq_worker(void *data)
+{
+	struct io_worker *worker = data;
+	struct io_wq *wq = worker->wq;
+	io_wq_handoff_fn *fn;
+	char buf[TASK_COMM_LEN] = {};
+
+	set_mask_bits(&worker->flags, 0,
+		      BIT(IO_WORKER_F_UP) | BIT(IO_WORKER_F_RUNNING));
+
+	snprintf(buf, sizeof(buf), "iou-wrk-%d", wq->task->pid);
+	set_task_comm(current, buf);
+
+	/*
+	 * Only returns if we got handed an identity. -EIOCBQUEUED means we got
+	 * demoted again while running it, back to the worker loop.
+	 */
+	for (;;) {
+		long ret;
+
+		fn = io_wq_worker_run(worker);
+		ret = fn();
+		/* what we return is what userspace gets on some archs */
+		if (ret != -EIOCBQUEUED)
+			return ret;
+		worker = current->worker_private;
+	}
+}
+
+/* find and claim an idle sleeping worker, see io_wq_worker_idle_done() */
+static struct io_worker *io_wq_acct_handoff_claim(struct io_wq *wq,
+						  struct io_wq_acct *acct,
+						  io_wq_handoff_fn *fn)
+{
+	struct io_worker *worker, *found = NULL;
+	struct hlist_nulls_node *n;
+
+	raw_spin_lock(&acct->workers_lock);
+	if (test_bit(IO_WQ_BIT_EXIT, &wq->state))
+		goto out_unlock;
+	hlist_nulls_for_each_entry(worker, n, &acct->free_list, nulls_node) {
+		/* only claimable inside the idle sleep of the worker loop */
+		if (!test_bit(IO_WORKER_F_IDLE_SLEEP, &worker->flags))
+			continue;
+		if (!thread_handoff_compatible(current, worker->task))
+			continue;
+		clear_bit(IO_WORKER_F_FREE, &worker->flags);
+		hlist_nulls_del_init_rcu(&worker->nulls_node);
+		worker->handoff_fn = fn;
+		worker->handoff_task = worker->task;
+		atomic_set_release(&worker->handoff, IO_WORKER_HANDOFF_PROMOTE);
+		found = worker;
+		break;
+	}
+out_unlock:
+	raw_spin_unlock(&acct->workers_lock);
+	if (found)
+		wake_up_process(found->handoff_task);
+	return found;
+}
+
+/*
+ * task_work_add() that doesn't notify a task inside a blocking inline issue,
+ * it'd interrupt the sleep. io_handoff_end() picks pending work up instead.
+ */
+int io_wq_task_work_add(struct task_struct *task, struct callback_head *cb,
+			enum task_work_notify_mode notify)
+{
+	int ret;
+
+	if (notify != TWA_SIGNAL && notify != TWA_SIGNAL_NO_IPI)
+		return task_work_add(task, cb, notify);
+
+	ret = task_work_add(task, cb, TWA_NONE);
+	if (ret)
+		return ret;
+	if (READ_ONCE(task->flags) & PF_IO_HANDOFF)
+		return 0;
+	if (notify == TWA_SIGNAL)
+		set_notify_signal(task);
+	else
+		__set_notify_signal(task);
 	return 0;
 }
 
+/* claim an idle worker to hand our identity to, pairs with _commit() */
+struct task_struct *io_wq_handoff_claim(struct io_wq *wq, bool bound,
+					io_wq_handoff_fn *fn)
+{
+	struct io_worker *worker;
+
+	worker = io_wq_acct_handoff_claim(wq, io_get_acct(wq, bound), fn);
+	if (!worker)
+		worker = io_wq_acct_handoff_claim(wq, io_get_acct(wq, !bound), fn);
+	if (worker)
+		return worker->task;
+	return NULL;
+}
+
+/* we take over the worker @dst was, @dst goes on to run the handoff fn */
+void io_wq_handoff_commit(struct task_struct *dst)
+{
+	struct io_worker *worker = dst->worker_private;
+	struct task_struct *src = current;
+	struct io_wq *wq = worker->wq;
+	struct callback_head *cb;
+
+	WARN_ON_ONCE(src->worker_private);
+	WARN_ON_ONCE(worker->task != dst);
+	WARN_ON_ONCE(wq->task != src);
+
+	/* the sched hooks around @dst's wakeup cope with NULL worker_private */
+	WRITE_ONCE(dst->worker_private, NULL);
+	src->worker_private = worker;
+	WRITE_ONCE(worker->task, src);
+	src->flags |= PF_IO_WORKER | PF_USER_WORKER;
+
+	/* the user task owns the wq */
+	get_task_struct(dst);
+	WRITE_ONCE(wq->task, dst);
+
+	/* move pending worker creations along, we may block for a while */
+	while ((cb = task_work_cancel_match(src, io_task_work_match, wq))) {
+		struct io_worker *w = container_of(cb, struct io_worker,
+						  create_work);
+
+		if (!task_work_add(dst, cb, TWA_SIGNAL))
+			continue;
+		io_worker_cancel_cb(w);
+		if (cb->func == create_worker_cont)
+			kfree(w);
+	}
+	put_task_struct(src);
+
+	atomic_set_release(&worker->handoff, IO_WORKER_HANDOFF_DONE);
+	wake_up_var(&worker->handoff);
+}
+
+/* worker loop entry for a demoted task, only returns on another handoff */
+io_wq_handoff_fn *io_wq_handoff_worker(void)
+{
+	struct io_worker *worker = current->worker_private;
+	char buf[TASK_COMM_LEN] = {};
+
+	WARN_ON_ONCE(!io_wq_current_is_worker());
+
+	/* the promoted task reads our state until it's done migrating it */
+	wait_var_event(&worker->handoff,
+		atomic_read_acquire(&worker->handoff) == IO_WORKER_HANDOFF_FINISHED);
+	atomic_set(&worker->handoff, IO_WORKER_HANDOFF_NONE);
+
+	snprintf(buf, sizeof(buf), "iou-wrk-%d", worker->wq->task->pid);
+	set_task_comm(current, buf);
+	set_cpus_allowed_ptr(current, worker->wq->cpu_mask);
+
+	return io_wq_worker_run(worker);
+}
+
+/* release the demoted task @tsk to run as the worker it now is */
+void io_wq_handoff_finished(struct task_struct *tsk)
+{
+	struct io_worker *worker = tsk->worker_private;
+
+	atomic_set_release(&worker->handoff, IO_WORKER_HANDOFF_FINISHED);
+	wake_up_var(&worker->handoff);
+}
+
+/* idle sleepers to keep around as handoff targets */
+#define IO_WQ_HANDOFF_SPARES	2
+
+/* true if a worker is claimable, @topup forks one if below the target */
+bool io_wq_handoff_spare(struct io_wq *wq, bool bound, bool topup)
+{
+	struct io_wq_acct *acct = io_get_acct(wq, bound);
+	unsigned int idle;
+
+	idle = atomic_read(&acct->nr_iosleep);
+	if (topup && idle < IO_WQ_HANDOFF_SPARES)
+		io_wq_create_worker(wq, acct);
+	return idle > 0;
+}
+
 /*
  * Called when a worker is scheduled in. Mark us as currently running.
  */
io_uring/io-wq.h +14 -0
index 42f00a47a9c9..98357b665e54 100644
@@ -4,6 +4,7 @@
 
 #include <linux/refcount.h>
 #include <linux/io_uring_types.h>
+#include <linux/task_work.h>
 
 struct io_wq;
 
@@ -47,6 +48,19 @@ void io_wq_set_exit_on_idle(struct io_wq *wq, bool enable);
 void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work);
 void io_wq_hash_work(struct io_wq_work *work, void *val);
 
+typedef long (io_wq_handoff_fn)(void);
+
+/* claim an idle worker, it runs @fn instead of the worker loop when woken */
+struct task_struct *io_wq_handoff_claim(struct io_wq *wq, bool bound,
+					io_wq_handoff_fn *fn);
+
+void io_wq_handoff_commit(struct task_struct *dst);
+io_wq_handoff_fn *io_wq_handoff_worker(void);
+bool io_wq_handoff_spare(struct io_wq *wq, bool bound, bool topup);
+void io_wq_handoff_finished(struct task_struct *tsk);
+int io_wq_task_work_add(struct task_struct *task, struct callback_head *cb,
+			enum task_work_notify_mode notify);
+
 int io_wq_cpu_affinity(struct io_uring_task *tctx, cpumask_var_t mask);
 int io_wq_max_workers(struct io_wq *wq, int *new_count);
 bool io_wq_worker_stopped(void);
kernel/fork.c +2 -1
index 510c8a9aa870..f31af9c5bae7 100644
@@ -2688,11 +2688,12 @@ struct task_struct * __init fork_idle(int cpu)
  * creating io_uring workers. It returns a created task, or an error pointer.
  * The returned task is inactive, and the caller must fire it up through
  * wake_up_new_task(p). All signals are blocked in the created task.
+ * CLONE_SYSVSEM as a worker may take over a user thread's identity.
  */
 struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
 {
 	unsigned long flags = CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|
-			      CLONE_IO|CLONE_VM|CLONE_UNTRACED;
+			      CLONE_IO|CLONE_VM|CLONE_UNTRACED|CLONE_SYSVSEM;
 	struct kernel_clone_args args = {
 		.flags		= flags,
 		.fn		= fn,
Signature · 1 line
2.55.0
author maintainer
#13 lore raw

PATCH 11/15 io_uring: enable handing submitter identity to an io-wq worker

In reply to Jens Axboe

If a blockable request issued inline from io_uring_enter() blocks, hand the submitter's identity to an idle io-wq worker. The worker finishes the io_uring_enter() call and returns to userspace as the submitter, while the original task finishes the request and lives on as the worker.

Requests are still issued with IO_URING_F_NONBLOCK, but any sleep they hit anyway (page fault, lock, allocation stall) no longer blocks the submitter.

---
 include/linux/io_uring.h       |   4 +
 include/linux/io_uring_types.h |  34 ++++
 io_uring/Makefile              |   1 +
 io_uring/handoff.c             | 326 +++++++++++++++++++++++++++++++++
 io_uring/handoff.h             | 102 +++++++++++
 io_uring/io_uring.c            |  84 +++++++--
 io_uring/io_uring.h            |  36 +++-
 io_uring/msg_ring.c            |   2 +-
 io_uring/rw.c                  |   2 +-
 io_uring/tctx.c                |   6 +-
 io_uring/tw.c                  |  14 +-
 io_uring/uring_cmd.c           |   5 +-
 12 files changed, 596 insertions(+), 20 deletions(-)
 create mode 100644 io_uring/handoff.c
 create mode 100644 io_uring/handoff.h
include/linux/io_uring.h +4 -0
index 969de22c3d0f..4137276ba8f1 100644
@@ -61,8 +61,12 @@ static inline int io_uring_fork(struct task_struct *tsk)
 #endif
 
 /* called from sched_submit_work() when a PF_IO_HANDOFF task blocks */
+#if defined(CONFIG_IO_URING) && defined(CONFIG_THREAD_HANDOFF)
+void io_uring_task_sleeping(struct task_struct *tsk);
+#else
 static inline void io_uring_task_sleeping(struct task_struct *tsk)
 {
 }
+#endif
 
 #endif
include/linux/io_uring_types.h +34 -0
index 0b0d73688b8c..81bc4810fcab 100644
@@ -4,9 +4,11 @@
 #include <linux/blk_plug.h>
 #include <linux/hashtable.h>
 #include <linux/task_work.h>
+#include <linux/thread_handoff.h>
 #include <linux/bitmap.h>
 #include <linux/llist.h>
 #include <linux/uio.h>
+#include <linux/signal_types.h>
 #include <uapi/linux/io_uring.h>
 
 struct iou_loop_params;
@@ -139,12 +141,42 @@ struct io_br_sel {
  */
 #define IO_RINGFD_REG_MAX 16
 
+/* handoff state of a submitter that blocked inline, see io_uring/handoff.c */
+struct io_handoff {
+	/* the request being issued, while a handoff is possible */
+	struct io_kiocb			*req;
+	/* its ring and io-wq pool, @req is the demoted task's after that */
+	struct io_ring_ctx		*ctx;
+	bool				bound;
+	/* the submitter's signal mask while blocking issues run without */
+	sigset_t			sigmask;
+	bool				sigsaved;
+	/* the task the identity came from, and the task refs it held */
+	struct task_struct		*src;
+	unsigned int			src_refs;
+	struct thread_handoff_stats	stats;
+	/* io_uring_enter() arguments, to resume the syscall */
+	struct file			*file;
+	u32				to_submit;
+	/* SQEs consumed so far by this syscall, across handoffs */
+	u32				consumed;
+	u32				min_complete;
+	u32				flags;
+	const void __user		*argp;
+	size_t				argsz;
+};
+
 struct io_uring_task {
 	/* submission side */
 	int				cached_refs;
 	const struct io_ring_ctx 	*last;
 	struct task_struct		*task;
+	/* serializes ->task changes against off-task reference puts */
+	raw_spinlock_t			task_ref_lock;
 	struct io_wq			*io_wq;
+#ifdef CONFIG_THREAD_HANDOFF
+	struct io_handoff		handoff;
+#endif
 	/*
 	 * Consumer cursor for ->task_list. Only popped by the task itself,
 	 * or by ->fallback_work once the task can no longer run task_work.
@@ -298,6 +330,8 @@ struct io_submit_state {
 	bool			need_plug;
 	bool			cq_flush;
 	unsigned short		submit_nr;
+	/* cached SQ head at the start of the batch */
+	unsigned int		sq_head;
 	/* the submitting task's plug, lives on its stack */
 	struct blk_plug		*plug;
 };
io_uring/Makefile +1 -0
index c54e328d1410..9fd99118d26a 100644
@@ -18,6 +18,7 @@ obj-$(CONFIG_IO_URING)		+= io_uring.o opdef.o kbuf.o rsrc.o notif.o \
 
 obj-$(CONFIG_IO_URING_ZCRX)	+= zcrx.o
 obj-$(CONFIG_IO_WQ)		+= io-wq.o
+obj-$(CONFIG_THREAD_HANDOFF)	+= handoff.o
 obj-$(CONFIG_FUTEX)		+= futex.o
 obj-$(CONFIG_EPOLL)		+= epoll.o
 obj-$(CONFIG_NET_RX_BUSY_POLL)	+= napi.o
io_uring/handoff.c new file +326 -0

326 lines not on this page · raw message

io_uring/handoff.h new file +102 -0

102 lines not on this page · raw message

io_uring/io_uring.c +72 -12

204 lines not on this page · raw message

io_uring/io_uring.h +34 -2
index 870bb4dcc415..79db0a8b9cc8 100644
@@ -211,6 +211,10 @@ void io_free_req(struct io_kiocb *req);
 void io_queue_next(struct io_kiocb *req);
 void io_task_refs_refill(struct io_uring_task *tctx);
 bool __io_alloc_req_refill(struct io_ring_ctx *ctx);
+int io_req_sqe_copy(struct io_kiocb *req, unsigned int issue_flags);
+unsigned int io_submit_sqes_abandon(struct io_ring_ctx *ctx);
+int io_uring_enter_finish(struct io_ring_ctx *ctx, int ret, u32 min_complete,
+			  u32 flags, const void __user *argp, size_t argsz);
 
 void io_activate_pollwq(struct io_ring_ctx *ctx);
 void io_restriction_clone(struct io_restriction *dst, struct io_restriction *src);
@@ -389,13 +393,41 @@ static inline void io_put_file(struct io_kiocb *req)
 		fput(req->file);
 }
 
+/* a handed off issue keeps its issue_flags but no longer holds uring_lock */
+static inline bool io_issue_handed_off(unsigned int issue_flags)
+{
+	return !(issue_flags & IO_URING_F_UNLOCKED) &&
+		(current->flags & (PF_IO_HANDOFF | PF_IO_WORKER)) ==
+			(PF_IO_HANDOFF | PF_IO_WORKER);
+}
+
+/* which io-wq pool a request belongs in, bound unless a non-reg file op */
+static inline bool io_req_unbound(struct io_kiocb *req)
+{
+	if (!io_issue_defs[req->opcode].unbound_nonreg_file)
+		return false;
+	if (req->file) {
+		umode_t mode = file_inode(req->file)->i_mode;
+
+		if (S_ISREG(mode) || S_ISBLK(mode))
+			return false;
+	}
+	return true;
+}
+
+static inline bool io_issue_needs_lock(unsigned int issue_flags)
+{
+	return (issue_flags & IO_URING_F_UNLOCKED) ||
+		io_issue_handed_off(issue_flags);
+}
+
 static inline void io_ring_submit_unlock(struct io_ring_ctx *ctx,
 					 unsigned issue_flags)
 {
 	lockdep_assert_held(&ctx->uring_lock);
 	lockdep_assert(ctx->submit_lock_depth > 0);
 	ctx->submit_lock_depth--;
-	if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
+	if (unlikely(io_issue_needs_lock(issue_flags)))
 		mutex_unlock(&ctx->uring_lock);
 }
 
@@ -408,7 +440,7 @@ static inline void io_ring_submit_lock(struct io_ring_ctx *ctx,
 	 * The only exception is when we've detached the request and issue it
 	 * from an async worker thread, grab the lock for that case.
 	 */
-	if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
+	if (unlikely(io_issue_needs_lock(issue_flags)))
 		mutex_lock(&ctx->uring_lock);
 	lockdep_assert_held(&ctx->uring_lock);
 	ctx->submit_lock_depth++;
io_uring/msg_ring.c +1 -1
index 3067c9343991..04f2ffe4381d 100644
@@ -245,7 +245,7 @@ static int io_msg_fd_remote(struct io_kiocb *req)
 	struct task_struct *task = ctx->submitter_task;
 
 	init_task_work(&msg->tw, io_msg_tw_fd_complete);
-	if (task_work_add(task, &msg->tw, TWA_SIGNAL))
+	if (io_wq_task_work_add(task, &msg->tw, TWA_SIGNAL))
 		return -EOWNERDEAD;
 
 	return IOU_ISSUE_SKIP_COMPLETE;
io_uring/rw.c +1 -1
index 95106dd1d7eb..23b078fe2991 100644
@@ -134,7 +134,7 @@ static bool io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_async_rw *rw = req->async_data;
 
-	if (unlikely(issue_flags & IO_URING_F_UNLOCKED))
+	if (unlikely(io_issue_needs_lock(issue_flags)))
 		return false;
 
 	io_alloc_cache_vec_kasan(&rw->vec);
io_uring/tctx.c +5 -1
index 737dfad4a976..f86aca489d15 100644
@@ -10,6 +10,7 @@
 #include <uapi/linux/io_uring.h>
 
 #include "io_uring.h"
+#include "handoff.h"
 #include "tctx.h"
 #include "bpf_filter.h"
 
@@ -104,6 +105,7 @@ __cold struct io_uring_task *io_uring_alloc_task_context(struct task_struct *tas
 	}
 
 	tctx->task = task;
+	raw_spin_lock_init(&tctx->task_ref_lock);
 	xa_init(&tctx->xa);
 	INIT_LIST_HEAD(&tctx->node_list);
 	init_waitqueue_head(&tctx->wait);
@@ -175,8 +177,10 @@ int __io_uring_add_tctx_node(struct io_ring_ctx *ctx)
 	 * been marked for idle-exit when the task temporarily had no active
 	 * io_uring instances.
 	 */
-	if (tctx->io_wq)
+	if (tctx->io_wq) {
 		io_wq_set_exit_on_idle(tctx->io_wq, false);
+		io_handoff_prime(tctx, ctx);
+	}
 
 	if (new_tctx)
 		current->io_uring = tctx;
io_uring/tw.c +13 -1
index f573bcc3af6a..f26faeb5d21c 100644
@@ -9,6 +9,7 @@
 #include <linux/indirect_call_wrapper.h>
 
 #include "io_uring.h"
+#include "handoff.h"
 #include "tctx.h"
 #include "poll.h"
 #include "rw.h"
@@ -130,6 +131,10 @@ void tctx_task_work(struct callback_head *cb)
 	unsigned int count = 0;
 
 	tctx = container_of(cb, struct io_uring_task, task_work);
+	/* the tctx may have moved while queued, run it there if so */
+	if (unlikely(READ_ONCE(tctx->task) != current) &&
+	    !task_work_add(tctx->task, cb, TWA_SIGNAL))
+		return;
 	tctx_task_work_run(tctx, UINT_MAX, &count);
 }
 
@@ -209,6 +214,7 @@ void io_req_normal_work_add(struct io_kiocb *req)
 {
 	struct io_uring_task *tctx = req->tctx;
 	struct io_ring_ctx *ctx = req->ctx;
+	struct task_struct *task;
 
 	/* tw run already pending, nothing else to do */
 	if (!mpscq_push(&tctx->task_list, &req->io_task_work.node))
@@ -227,8 +233,14 @@ void io_req_normal_work_add(struct io_kiocb *req)
 		return;
 	}
 
-	if (likely(!task_work_add(tctx->task, &tctx->task_work, ctx->notify_method)))
+	task = READ_ONCE(tctx->task);
+	if (likely(!io_wq_task_work_add(task, &tctx->task_work,
+					 ctx->notify_method))) {
+		/* the tctx moved while we were adding, move the work along */
+		if (unlikely(READ_ONCE(tctx->task) != task))
+			io_handoff_tw_moved(tctx, task);
 		return;
+	}
 
 	io_fallback_tw(tctx);
 }
io_uring/uring_cmd.c +3 -2
index 726a659f38c3..ba36a555db98 100644
@@ -28,7 +28,7 @@ static void io_req_uring_cleanup(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
 	struct io_async_cmd *ac = req->async_data;
 
-	if (issue_flags & IO_URING_F_UNLOCKED)
+	if (io_issue_needs_lock(issue_flags))
 		return;
 
 	io_alloc_cache_vec_kasan(&ac->vec);
@@ -172,7 +172,8 @@ void __io_uring_cmd_done(struct io_uring_cmd *ioucmd, s32 ret, u64 res2,
 	if (req->flags & REQ_F_IOPOLL) {
 		/* order with io_do_iopoll() checking ->iopoll_completed */
 		smp_store_release(&req->iopoll_completed, 1);
-	} else if (issue_flags & IO_URING_F_COMPLETE_DEFER) {
+	} else if ((issue_flags & IO_URING_F_COMPLETE_DEFER) &&
+		   !io_issue_handed_off(issue_flags)) {
 		if (WARN_ON_ONCE(issue_flags & IO_URING_F_UNLOCKED))
 			return;
 		io_req_complete_defer(req);
Signature · 1 line
2.55.0
author maintainer
#14 lore raw

PATCH 12/15 io_uring: defer the identity migration to the end of the submission

In reply to Jens Axboe

A handoff currently migrates the full identity (tid, signals, cgroup, sched attributes, register state) before the promoted worker resumes the SQ ring. With N blockable SQEs in one io_uring_enter(), that puts a complete migration between each of them, where the old behaviour was N cheap io-wq punts.

None of that is needed to run kernel code on the submitter's behalf, only its creds and io_uring context are. Have the promoted task adopt the creds and continue the submission right away. If it blocks and hands off again, it just goes back to being a worker. Only the task that ends the submission migrates the identity, once, from the original submitter which is parked in io_wq_handoff_worker() until then.

A task demoted while running the handoff function now also goes through io_wq_handoff_worker() rather than straight into the worker loop, so it doesn't exit or rename itself while its state is still being read.

---
 include/linux/io_uring_types.h |  5 +--
 include/linux/thread_handoff.h |  4 +++
 io_uring/handoff.c             | 64 +++++++++++++++++++++++-----------
 io_uring/handoff.h             |  7 ++--
 io_uring/io-wq.c               | 33 +++++++++++-------
 io_uring/io-wq.h               |  3 +-
 kernel/thread_handoff.c        |  6 ++++
 7 files changed, 83 insertions(+), 39 deletions(-)
include/linux/io_uring_types.h +3 -2
index 81bc4810fcab..6c8fe7232aa2 100644
@@ -151,9 +151,10 @@ struct io_handoff {
 	/* the submitter's signal mask while blocking issues run without */
 	sigset_t			sigmask;
 	bool				sigsaved;
-	/* the task the identity came from, and the task refs it held */
+	/* identity source, and the task this hop took the worker from */
 	struct task_struct		*src;
-	unsigned int			src_refs;
+	struct task_struct		*prev;
+	unsigned int			prev_refs;
 	struct thread_handoff_stats	stats;
 	/* io_uring_enter() arguments, to resume the syscall */
 	struct file			*file;
include/linux/thread_handoff.h +4 -0
index e1c17b833e7c..203ab6ef73e0 100644
@@ -29,6 +29,7 @@ bool thread_handoff_compatible(struct task_struct *src,
 			       struct task_struct *dst);
 bool thread_handoff_prepare(struct task_struct *tsk);
 void thread_handoff_stats_take(struct thread_handoff_stats *st);
+void thread_handoff_adopt_creds(struct task_struct *src);
 int thread_handoff_finish(struct task_struct *src,
 			  struct thread_handoff_stats *st);
 
@@ -62,6 +63,9 @@ static inline bool thread_handoff_prepare(struct task_struct *tsk)
 static inline void thread_handoff_stats_take(struct thread_handoff_stats *st)
 {
 }
+static inline void thread_handoff_adopt_creds(struct task_struct *src)
+{
+}
 static inline int thread_handoff_finish(struct task_struct *src,
 					struct thread_handoff_stats *st)
 {
io_uring/handoff.c +43 -21

135 lines not on this page · raw message

io_uring/handoff.h +5 -2
index 833c6314d3b7..b8ded4916606 100644
@@ -44,12 +44,15 @@ static inline void io_handoff_enter(struct file *file, u32 to_submit,
 	ho->argsz = argsz;
 }
 
-/* a submit call is done issuing, restore the signal mask if we changed it */
+/*
+ * Done issuing, restore the signal mask if we changed it. Not with a handoff
+ * in flight, io_handoff_resume() does that once it has the identity.
+ */
 static inline void io_handoff_submit_end(void)
 {
 	struct io_handoff *ho = &current->io_uring->handoff;
 
-	if (unlikely(ho->sigsaved))
+	if (unlikely(ho->sigsaved) && !ho->src)
 		__io_handoff_restore_signals(ho);
 }
 
io_uring/io-wq.c +20 -13
index 3d4eb4992d5b..d29e5a80eddd 100644
@@ -829,22 +829,22 @@ static int io_wq_worker(void *data)
 	 * Only returns if we got handed an identity. -EIOCBQUEUED means we got
 	 * demoted again while running it, back to the worker loop.
 	 */
+	fn = io_wq_worker_run(worker);
 	for (;;) {
-		long ret;
+		long ret = fn();
 
-		fn = io_wq_worker_run(worker);
-		ret = fn();
 		/* what we return is what userspace gets on some archs */
 		if (ret != -EIOCBQUEUED)
 			return ret;
-		worker = current->worker_private;
+		fn = io_wq_handoff_worker();
 	}
 }
 
 /* find and claim an idle sleeping worker, see io_wq_worker_idle_done() */
 static struct io_worker *io_wq_acct_handoff_claim(struct io_wq *wq,
 						  struct io_wq_acct *acct,
-						  io_wq_handoff_fn *fn)
+						  io_wq_handoff_fn *fn,
+						  struct task_struct *src)
 {
 	struct io_worker *worker, *found = NULL;
 	struct hlist_nulls_node *n;
@@ -856,7 +856,7 @@ static struct io_worker *io_wq_acct_handoff_claim(struct io_wq *wq,
 		/* only claimable inside the idle sleep of the worker loop */
 		if (!test_bit(IO_WORKER_F_IDLE_SLEEP, &worker->flags))
 			continue;
-		if (!thread_handoff_compatible(current, worker->task))
+		if (!thread_handoff_compatible(src, worker->task))
 			continue;
 		clear_bit(IO_WORKER_F_FREE, &worker->flags);
 		hlist_nulls_del_init_rcu(&worker->nulls_node);
@@ -897,15 +897,17 @@ int io_wq_task_work_add(struct task_struct *task, struct callback_head *cb,
 	return 0;
 }
 
-/* claim an idle worker to hand our identity to, pairs with _commit() */
+/* claim an idle worker to hand @src's identity to, pairs with _commit() */
 struct task_struct *io_wq_handoff_claim(struct io_wq *wq, bool bound,
-					io_wq_handoff_fn *fn)
+					io_wq_handoff_fn *fn,
+					struct task_struct *src)
 {
 	struct io_worker *worker;
 
-	worker = io_wq_acct_handoff_claim(wq, io_get_acct(wq, bound), fn);
+	worker = io_wq_acct_handoff_claim(wq, io_get_acct(wq, bound), fn, src);
 	if (!worker)
-		worker = io_wq_acct_handoff_claim(wq, io_get_acct(wq, !bound), fn);
+		worker = io_wq_acct_handoff_claim(wq, io_get_acct(wq, !bound),
+						  fn, src);
 	if (worker)
 		return worker->task;
 	return NULL;
@@ -958,9 +960,14 @@ io_wq_handoff_fn *io_wq_handoff_worker(void)
 
 	WARN_ON_ONCE(!io_wq_current_is_worker());
 
-	/* the promoted task reads our state until it's done migrating it */
-	wait_var_event(&worker->handoff,
-		atomic_read_acquire(&worker->handoff) == IO_WORKER_HANDOFF_FINISHED);
+	/*
+	 * Wait until nobody needs our state anymore, which may be a while if
+	 * an identity is still parked on us. Hence TASK_IDLE.
+	 */
+	___wait_var_event(&worker->handoff,
+			  atomic_read_acquire(&worker->handoff) ==
+						IO_WORKER_HANDOFF_FINISHED,
+			  TASK_IDLE, 0, 0, schedule());
 	atomic_set(&worker->handoff, IO_WORKER_HANDOFF_NONE);
 
 	snprintf(buf, sizeof(buf), "iou-wrk-%d", worker->wq->task->pid);
io_uring/io-wq.h +2 -1
index 98357b665e54..df451838828d 100644
@@ -52,7 +52,8 @@ typedef long (io_wq_handoff_fn)(void);
 
 /* claim an idle worker, it runs @fn instead of the worker loop when woken */
 struct task_struct *io_wq_handoff_claim(struct io_wq *wq, bool bound,
-					io_wq_handoff_fn *fn);
+					io_wq_handoff_fn *fn,
+					struct task_struct *src);
 
 void io_wq_handoff_commit(struct task_struct *dst);
 io_wq_handoff_fn *io_wq_handoff_worker(void);
kernel/thread_handoff.c +6 -0
index 1901eb85bae8..822fdd9a0e7f 100644
@@ -393,6 +393,12 @@ static void thread_handoff_creds(struct task_struct *dst,
 	put_cred_many(old, 2);
 }
 
+/* the part of thread_handoff_finish() needed to run kernel code for @src */
+void thread_handoff_adopt_creds(struct task_struct *src)
+{
+	thread_handoff_creds(current, src);
+}
+
 /* the user requested affinity follows, the effective mask derives from it */
 static void thread_handoff_affinity(struct task_struct *dst,
 				    struct task_struct *src)
Signature · 1 line
2.55.0
author maintainer
#15 lore raw

PATCH 13/15 io_uring: issue blockable requests inline in blocking mode

In reply to Jens Axboe

With a handoff available, there's no point in issuing a blockable opcode nonblocking first. Clear IO_URING_F_NONBLOCK if io_handoff_begin() succeeds, and issue REQ_F_FORCE_ASYNC requests inline rather than punting them to io-wq upfront.

Requests with a working nonblocking issue path, like reads and writes on FMODE_NOWAIT files, behave as before. The handoff is for requests that otherwise would have required an io-wq punt upfront, most of which never block. SQEs marked IOSQE_ASYNC keep their explicit io-wq offload.

---
 include/linux/io_uring_types.h |  6 +++
 io_uring/handoff.c             | 75 ++++++++++++++++++++++------------
 io_uring/handoff.h             | 12 +++---
 io_uring/io_uring.c            | 48 +++++++++++++++++++---
 io_uring/io_uring.h            |  7 ++++
 io_uring/splice.c              |  6 +++
 6 files changed, 116 insertions(+), 38 deletions(-)
include/linux/io_uring_types.h +6 -0
index 6c8fe7232aa2..37c56ad37e05 100644
@@ -651,6 +651,8 @@ enum {
 	REQ_F_IMPORT_BUFFER_BIT,
 	REQ_F_SQE_COPIED_BIT,
 	REQ_F_IOPOLL_BIT,
+	REQ_F_ASYNC_USER_BIT,
+	REQ_F_HANDOFF_BIT,
 
 	/* not a real bit, just to check we're not overflowing the space */
 	__REQ_F_LAST_BIT,
@@ -746,6 +748,10 @@ enum {
 	REQ_F_SQE_COPIED	= IO_REQ_FLAG(REQ_F_SQE_COPIED_BIT),
 	/* request must be iopolled to completion (set in ->issue()) */
 	REQ_F_IOPOLL		= IO_REQ_FLAG(REQ_F_IOPOLL_BIT),
+	/* IOSQE_ASYNC was set on the SQE, not just by prep */
+	REQ_F_ASYNC_USER	= IO_REQ_FLAG(REQ_F_ASYNC_USER_BIT),
+	/* vetted at submit for an inline blocking issue with a handoff */
+	REQ_F_HANDOFF		= IO_REQ_FLAG(REQ_F_HANDOFF_BIT),
 };
 
 struct io_tw_req {
io_uring/handoff.c +49 -26

93 lines not on this page · raw message

io_uring/handoff.h +5 -7
index b8ded4916606..f315f2ae8d80 100644
@@ -6,16 +6,10 @@
 #include "opdef.h"
 #include "tw.h"
 
-/* a blocking issue got interrupted, retry on io-wq rather than restart */
-static inline bool io_issue_wants_restart(int ret)
-{
-	return ret == -ERESTARTSYS || ret == -ERESTARTNOINTR ||
-	       ret == -ERESTARTNOHAND || ret == -ERESTART_RESTARTBLOCK;
-}
-
 #ifdef CONFIG_THREAD_HANDOFF
 extern int sysctl_io_uring_handoff;
 
+bool io_handoff_possible(struct io_kiocb *req);
 bool __io_handoff_begin(struct io_kiocb *req);
 void io_handoff_prime(struct io_uring_task *tctx, struct io_ring_ctx *ctx);
 bool io_handoff_end(void);
@@ -77,6 +71,10 @@ static inline bool io_handoff_begin(struct io_kiocb *req,
 {
 	return false;
 }
+static inline bool io_handoff_possible(struct io_kiocb *req)
+{
+	return false;
+}
 static inline void io_handoff_submit_end(void)
 {
 }
io_uring/io_uring.c +43 -5

83 lines not on this page · raw message

io_uring/io_uring.h +7 -0
index 79db0a8b9cc8..1f536617b584 100644
@@ -421,6 +421,13 @@ static inline bool io_issue_needs_lock(unsigned int issue_flags)
 		io_issue_handed_off(issue_flags);
 }
 
+/* a blocking issue got interrupted, retry on io-wq rather than restart */
+static inline bool io_issue_wants_restart(int ret)
+{
+	return ret == -ERESTARTSYS || ret == -ERESTARTNOINTR ||
+	       ret == -ERESTARTNOHAND || ret == -ERESTART_RESTARTBLOCK;
+}
+
 static inline void io_ring_submit_unlock(struct io_ring_ctx *ctx,
 					 unsigned issue_flags)
 {
io_uring/splice.c +6 -0
index e81ebbb91925..7d464deb0972 100644
@@ -100,6 +100,9 @@ int io_tee(struct io_kiocb *req, unsigned int issue_flags)
 
 	if (!(sp->flags & SPLICE_F_FD_IN_FIXED))
 		fput(in);
+	/* interrupted before making progress, have the core retry it */
+	if (io_issue_wants_restart(ret))
+		return -EAGAIN;
 done:
 	if (ret != sp->len)
 		req_set_fail(req);
@@ -141,6 +144,9 @@ int io_splice(struct io_kiocb *req, unsigned int issue_flags)
 
 	if (!(sp->flags & SPLICE_F_FD_IN_FIXED))
 		fput(in);
+	/* interrupted before making progress, have the core retry it */
+	if (io_issue_wants_restart(ret))
+		return -EAGAIN;
 done:
 	if (ret != sp->len)
 		req_set_fail(req);
Signature · 1 line
2.55.0
author maintainer
#16 lore raw

PATCH 14/15 io_uring: add tracepoints for the handoff operation

In reply to Jens Axboe

Add tracepoints for a handoff, a handoff that didn't happen with the reason, and the promoted task resuming the submission.

---
 include/trace/events/io_uring.h | 114 ++++++++++++++++++++++++++++++++
 io_uring/handoff.c              |  42 +++++++++---
 2 files changed, 147 insertions(+), 9 deletions(-)
include/trace/events/io_uring.h +114 -0
index 34b31a855ea4..6043c5d46dbd 100644
@@ -671,6 +671,120 @@ TRACE_EVENT(io_uring_local_work_run,
 	TP_printk("ring %p, count %d, loops %u", __entry->ctx, __entry->count, __entry->loops)
 );
 
+/**
+ * io_uring_handoff - a blocked submitter hands its identity to a worker
+ *
+ * @req:	pointer to a submitted request
+ * @dst:	the idle io-wq worker task taking over
+ */
+TRACE_EVENT(io_uring_handoff,
+
+	TP_PROTO(struct io_kiocb *req, struct task_struct *dst),
+
+	TP_ARGS(req, dst),
+
+	TP_STRUCT__entry (
+		__field(  void *,	ctx		)
+		__field(  void *,	req		)
+		__field(  u64,		user_data	)
+		__field(  u8,		opcode		)
+		__field(  pid_t,	src_pid		)
+		__field(  pid_t,	dst_pid		)
+
+		__string( op_str, io_uring_get_opcode(req->opcode)	)
+	),
+
+	TP_fast_assign(
+		__entry->ctx		= req->ctx;
+		__entry->req		= req;
+		__entry->user_data	= req->cqe.user_data;
+		__entry->opcode		= req->opcode;
+		__entry->src_pid	= task_pid_nr(current);
+		__entry->dst_pid	= task_pid_nr(dst);
+
+		__assign_str(op_str);
+	),
+
+	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s, identity %d handed to worker %d",
+		__entry->ctx, __entry->req, __entry->user_data,
+		__get_str(op_str), __entry->src_pid, __entry->dst_pid)
+);
+
+/**
+ * io_uring_handoff_fail - a handoff didn't happen for a request
+ *
+ * @req:	pointer to the request being issued
+ * @reason:	why. "lock", "prepare" and "worker" mean the task blocked in
+ *		place, anything else that it took the io-wq punt path instead.
+ */
+TRACE_EVENT(io_uring_handoff_fail,
+
+	TP_PROTO(struct io_kiocb *req, const char *reason),
+
+	TP_ARGS(req, reason),
+
+	TP_STRUCT__entry (
+		__field(  void *,	ctx		)
+		__field(  void *,	req		)
+		__field(  u64,		user_data	)
+		__field(  u8,		opcode		)
+
+		__string( op_str, io_uring_get_opcode(req->opcode)	)
+		__string( reason, reason			)
+	),
+
+	TP_fast_assign(
+		__entry->ctx		= req->ctx;
+		__entry->req		= req;
+		__entry->user_data	= req->cqe.user_data;
+		__entry->opcode		= req->opcode;
+
+		__assign_str(op_str);
+		__assign_str(reason);
+	),
+
+	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s, %s",
+		__entry->ctx, __entry->req, __entry->user_data,
+		__get_str(op_str), __get_str(reason))
+);
+
+/**
+ * io_uring_handoff_resume - a promoted worker continues the submission
+ *
+ * @ctx:	pointer to a ring context structure
+ * @req:	the request that blocked, owned by the demoted task by now
+ * @worker:	pid the demoted task now runs under
+ * @consumed:	SQEs consumed by earlier handoffs of this syscall
+ * @to_submit:	SQE count the syscall asked for
+ */
+TRACE_EVENT(io_uring_handoff_resume,
+
+	TP_PROTO(void *ctx, void *req, pid_t worker, unsigned int consumed,
+		 unsigned int to_submit),
+
+	TP_ARGS(ctx, req, worker, consumed, to_submit),
+
+	TP_STRUCT__entry (
+		__field(  void *,	ctx		)
+		__field(  void *,	req		)
+		__field(  pid_t,	worker		)
+		__field(  unsigned int,	consumed	)
+		__field(  unsigned int,	to_submit	)
+	),
+
+	TP_fast_assign(
+		__entry->ctx		= ctx;
+		__entry->req		= req;
+		__entry->worker		= worker;
+		__entry->consumed	= consumed;
+		__entry->to_submit	= to_submit;
+	),
+
+	TP_printk("ring %p, request %p now on worker %d, consumed %u, to_submit %u",
+		__entry->ctx, __entry->req, __entry->worker,
+		__entry->consumed, __entry->to_submit)
+);
+
 #endif /* _TRACE_IO_URING_H */
 
 /* This part must be outside protection */
io_uring/handoff.c +33 -9

103 lines not on this page · raw message

Signature · 1 line
2.55.0
author maintainer
#17 lore raw

PATCH 15/15 io_uring: issue IOSQE_ASYNC requests inline when a handoff is possible

In reply to Jens Axboe

NOTE: undecided if this should be the behavior.

---
 include/linux/io_uring_types.h | 3 ---
 io_uring/io_uring.c            | 5 -----
 2 files changed, 8 deletions(-)
include/linux/io_uring_types.h +0 -3
index 37c56ad37e05..dadbe3655138 100644
@@ -651,7 +651,6 @@ enum {
 	REQ_F_IMPORT_BUFFER_BIT,
 	REQ_F_SQE_COPIED_BIT,
 	REQ_F_IOPOLL_BIT,
-	REQ_F_ASYNC_USER_BIT,
 	REQ_F_HANDOFF_BIT,
 
 	/* not a real bit, just to check we're not overflowing the space */
@@ -748,8 +747,6 @@ enum {
 	REQ_F_SQE_COPIED	= IO_REQ_FLAG(REQ_F_SQE_COPIED_BIT),
 	/* request must be iopolled to completion (set in ->issue()) */
 	REQ_F_IOPOLL		= IO_REQ_FLAG(REQ_F_IOPOLL_BIT),
-	/* IOSQE_ASYNC was set on the SQE, not just by prep */
-	REQ_F_ASYNC_USER	= IO_REQ_FLAG(REQ_F_ASYNC_USER_BIT),
 	/* vetted at submit for an inline blocking issue with a handoff */
 	REQ_F_HANDOFF		= IO_REQ_FLAG(REQ_F_HANDOFF_BIT),
 };
io_uring/io_uring.c +0 -5
index 7c2aa0cfacc8..04e2fbdcb3dd 100644
@@ -1771,8 +1771,6 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
 	/* same numerical values with corresponding REQ_F_*, safe to copy */
 	sqe_flags = READ_ONCE(sqe->flags);
 	req->flags = (__force io_req_flags_t) sqe_flags;
-	if (sqe_flags & IOSQE_ASYNC)
-		req->flags |= REQ_F_ASYNC_USER;
 	req->cqe.user_data = READ_ONCE(sqe->user_data);
 	req->file = NULL;
 	req->tctx = current->io_uring;
@@ -1919,9 +1917,6 @@ static bool io_req_force_async(struct io_kiocb *req)
 		return true;
 	if (!(req->flags & REQ_F_FORCE_ASYNC))
 		return false;
-	/* userspace asked for it, keep the explicit offload */
-	if (req->flags & REQ_F_ASYNC_USER)
-		return true;
 	if (req->ctx->int_flags & IO_RING_F_DRAIN_ACTIVE)
 		return true;
 	/* the file decides on pollability, resolve it now if fixed */
Signature · 1 line
2.55.0
#18 lore raw
In reply to Jens Axboe
Quoted 14 lines · Jens Axboe

Hi,

io_uring issues requests inline with IO_URING_F_NONBLOCK and punts to io-wq when that isn't possible. For a range of opcodes it isn't possible at all, as there's no nonblocking path in the kernel for them: fsync, statx, openat, the *at family, xattr, fadvise, splice, etc. Those are punted unconditionally, and the punt costs a thread wakeup, a context switch and a task_work completion round trip per request. io_uring HAS to be cautious to prevent accidental blocking in the kernel, even if the operations predominantly never block. Sad story. Examples of that are things like an fdatasync that doesn't block, statx that hits dcache, openat for O_TMPFILE, etc. All of those would've completed inline just fine, but io_uring just cannot rely on that.

Quoted 13 lines

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct. Folks that have been around a while may remember earlier attempts at this about 20 years ago.

This is both really cool and seems like very dangerous thing :) Count me amazed. I worry this impersonating method will become as tricky as the kthread impersonating model that you replaced with the user workers, though. I haven't looked at your patches yet, but I wonder how you handle other tasks that have a reference to your task_struct.

I was actually working something much simpler to improve this problem, which still require subsystems to cooperate, but largely reduces issue:

My idea was to reuse the non_block_count which already exists in task_struct preserved for every kernel config that has io_uring. We we scope the inline path with it. We then provide new mutex, semaphore callers that will check the flag and fail refusing to sleep, similar to a try_lock. The new callers are required because we want subsystems to opt-in the behavior, properly clean after themselves, and return EWOULDBLOCK. This is why we need to clean blocking paths in io_uring. sched throws a WARN_ON if we schedule out with the counter> 0, making it easy to find issues.

It has the downside of still requiring fixes to every path and we need to handle every new case that comes by, but it is much cleaner than plumbing a nonblock flag several layers down the stack across each subsystem or having subsystem-specific details in io_uring, which is what we have today. On the upper side, it is much less complex than your approach. It also allow us to just back off during memory allocations that would block, solving the memory allocations anywhere in the submission path, not only inside ->issue(), which we discussed recently on discord.

I'll give a try to this series and report back.

Quoted 260 lines

We catch the blocking through a scheduler hook. A task in a blocking inline issue carries PF_IO_HANDOFF, and sched_submit_work() calls into io_uring for it, next to the existing io-wq and workqueue hooks. This is where the identity is handed off, with the uring_lock still held by the blocking task and released for the promoted worker on its behalf.

Structure of the series:

1 kernel: the thread identity handoff itself, in
kernel/thread_handoff.c. Independent of io_uring.
2 sched: the PF_IO_HANDOFF hook.
3-4 arm64 and x86 support. The arch hooks sync live register state
before the source blocks and load it on the destination.
5-9 io_uring prep: helper cleanups, a tctx node list, tracking of
uring_lock sections in the issue path so the hook knows when the lock may be dropped, splitting io_uring_enter() so it can be resumed by another task, and keeping the block plug on the submitter's stack.
10 io-wq: claiming an idle worker for a handoff, and keeping a
couple of idle spares around as targets.
11-12 io_uring: the handoff itself, and deferring the identity
migration to the end of the submission so a batch of blocking SQEs costs one migration rather than one per SQE.
13 io_uring: issue blockable requests inline in blocking mode.
This is where behavior changes.
14 tracepoints.
15 treat IOSQE_ASYNC the same way. Separate as it's a userspace
visible policy change, and I'm not sure yet it should be done.

Some things are refused for handoff up front: traced tasks, per-task perf contexts, PI futexes, audit contexts, armed per-thread CPU timers, core scheduling cookies, vfork parents. Per-thread accounting moves with the handoff, so the counters userspace sees for a tid stay monotonic. Known gaps are LSM state kept in the task rather than the cred, and PR_SET_IO_FLUSHER. Neither moves, and not moving them can only ever restrict.

Reads and writes on files with FMODE_NOWAIT are excluded. They have a working nonblocking path and poll retry, and a handoff per op would be worse than that. The handoff covers requests that previously would always have been handed to io-wq upfront. uring_cmd is excluded too, as drivers like ublk bind state to the submitting task.

Some test results:

Measured in a virtme-ng guest, 8 vcpu, non-debug x86 config, same kernel with a sysctl toggle for turning the feature on and off. CPU is the usage of the whole process including io-wq workers, as a percentage of one CPU. Mean of two runs.

ops/s cpu
=====================================================
fsync, tmpfs, qd 1
baseline 28.2k 96%
handoff 221k 100%
change +681% +5%
fsync, tmpfs, qd 8

   baseline                                195k   141%
   handoff                                 526k   100%
   change                                 +170%   -29%
 fsync, tmpfs, qd 32
   baseline                                357k   219%
   handoff                                 611k   100%
   change                                  +71%   -54%

fsync, ext4 (flushes, always blocks), qd 1

   baseline                                9.6k    66%
   handoff                                 7.1k    91%
   change                                  -26%   +37%
 fsync, ext4 (flushes, always blocks), qd 8
   baseline                               29.8k   212%
   handoff                                14.7k   146%
   change                                  -51%   -31%
 fsync, ext4 (flushes, always blocks), qd 32
   baseline                               42.2k   270%
   handoff                                14.6k   144%
   change                                  -65%   -47%

statx, ext4, qd 1
baseline 23.6k 96%
handoff 121k 100%
change +414% +5%
statx, ext4, qd 8

   baseline                                129k   234%
   handoff                                 179k   100%
   change                                  +38%   -57%
 statx, ext4, qd 32
   baseline                                197k   276%
   handoff                                 140k   100%
   change                                  -29%   -64%

statx, tmpfs, qd 1
baseline 24.4k 96%
handoff 117k 100%
change +378% +4%
statx, tmpfs, qd 8

   baseline                                131k   232%
   handoff                                 180k   100%
   change                                  +37%   -57%
 statx, tmpfs, qd 32
   baseline                                219k   295%
   handoff                                 190k   100%
   change                                  -13%   -66%

fadvise DONTNEED, ext4, qd 1
baseline 26.1k 95%
handoff 150k 100%
change +478% +5%
fadvise DONTNEED, ext4, qd 8

   baseline                                127k   170%
   handoff                                 252k   100%
   change                                  +99%   -41%
 fadvise DONTNEED, ext4, qd 32
   baseline                                270k   234%
   handoff                                 273k   100%
   change                                   +1%   -57%

renameat, ext4, qd 1
baseline 7.4k 98%
handoff 13.4k 100%
change +81% +2%
renameat, ext4, qd 8

   baseline                               12.0k   544%
   handoff                                14.6k   100%
   change                                  +21%   -82%
 renameat, ext4, qd 32
   baseline                               11.2k   500%
   handoff                                14.3k   100%
   change                                  +28%   -80%

renameat, tmpfs, qd 1

   baseline                               15.0k    97%
   handoff                                48.5k    99%
   change                                 +223%    +2%

renameat, tmpfs, qd 8
baseline 65.6k 514%
handoff 55.4k 98%
change -15% -81%
renameat, tmpfs, qd 32
baseline 39.4k 462%
handoff 38.9k 98%
change -1% -79%

openat O_TMPFILE, ext4, qd 1
baseline 6.3k 100%
handoff 11.3k 100%
change +81% +1%
openat O_TMPFILE, ext4, qd 8
baseline 23.0k 258%
handoff 12.9k 99%
change -44% -62%
openat O_TMPFILE, ext4, qd 32
baseline 26.2k 248%
handoff 13.4k 99%
change -49% -60%

openat O_TMPFILE, tmpfs, qd 1

   baseline                               13.4k    95%
   handoff                                43.0k    98%
   change                                 +222%    +3%

openat O_TMPFILE, tmpfs, qd 8
baseline 57.8k 220%
handoff 51.7k 96%
change -11% -56%
openat O_TMPFILE, tmpfs, qd 32
baseline 69.3k 218%
handoff 56.8k 96%
change -18% -56%

splice to pipe, ext4, qd 1

   baseline                               19.4k    96%
   handoff                                21.1k    98%
   change                                   +8%    +2%
 splice to pipe, ext4, qd 8
   baseline                               48.2k   176%
   handoff                                48.4k   176%
   change                                   +0%    +0%
 splice to pipe, ext4, qd 32
   baseline                               53.3k   188%
   handoff                                48.1k   182%

change -10% -3%

As you can tell, normal QD=1 type issues see big wins. Conversely, for higher queue depth, there are losses. The losses are generally from one of two reasons:

1) The syscall part is fairly expensive, and previously we farmed all of this work across a bunch of io-wq workers, and the parallelization there helps performance. For QD=1 that obviously isn't the case. The more expensive the lower level kernel parts are, the lower the QD required to see a perf loss. openat is the obvious worse case for this.

2) The syscall part ALWAYS blocks. For this case, io-wq is going to be quicker, just punt the opcode upfront. fsync on ext4, as shown in the table above, is indeed that case. Each of those block.

I've got some ideas for how to mitigate the losses for higher queue depths, but a) I didn't think they were THAT interesting for an RFC, as low QD is generally what people do with these kinds of ops, and b) the main concept behind this handoff is really the interesting part right now.

Passes the full liburing test suite, on both x86-64 and arm64. Other archs don't support this yet.

This is obviously an RFC, in terms of what I'd love people to take a closer look at:

- The scheduler hook and the identity move itself, kernel/thread_handoff.c. Is the set of refused states complete enough, and is moving thread group leadership this way (the leader must stay first on ->thread_head, like de_thread() keeps it) acceptable / kosher.
- The x86 and arm64 register state handling. x86 refuses AMX users, and arm64 refuses SME.
- Whether anyone relies on a task's user identity staying on one task_struct in ways not covered above in the series.

Patches are against 7.3-rc2. Also available at:

git://git.kernel.dk/linux.git io_uring-thread-handoff.3

 arch/Kconfig                    |   7 +
 arch/arm64/Kconfig              |   1 +
 arch/arm64/kernel/process.c     | 109 +++++++
 arch/x86/Kconfig                |   1 +
 arch/x86/kernel/process.c       |  10 +-
 arch/x86/kernel/process_64.c    | 139 +++++++++
 include/linux/io_uring.h        |   9 +
 include/linux/io_uring_types.h  |  48 +++-
 include/linux/sched.h           |   2 +-
 include/linux/thread_handoff.h  |  76 +++++
 include/trace/events/io_uring.h | 114 ++++++++
 init/Kconfig                    |  11 +
 io_uring/Makefile               |   1 +
 io_uring/handoff.c              | 395 +++++++++++++++++++++++++
 io_uring/handoff.h              | 103 +++++++
 io_uring/io-wq.c                | 270 +++++++++++++++++-
 io_uring/io-wq.h                |  15 +
 io_uring/io_uring.c             | 310 ++++++++++++++------
 io_uring/io_uring.h             |  46 ++-
 io_uring/kbuf.c                 |   5 +-
 io_uring/msg_ring.c             |   2 +-
 io_uring/opdef.c                |  29 ++
 io_uring/opdef.h                |   2 +
 io_uring/rw.c                   |   2 +-
 io_uring/splice.c               |   6 +
 io_uring/tctx.c                 |  16 +-
 io_uring/tctx.h                 |   2 +
 io_uring/tw.c                   |  14 +-
 io_uring/uring_cmd.c            |   5 +-
 kernel/Makefile                 |   1 +
 kernel/fork.c                   |   6 +-
 kernel/sched/core.c             |  36 +++
 kernel/thread_handoff.c         | 490 ++++++++++++++++++++++++++++++++
 33 files changed, 2167 insertions(+), 116 deletions(-)
Signature · 1 line
Jens Axboe
Signature · 1 line
Gabriel Krisman Bertazi
author maintainer
#19 lore raw
Quoted 30 lines · Gabriel Krisman Bertazi
Quoted 14 lines · Jens Axboe

Hi,

io_uring issues requests inline with IO_URING_F_NONBLOCK and punts to io-wq when that isn't possible. For a range of opcodes it isn't possible at all, as there's no nonblocking path in the kernel for them: fsync, statx, openat, the *at family, xattr, fadvise, splice, etc. Those are punted unconditionally, and the punt costs a thread wakeup, a context switch and a task_work completion round trip per request. io_uring HAS to be cautious to prevent accidental blocking in the kernel, even if the operations predominantly never block. Sad story. Examples of that are things like an fdatasync that doesn't block, statx that hits dcache, openat for O_TMPFILE, etc. All of those would've completed inline just fine, but io_uring just cannot rely on that.

Quoted 13 lines

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct. Folks that have been around a while may remember earlier attempts at this about 20 years ago.

Quoted 1 line

This is both really cool and seems like very dangerous thing :) Count me

Oh yeah, it's definitely crazy and deeply an RFC.

Quoted 4 lines

amazed. I worry this impersonating method will become as tricky as the kthread impersonating model that you replaced with the user workers, though. I haven't looked at your patches yet, but I wonder how you handle other tasks that have a reference to your task_struct.

That one was different, because these are normal threads, not kthreads. They are created similarly to if you did pthread_create() in userspace, this is what io-wq workers are already. So it's mostly as safe as io-wq already is, by design, which is why the PF_IO_WORKER work happened and why kthreads haven't been used since back in the early 5.x days.

So I don't think there's too much to worry about on the security front, it's mostly a "this will confuse the application" kind of thing because something has been missed. And yes that is no good either, but it's not a security concern. That's VERY different from the kthread case, where if you missed some kind of personality, then congrats you're now running with fully elevated privileges.

Quoted 12 lines

I was actually working something much simpler to improve this problem, which still require subsystems to cooperate, but largely reduces issue:

My idea was to reuse the non_block_count which already exists in task_struct preserved for every kernel config that has io_uring. We we scope the inline path with it. We then provide new mutex, semaphore callers that will check the flag and fail refusing to sleep, similar to a try_lock. The new callers are required because we want subsystems to opt-in the behavior, properly clean after themselves, and return EWOULDBLOCK. This is why we need to clean blocking paths in io_uring. sched throws a WARN_ON if we schedule out with the counter> 0, making it easy to find issues.

I think that would be a tough sell, mostly because of how many locking primitives we have and how widely they are used, and how difficult (or impossible) it is to introduce error paths for code that previously had none. That alone would make it a non-starter for me. Let alone is that it'd be a continual whack-a-mole kind of work, it'll never be fully done.

Quoted 9 lines

It has the downside of still requiring fixes to every path and we need to handle every new case that comes by, but it is much cleaner than plumbing a nonblock flag several layers down the stack across each subsystem or having subsystem-specific details in io_uring, which is what we have today. On the upper side, it is much less complex than your approach. It also allow us to just back off during memory allocations that would block, solving the memory allocations anywhere in the submission path, not only inside ->issue(), which we discussed recently on discord.

I think you'll find it'll be a lot MORE complicated than my approach! Backing out error handling is going to be impossible in some cases, think file systems for example. How would those cases be handled?

Quoted 1 line

I'll give a try to this series and report back.

Thanks!

Signature · 1 line
Jens Axboe
#20 lore raw
Quoted 11 lines · Jens Axboe
Quoted 9 lines · Gabriel Krisman Bertazi

It has the downside of still requiring fixes to every path and we need to handle every new case that comes by, but it is much cleaner than plumbing a nonblock flag several layers down the stack across each subsystem or having subsystem-specific details in io_uring, which is what we have today. On the upper side, it is much less complex than your approach. It also allow us to just back off during memory allocations that would block, solving the memory allocations anywhere in the submission path, not only inside ->issue(), which we discussed recently on discord.

Quoted 3 lines

I think you'll find it'll be a lot MORE complicated than my approach! Backing out error handling is going to be impossible in some cases, think file systems for example. How would those cases be handled?

I understand there are many cases where it would be impossible, most, if not all of them, involving FS. But I naively imagine we could back-off those early, before we get to the critical session, without even trying the nonblock approach, similar to what we do now, and punt to the io-wq, which is not going away anyway. What I'd like to solve is drop the logic of other parts of the kernel that we need to keep in the io_uring layer, such as which network protocols will block and which won't.

It is a hard sell, yes. And it does gives off vibes of re-editing the PF_MEMALLOC_NORECLAIM controversy. Long term, though, it would be much better for the kernel that can reliably handle failure and back-off than keep going, including for predictable performance.

Signature · 1 line
Gabriel Krisman Bertazi
author maintainer
#21 lore raw
Quoted 17 lines · Gabriel Krisman Bertazi
Quoted 11 lines · Jens Axboe
Quoted 9 lines · Gabriel Krisman Bertazi

It has the downside of still requiring fixes to every path and we need to handle every new case that comes by, but it is much cleaner than plumbing a nonblock flag several layers down the stack across each subsystem or having subsystem-specific details in io_uring, which is what we have today. On the upper side, it is much less complex than your approach. It also allow us to just back off during memory allocations that would block, solving the memory allocations anywhere in the submission path, not only inside ->issue(), which we discussed recently on discord.

Quoted 3 lines

I think you'll find it'll be a lot MORE complicated than my approach! Backing out error handling is going to be impossible in some cases, think file systems for example. How would those cases be handled?

Quoted 7 lines

I understand there are many cases where it would be impossible, most, if not all of them, involving FS. But I naively imagine we could back-off those early, before we get to the critical session, without even trying the nonblock approach, similar to what we do now, and punt to the io-wq, which is not going away anyway. What I'd like to solve is drop the logic of other parts of the kernel that we need to keep in the io_uring layer, such as which network protocols will block and which won't.

The problem is that you don't always know until you're at the point of no return. If it was that simple, we would not be talking about these patches :-)

Signature · 1 line
Jens Axboe
maintainer
#22 lore raw
In reply to Jens Axboe
Quoted 4 lines · Jens Axboe

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can

Quoted 8 lines

move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct.

I'm still struggling my way through this thing.

So thread T1 is doing this blocking syscall. When it actually blocks, you hand-over the userspace identifying part to another thread T2, which will return to userspace as if it were T1.

Is this really a hand-over, or a swap? I would imagine we not have two threads with the same tid and all that.

Also, I'm a wee bit confused, why not swap out the kernel stack with a io-wq worker and have the original thread return to userspace. This seems like a better defined situation. In so far as anything here is well defined.

Swapping the kernel state seems like a simpler endeavour than swapping all that is or might be user visible.

Quoted 2 lines

Folks that have been around a while may remember earlier attempts at this about 20 years ago.

That was the whole threadlet thing, right? Very hazy memories of that. I'll have to go read that back, I'm sure I still have that in my inbox *somewhere*.

Surely we abandoned that approach for a reason. ISTR there being some significant ick there, much like the thing you're proposing now.

Not sure yet on how things compare. Will have to dig through that stuff again.

author maintainer
#23 lore raw
Quoted 15 lines · Peter Zijlstra
Quoted 4 lines · Jens Axboe

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can

Quoted 8 lines

move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct.

Quoted 1 line

I'm still struggling my way through this thing.

I don't blame you.

Quoted 3 lines

So thread T1 is doing this blocking syscall. When it actually blocks, you hand-over the userspace identifying part to another thread T2, which will return to userspace as if it were T1.

Correct.

Quoted 2 lines

Is this really a hand-over, or a swap? I would imagine we not have two threads with the same tid and all that.

It's a swap, yes we won't have two threads with the same tid.

Quoted 7 lines

Also, I'm a wee bit confused, why not swap out the kernel stack with a io-wq worker and have the original thread return to userspace. This seems like a better defined situation. In so far as anything here is well defined.

Swapping the kernel state seems like a simpler endeavour than swapping all that is or might be user visible.

I think we'd just be trading one set of problems for another. In terms of the kernel side, we have the following set of issues around swapping the kernel stack instead:

- 'current' itself, this is used throughout the waiting helpers. Things like wait_queue_entries that use private = current.

- Any kind of sleeping lock that also stashes away the value of 'current;.

- signal interrupible sleeps via TASK_KILLABLE

- Just like the userspace side, we have a bunch of kernel side state as well in the task_struct. Not a complete list, but:
- plug, for any kind of in-progress IO submission state
- journal_info
- reclaim_state
- io_uring
- PF_MEMALLOC and friends
- task_work
- lockdep
- preempt/irq stuff in thread_info

- Kernel stack itself in ways it's tied to the task_struct
- stack canary
- KASAN
- vma tracking
- thread.sp
- pt_regs

Probably not a complete list, but it's a start... I do agree that the kernel stack swap seems like the more immediate idea, but I think it's a harder problem.

Quoted 6 lines
Quoted 2 lines

Folks that have been around a while may remember earlier attempts at this about 20 years ago.

That was the whole threadlet thing, right? Very hazy memories of that. I'll have to go read that back, I'm sure I still have that in my inbox *somewhere*.

Right, and it's actually pretty related to what we're discussing above. The first one was Zach's fibrils which attempted the kernel stack swap, and then we had threadlets from Ingo which went the route that this patch set is also taking and dealt with userspace state swaps instead.

Quoted 2 lines

Surely we abandoned that approach for a reason. ISTR there being some significant ick there, much like the thing you're proposing now.

Well like you that state has long since been swapped out on my end, my recollection was disagreement on the API, not the swapping itself. But also extremely hazy there...

Quoted 2 lines

Not sure yet on how things compare. Will have to dig through that stuff again.

Let me know if you find something interesting there.

Signature · 1 line
Jens Axboe
#24 lore raw
In reply to Jens Axboe
Quoted 14 lines · Jens Axboe

Hi,

io_uring issues requests inline with IO_URING_F_NONBLOCK and punts to io-wq when that isn't possible. For a range of opcodes it isn't possible at all, as there's no nonblocking path in the kernel for them: fsync, statx, openat, the *at family, xattr, fadvise, splice, etc. Those are punted unconditionally, and the punt costs a thread wakeup, a context switch and a task_work completion round trip per request. io_uring HAS to be cautious to prevent accidental blocking in the kernel, even if the operations predominantly never block. Sad story. Examples of that are things like an fdatasync that doesn't block, statx that hits dcache, openat for O_TMPFILE, etc. All of those would've completed inline just fine, but io_uring just cannot rely on that.

Quoted 13 lines

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct. Folks that have been around a while may remember earlier attempts at this about 20 years ago.

I don't see anything immediately wrong, but I suspect I am just not looking hard enough.

In my time working with the kernel I have never seen anyone actually get this kind of thing correct.

The handoff that we do during exec has a bug with posix timers that I think is 23 years old that we just caught, and still hasn't been merged to Linus.

There was the old daemonize call that got it wrong so often I added kthreadd.

Maybe you want something like the old solaris doors, or vfork. Perform a synchronous task switch to this other thread, and call this function in the other thread. Then block waiting on the other thread until the other thread blocks, or the function you called finishes.

Is there a reason you didn't try and do it that way? Just a synchronous switch to and from a thread in your thread pool?

You aren't changing the mm so I really doubt changing the stack pointer and a registers will be that expensive.

Eric

author maintainer
#25 lore raw
Quoted 36 lines · Eric W. Biederman
Quoted 14 lines · Jens Axboe

Hi,

io_uring issues requests inline with IO_URING_F_NONBLOCK and punts to io-wq when that isn't possible. For a range of opcodes it isn't possible at all, as there's no nonblocking path in the kernel for them: fsync, statx, openat, the *at family, xattr, fadvise, splice, etc. Those are punted unconditionally, and the punt costs a thread wakeup, a context switch and a task_work completion round trip per request. io_uring HAS to be cautious to prevent accidental blocking in the kernel, even if the operations predominantly never block. Sad story. Examples of that are things like an fdatasync that doesn't block, statx that hits dcache, openat for O_TMPFILE, etc. All of those would've completed inline just fine, but io_uring just cannot rely on that.

Quoted 13 lines

This series issues those requests inline in blocking mode instead, and only pays for the offload if the request actually blocks. But by the time it blocks, the submitter is deep in the kernel with the request on its stack, so the work can't be moved to another thread. What we can move is the identity. If the submitting task blocks, an idle io-wq worker takes over its user visible identity (tid, signal state, credentials, scheduling attributes, cgroup, user register state), finishes the io_uring_enter() call and returns to userspace as the submitter. The original task finishes the request as an io-wq worker and joins the pool. Userspace is none the wiser, hopefully, the same tid came back from the syscall, it's just on a different task_struct. Folks that have been around a while may remember earlier attempts at this about 20 years ago.

I don't see anything immediately wrong, but I suspect I am just not looking hard enough.

In my time working with the kernel I have never seen anyone actually get this kind of thing correct.

Quoted 6 lines

The handoff that we do during exec has a bug with posix timers that I think is 23 years old that we just caught, and still hasn't been merged to Linus.

There was the old daemonize call that got it wrong so often I added kthreadd.

I agree entirely with you, which is why this is (deeply) and RFC and I mostly pulled it to (some notion of) completion so I could run some testing and see how it performs.

Quoted 10 lines

Maybe you want something like the old solaris doors, or vfork. Perform a synchronous task switch to this other thread, and call this function in the other thread. Then block waiting on the other thread until the other thread blocks, or the function you called finishes.

Is there a reason you didn't try and do it that way? Just a synchronous switch to and from a thread in your thread pool?

You aren't changing the mm so I really doubt changing the stack pointer and a registers will be that expensive.

And replies like this are also why I wanted to get it out, because I think it's a problem worth solving, and it's the best way to solicit ideas. I think there's some potential in your suggestion, let me try and dig at it a little bit and experiment...

I'll be back with more details.

Signature · 1 line
Jens Axboe
#26 lore raw
In reply to Jens Axboe
Quoted 2 lines · Jens Axboe

Hi,

...

Quoted 4 lines

This is obviously an RFC, in terms of what I'd love people to take a closer look at:

Quoted 8 lines

- The scheduler hook and the identity move itself, kernel/thread_handoff.c. Is the set of refused states complete enough, and is moving thread group leadership this way (the leader must stay first on ->thread_head, like de_thread() keeps it) acceptable / kosher.
- The x86 and arm64 register state handling. x86 refuses AMX users, and arm64 refuses SME.
- Whether anyone relies on a task's user identity staying on one task_struct in ways not covered above in the series.

This way may break ublk server given 'task_struct *' can be changed because the server issues blocking OP(fallocate, ...), then the current check in ublk_dispatch_req() can be hit.

Also there is risk in 'task_struct *' keyed bpf task storage.

task_work queuing could become tricky too.

Thanks,
Ming

author maintainer
#27 lore raw
Quoted 19 lines · Ming Lei
Quoted 2 lines · Jens Axboe

Hi,

...

Quoted 4 lines

This is obviously an RFC, in terms of what I'd love people to take a closer look at:

Quoted 8 lines

- The scheduler hook and the identity move itself, kernel/thread_handoff.c. Is the set of refused states complete enough, and is moving thread group leadership this way (the leader must stay first on ->thread_head, like de_thread() keeps it) acceptable / kosher.
- The x86 and arm64 register state handling. x86 refuses AMX users, and arm64 refuses SME.
- Whether anyone relies on a task's user identity staying on one task_struct in ways not covered above in the series.

Quoted 3 lines

This way may break ublk server given 'task_struct *' can be changed because the server issues blocking OP(fallocate, ...), then the current check in ublk_dispatch_req() can be hit.

Yep, which is why is isn't enabled there.

Quoted 3 lines

Also there is risk in 'task_struct *' keyed bpf task storage.

task_work queuing could become tricky too.

Like the above, the series does attempt to handle that.

Signature · 1 line
Jens Axboe
#28 lore raw
Quoted 20 lines · Jens Axboe
Quoted 19 lines · Ming Lei
Quoted 2 lines · Jens Axboe

Hi,

...

Quoted 12 lines

This is obviously an RFC, in terms of what I'd love people to take a closer look at:

- The scheduler hook and the identity move itself, kernel/thread_handoff.c. Is the set of refused states complete enough, and is moving thread group leadership this way (the leader must stay first on ->thread_head, like de_thread() keeps it) acceptable / kosher.
- The x86 and arm64 register state handling. x86 refuses AMX users, and arm64 refuses SME.
- Whether anyone relies on a task's user identity staying on one task_struct in ways not covered above in the series.

Quoted 5 lines
Quoted 3 lines

This way may break ublk server given 'task_struct *' can be changed because the server issues blocking OP(fallocate, ...), then the current check in ublk_dispatch_req() can be hit.

Yep, which is why is isn't enabled there.

If you mean .blockable isn't set for IORING_OP_URING_CMD, it can't avoid this issue, because io->task crosses IORING_OP_URING_CMD, and one io-uring(fallocate) changes ublk server's task_struct *. It can be solved in ublk driver by storing and checking tid.

The usage is similar with prctl(PR_IO_FLUSHER).

Quoted 3 lines

Known gaps are LSM state kept in the task rather than the cred, and PR_SET_IO_FLUSHER. Neither moves, and not moving them can only ever restrict.

A spare forked before PR_SET_IO_FLUSHER lacks the flags, so the user thread loses them after the handoff, for good, since later spares are forked from the new task.

Quoted 6 lines
Quoted 4 lines

Also there is risk in 'task_struct *' keyed bpf task storage.

task_work queuing could become tricky too.

Like the above, the series does attempt to handle that.

I meant kernel tw uses instead of io-uring's, one example:

With CLOSE + FSYNC in one io_uring_enter(), the CLOSE isn't blocked and its CQE is posted before the syscall returns, but ____fput stays queued on the old submitter until the FSYNC completes because of handoff, so the app still sees the file open.

Thanks,
Ming