[PATCH] io_uring: Add new functions to handle user fault scenarios

[PATCH] io_uring: Add new functions to handle user fault scenarios

[Zhiwei Jiang]

In the Firecracker VM scenario, sporadically encountered threads with
the UN state in the following call stack:
[<0>] io_wq_put_and_exit+0xa1/0x210
[<0>] io_uring_clean_tctx+0x8e/0xd0
[<0>] io_uring_cancel_generic+0x19f/0x370
[<0>] __io_uring_cancel+0x14/0x20
[<0>] do_exit+0x17f/0x510
[<0>] do_group_exit+0x35/0x90
[<0>] get_signal+0x963/0x970
[<0>] arch_do_signal_or_restart+0x39/0x120
[<0>] syscall_exit_to_user_mode+0x206/0x260
[<0>] do_syscall_64+0x8d/0x170
[<0>] entry_SYSCALL_64_after_hwframe+0x78/0x80
The cause is a large number of IOU kernel threads saturating the CPU
and not exiting. When the issue occurs, CPU usage 100% and can only
be resolved by rebooting. Each thread's appears as follows:
iou-wrk-44588  [kernel.kallsyms]  [k] ret_from_fork_asm
iou-wrk-44588  [kernel.kallsyms]  [k] ret_from_fork
iou-wrk-44588  [kernel.kallsyms]  [k] io_wq_worker
iou-wrk-44588  [kernel.kallsyms]  [k] io_worker_handle_work
iou-wrk-44588  [kernel.kallsyms]  [k] io_wq_submit_work
iou-wrk-44588  [kernel.kallsyms]  [k] io_issue_sqe
iou-wrk-44588  [kernel.kallsyms]  [k] io_write
iou-wrk-44588  [kernel.kallsyms]  [k] blkdev_write_iter
iou-wrk-44588  [kernel.kallsyms]  [k] iomap_file_buffered_write
iou-wrk-44588  [kernel.kallsyms]  [k] iomap_write_iter
iou-wrk-44588  [kernel.kallsyms]  [k] fault_in_iov_iter_readable
iou-wrk-44588  [kernel.kallsyms]  [k] fault_in_readable
iou-wrk-44588  [kernel.kallsyms]  [k] asm_exc_page_fault
iou-wrk-44588  [kernel.kallsyms]  [k] exc_page_fault
iou-wrk-44588  [kernel.kallsyms]  [k] do_user_addr_fault
iou-wrk-44588  [kernel.kallsyms]  [k] handle_mm_fault
iou-wrk-44588  [kernel.kallsyms]  [k] hugetlb_fault
iou-wrk-44588  [kernel.kallsyms]  [k] hugetlb_no_page
iou-wrk-44588  [kernel.kallsyms]  [k] hugetlb_handle_userfault
iou-wrk-44588  [kernel.kallsyms]  [k] handle_userfault
iou-wrk-44588  [kernel.kallsyms]  [k] schedule
iou-wrk-44588  [kernel.kallsyms]  [k] __schedule
iou-wrk-44588  [kernel.kallsyms]  [k] __raw_spin_unlock_irq
iou-wrk-44588  [kernel.kallsyms]  [k] io_wq_worker_sleeping

I tracked the address that triggered the fault and the related function
graph, as well as the wake-up side of the user fault, and discovered this
: In the IOU worker, when fault in a user space page, this space is
associated with a userfault but does not sleep. This is because during
scheduling, the judgment in the IOU worker context leads to early return.
Meanwhile, the listener on the userfaultfd user side never performs a COPY
to respond, causing the page table entry to remain empty. However, due to
the early return, it does not sleep and wait to be awakened as in a normal
user fault, thus continuously faulting at the same address,so CPU loop.

Therefore, I believe it is necessary to specifically handle user faults by
setting a new flag to allow schedule function to continue in such cases,
make sure the thread to sleep.Export the relevant functions and struct for
user fault.

Signed-off-by: Zhiwei Jiang <qq282012236@gmail.com>
---
 io_uring/io-wq.c | 57 +++++++++++++++---------------------------------
 io_uring/io-wq.h | 45 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
index 04a75d666195..8faad766d565 100644
--- a/io_uring/io-wq.c
+++ b/io_uring/io-wq.c
@@ -26,12 +26,6 @@
 #define WORKER_IDLE_TIMEOUT	(5 * HZ)
 #define WORKER_INIT_LIMIT	3
 
-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 */
-};
-
 enum {
 	IO_WQ_BIT_EXIT		= 0,	/* wq exiting */
 };
@@ -40,33 +34,6 @@ enum {
 	IO_ACCT_STALLED_BIT	= 0,	/* stalled on hash */
 };
 
-/*
- * One for each thread in a wq pool
- */
-struct io_worker {
-	refcount_t ref;
-	unsigned long flags;
-	struct hlist_nulls_node nulls_node;
-	struct list_head all_list;
-	struct task_struct *task;
-	struct io_wq *wq;
-	struct io_wq_acct *acct;
-
-	struct io_wq_work *cur_work;
-	raw_spinlock_t lock;
-
-	struct completion ref_done;
-
-	unsigned long create_state;
-	struct callback_head create_work;
-	int init_retries;
-
-	union {
-		struct rcu_head rcu;
-		struct delayed_work work;
-	};
-};
-
 #if BITS_PER_LONG == 64
 #define IO_WQ_HASH_ORDER	6
 #else
@@ -706,6 +673,16 @@ static int io_wq_worker(void *data)
 	return 0;
 }
 
+void set_userfault_flag_for_ioworker(struct io_worker *worker)
+{
+	set_bit(IO_WORKER_F_FAULT, &worker->flags);
+}
+
+void clear_userfault_flag_for_ioworker(struct io_worker *worker)
+{
+	clear_bit(IO_WORKER_F_FAULT, &worker->flags);
+}
+
 /*
  * Called when a worker is scheduled in. Mark us as currently running.
  */
@@ -715,12 +692,14 @@ void io_wq_worker_running(struct task_struct *tsk)
 
 	if (!worker)
 		return;
-	if (!test_bit(IO_WORKER_F_UP, &worker->flags))
-		return;
-	if (test_bit(IO_WORKER_F_RUNNING, &worker->flags))
-		return;
-	set_bit(IO_WORKER_F_RUNNING, &worker->flags);
-	io_wq_inc_running(worker);
+	if (!test_bit(IO_WORKER_F_FAULT, &worker->flags)) {
+		if (!test_bit(IO_WORKER_F_UP, &worker->flags))
+			return;
+		if (test_bit(IO_WORKER_F_RUNNING, &worker->flags))
+			return;
+		set_bit(IO_WORKER_F_RUNNING, &worker->flags);
+		io_wq_inc_running(worker);
+	}
 }
 
 /*
diff --git a/io_uring/io-wq.h b/io_uring/io-wq.h
index d4fb2940e435..9444912d038d 100644
--- a/io_uring/io-wq.h
+++ b/io_uring/io-wq.h
@@ -15,6 +15,13 @@ enum {
 	IO_WQ_HASH_SHIFT	= 24,	/* upper 8 bits are used for hash key */
 };
 
+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_FAULT	= 3,	/* used for userfault */
+};
+
 enum io_wq_cancel {
 	IO_WQ_CANCEL_OK,	/* cancelled before started */
 	IO_WQ_CANCEL_RUNNING,	/* found, running, and attempted cancelled */
@@ -24,6 +31,32 @@ enum io_wq_cancel {
 typedef struct io_wq_work *(free_work_fn)(struct io_wq_work *);
 typedef void (io_wq_work_fn)(struct io_wq_work *);
 
+/*
+ * One for each thread in a wq pool
+ */
+struct io_worker {
+	refcount_t ref;
+	unsigned long flags;
+	struct hlist_nulls_node nulls_node;
+	struct list_head all_list;
+	struct task_struct *task;
+	struct io_wq *wq;
+	struct io_wq_acct *acct;
+
+	struct io_wq_work *cur_work;
+	raw_spinlock_t lock;
+	struct completion ref_done;
+
+	unsigned long create_state;
+	struct callback_head create_work;
+	int init_retries;
+
+	union {
+		struct rcu_head rcu;
+		struct delayed_work work;
+	};
+};
+
 struct io_wq_hash {
 	refcount_t refs;
 	unsigned long map;
@@ -70,8 +103,10 @@ enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
 					void *data, bool cancel_all);
 
 #if defined(CONFIG_IO_WQ)
-extern void io_wq_worker_sleeping(struct task_struct *);
-extern void io_wq_worker_running(struct task_struct *);
+extern void io_wq_worker_sleeping(struct task_struct *tsk);
+extern void io_wq_worker_running(struct task_struct *tsk);
+extern void set_userfault_flag_for_ioworker(struct io_worker *worker);
+extern void clear_userfault_flag_for_ioworker(struct io_worker *worker);
 #else
 static inline void io_wq_worker_sleeping(struct task_struct *tsk)
 {
@@ -79,6 +114,12 @@ static inline void io_wq_worker_sleeping(struct task_struct *tsk)
 static inline void io_wq_worker_running(struct task_struct *tsk)
 {
 }
+static inline void set_userfault_flag_for_ioworker(struct io_worker *worker)
+{
+}
+static inline void clear_userfault_flag_for_ioworker(struct io_worker *worker)
+{
+}
 #endif
 
 static inline bool io_wq_current_is_worker(void)
-- 
2.34.1

Re: [PATCH] io_uring: Add new functions to handle user fault scenarios

[Pavel Begunkov]

On 4/22/25 04:01, Zhiwei Jiang wrote:
...
> I tracked the address that triggered the fault and the related function
> graph, as well as the wake-up side of the user fault, and discovered this
> : In the IOU worker, when fault in a user space page, this space is
> associated with a userfault but does not sleep. This is because during
> scheduling, the judgment in the IOU worker context leads to early return.
> Meanwhile, the listener on the userfaultfd user side never performs a COPY
> to respond, causing the page table entry to remain empty. However, due to
> the early return, it does not sleep and wait to be awakened as in a normal
> user fault, thus continuously faulting at the same address,so CPU loop.
> 
> Therefore, I believe it is necessary to specifically handle user faults by
> setting a new flag to allow schedule function to continue in such cases,
> make sure the thread to sleep.Export the relevant functions and struct for
> user fault.

That's an interesting scenario. Not looking deeper into it, I don't see
any callers to set_userfault_flag_for_ioworker(), and so there is no one
to set IO_WORKER_F_FAULT. Is there a second patch patch I lost?

-- 
Pavel Begunkov

Re: [PATCH] io_uring: Add new functions to handle user fault scenarios

[姜智伟]

On Tue, Apr 22, 2025 at 3:59 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 4/22/25 04:01, Zhiwei Jiang wrote:
> ...
> > I tracked the address that triggered the fault and the related function
> > graph, as well as the wake-up side of the user fault, and discovered this
> > : In the IOU worker, when fault in a user space page, this space is
> > associated with a userfault but does not sleep. This is because during
> > scheduling, the judgment in the IOU worker context leads to early return.
> > Meanwhile, the listener on the userfaultfd user side never performs a COPY
> > to respond, causing the page table entry to remain empty. However, due to
> > the early return, it does not sleep and wait to be awakened as in a normal
> > user fault, thus continuously faulting at the same address,so CPU loop.
> >
> > Therefore, I believe it is necessary to specifically handle user faults by
> > setting a new flag to allow schedule function to continue in such cases,
> > make sure the thread to sleep.Export the relevant functions and struct for
> > user fault.
>
> That's an interesting scenario. Not looking deeper into it, I don't see
> any callers to set_userfault_flag_for_ioworker(), and so there is no one
> to set IO_WORKER_F_FAULT. Is there a second patch patch I lost?
>
> --
> Pavel Begunkov
>
Sorry, the following changes haven't been submitted yet. I was planning
to submit them separately, thinking they belong to two different subsystems.
The other changes that haven't been submitted are as follows:
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index d80f94346199..74bead069e85 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -32,6 +32,7 @@
 #include <linux/swapops.h>
 #include <linux/miscdevice.h>
 #include <linux/uio.h>
+#include "../io_uring/io-wq.h"

 static int sysctl_unprivileged_userfaultfd __read_mostly;

@@ -369,7 +370,10 @@ vm_fault_t handle_userfault(struct vm_fault *vmf,
unsigned long reason)
        vm_fault_t ret = VM_FAULT_SIGBUS;
        bool must_wait;
        unsigned int blocking_state;
+       struct io_worker *worker = current->worker_private;

+       if (worker)
+               set_userfault_flag_for_ioworker(worker);
        /*
         * We don't do userfault handling for the final child pid update
         * and when coredumping (faults triggered by get_dump_page()).
@@ -506,6 +510,9 @@ vm_fault_t handle_userfault(struct vm_fault *vmf,
unsigned long reason)

        __set_current_state(TASK_RUNNING);

+       if (worker)
+               clear_userfault_flag_for_ioworker(worker);
+
        /*
         * Here we race with the list_del; list_add in
         * userfaultfd_ctx_read(), however because we don't ever run

Re: [PATCH] io_uring: Add new functions to handle user fault scenarios

[Pavel Begunkov]

On 4/22/25 09:22, 姜智伟 wrote:
> On Tue, Apr 22, 2025 at 3:59 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> On 4/22/25 04:01, Zhiwei Jiang wrote:
>> ...
>>> I tracked the address that triggered the fault and the related function
>>> graph, as well as the wake-up side of the user fault, and discovered this
>>> : In the IOU worker, when fault in a user space page, this space is
>>> associated with a userfault but does not sleep. This is because during
>>> scheduling, the judgment in the IOU worker context leads to early return.
>>> Meanwhile, the listener on the userfaultfd user side never performs a COPY
>>> to respond, causing the page table entry to remain empty. However, due to
>>> the early return, it does not sleep and wait to be awakened as in a normal
>>> user fault, thus continuously faulting at the same address,so CPU loop.
>>>
>>> Therefore, I believe it is necessary to specifically handle user faults by
>>> setting a new flag to allow schedule function to continue in such cases,
>>> make sure the thread to sleep.Export the relevant functions and struct for
>>> user fault.
>>
>> That's an interesting scenario. Not looking deeper into it, I don't see
>> any callers to set_userfault_flag_for_ioworker(), and so there is no one
>> to set IO_WORKER_F_FAULT. Is there a second patch patch I lost?
>>
>> --
>> Pavel Begunkov
>>
> Sorry, the following changes haven't been submitted yet. I was planning
> to submit them separately, thinking they belong to two different subsystems.
> The other changes that haven't been submitted are as follows:

They should always come together, there is no way to review it
otherwise. Maintainers will decide how to apply patches best
when it's time for that.

-- 
Pavel Begunkov