io_uring/cancel.c | 1 + io_uring/fdinfo.c | 2 +- io_uring/filetable.c | 3 ++- io_uring/io_uring.c | 58 +++++++++++++++++++++++++++----------------- io_uring/io_uring.h | 43 ++++++++++++++++++++++++++------ io_uring/kbuf.c | 6 ++--- io_uring/net.c | 1 + io_uring/notif.c | 5 ++-- io_uring/notif.h | 3 ++- io_uring/openclose.c | 1 + io_uring/poll.c | 2 +- io_uring/register.c | 1 + io_uring/rsrc.c | 10 +++++++- io_uring/rsrc.h | 3 ++- io_uring/rw.c | 3 ++- io_uring/splice.c | 1 + io_uring/waitid.c | 2 +- 17 files changed, 102 insertions(+), 43 deletions(-)
As far as I can tell, setting IORING_SETUP_SINGLE_ISSUER when creating an io_uring doesn't actually enable any additional optimizations (aside from being a requirement for IORING_SETUP_DEFER_TASKRUN). This series leverages IORING_SETUP_SINGLE_ISSUER's guarantee that only one task submits SQEs to skip taking the uring_lock mutex in the submission and task work paths. First, we need to close a hole in the IORING_SETUP_SINGLE_ISSUER checks where IORING_REGISTER_CLONE_BUFFERS only checks whether the thread is allowed to access one of the two io_urings. It assumes the uring_lock will prevent concurrent access to the other io_uring, but this will no longer be the case after the optimization to skip taking uring_lock. We also need to remove the unused filetable.h #include from io_uring.h to avoid an #include cycle. Caleb Sander Mateos (4): io_uring: don't include filetable.h in io_uring.h io_uring/rsrc: respect submitter_task in io_register_clone_buffers() io_uring: factor out uring_lock helpers io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER io_uring/cancel.c | 1 + io_uring/fdinfo.c | 2 +- io_uring/filetable.c | 3 ++- io_uring/io_uring.c | 58 +++++++++++++++++++++++++++----------------- io_uring/io_uring.h | 43 ++++++++++++++++++++++++++------ io_uring/kbuf.c | 6 ++--- io_uring/net.c | 1 + io_uring/notif.c | 5 ++-- io_uring/notif.h | 3 ++- io_uring/openclose.c | 1 + io_uring/poll.c | 2 +- io_uring/register.c | 1 + io_uring/rsrc.c | 10 +++++++- io_uring/rsrc.h | 3 ++- io_uring/rw.c | 3 ++- io_uring/splice.c | 1 + io_uring/waitid.c | 2 +- 17 files changed, 102 insertions(+), 43 deletions(-) -- 2.45.2
syzbot ci has tested the following series [v1] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER https://lore.kernel.org/all/20250903032656.2012337-1-csander@purestorage.com * [PATCH 1/4] io_uring: don't include filetable.h in io_uring.h * [PATCH 2/4] io_uring/rsrc: respect submitter_task in io_register_clone_buffers() * [PATCH 3/4] io_uring: factor out uring_lock helpers * [PATCH 4/4] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER and found the following issue: WARNING in io_handle_tw_list Full report is available here: https://ci.syzbot.org/series/54ae0eae-5e47-4cfe-9ae7-9eaaf959b5ae *** WARNING in io_handle_tw_list tree: linux-next URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next base: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 arch: amd64 compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8 config: https://ci.syzbot.org/builds/1de646dd-4ee2-418d-9c62-617d88ed4fd2/config syz repro: https://ci.syzbot.org/findings/e229a878-375f-4286-89fe-b6724c23addd/syz_repro ------------[ cut here ]------------ WARNING: io_uring/io_uring.h:127 at io_ring_ctx_lock io_uring/io_uring.h:127 [inline], CPU#1: iou-sqp-6294/6297 WARNING: io_uring/io_uring.h:127 at io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155, CPU#1: iou-sqp-6294/6297 Modules linked in: CPU: 1 UID: 0 PID: 6297 Comm: iou-sqp-6294 Not tainted syzkaller #0 PREEMPT(full) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 RIP: 0010:io_ring_ctx_lock io_uring/io_uring.h:127 [inline] RIP: 0010:io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155 Code: 00 00 48 c7 c7 e0 90 02 8c be 8e 04 00 00 31 d2 e8 01 e5 d2 fc 2e 2e 2e 31 c0 45 31 e4 4d 85 ff 75 89 eb 7c e8 ad fb 00 fd 90 <0f> 0b 90 e9 cf fe ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 22 ff RSP: 0018:ffffc900032cf938 EFLAGS: 00010293 RAX: ffffffff84bfcba3 RBX: dffffc0000000000 RCX: ffff888107f61cc0 RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000000000000 RBP: ffff8881119a8008 R08: ffff888110bb69c7 R09: 1ffff11022176d38 R10: dffffc0000000000 R11: ffffed1022176d39 R12: ffff8881119a8000 R13: ffff888108441e90 R14: ffff888107f61cc0 R15: 0000000000000000 FS: 00007f81f25716c0(0000) GS:ffff8881a39f5000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000001b31b63fff CR3: 000000010f24c000 CR4: 00000000000006f0 Call Trace: <TASK> tctx_task_work_run+0x99/0x370 io_uring/io_uring.c:1223 io_sq_tw io_uring/sqpoll.c:244 [inline] io_sq_thread+0xed1/0x1e50 io_uring/sqpoll.c:327 ret_from_fork+0x47f/0x820 arch/x86/kernel/process.c:148 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 </TASK> *** If these findings have caused you to resend the series or submit a separate fix, please add the following tag to your commit message: Tested-by: syzbot@syzkaller.appspotmail.com --- This report is generated by a bot. It may contain errors. syzbot ci engineers can be reached at syzkaller@googlegroups.com.
On 9/3/25 3:55 PM, syzbot ci wrote: > syzbot ci has tested the following series > > [v1] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > https://lore.kernel.org/all/20250903032656.2012337-1-csander@purestorage.com > * [PATCH 1/4] io_uring: don't include filetable.h in io_uring.h > * [PATCH 2/4] io_uring/rsrc: respect submitter_task in io_register_clone_buffers() > * [PATCH 3/4] io_uring: factor out uring_lock helpers > * [PATCH 4/4] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > and found the following issue: > WARNING in io_handle_tw_list > > Full report is available here: > https://ci.syzbot.org/series/54ae0eae-5e47-4cfe-9ae7-9eaaf959b5ae > > *** > > WARNING in io_handle_tw_list > > tree: linux-next > URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next > base: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 > arch: amd64 > compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8 > config: https://ci.syzbot.org/builds/1de646dd-4ee2-418d-9c62-617d88ed4fd2/config > syz repro: https://ci.syzbot.org/findings/e229a878-375f-4286-89fe-b6724c23addd/syz_repro > > ------------[ cut here ]------------ > WARNING: io_uring/io_uring.h:127 at io_ring_ctx_lock io_uring/io_uring.h:127 [inline], CPU#1: iou-sqp-6294/6297 > WARNING: io_uring/io_uring.h:127 at io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155, CPU#1: iou-sqp-6294/6297 > Modules linked in: > CPU: 1 UID: 0 PID: 6297 Comm: iou-sqp-6294 Not tainted syzkaller #0 PREEMPT(full) > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > RIP: 0010:io_ring_ctx_lock io_uring/io_uring.h:127 [inline] > RIP: 0010:io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155 > Code: 00 00 48 c7 c7 e0 90 02 8c be 8e 04 00 00 31 d2 e8 01 e5 d2 fc 2e 2e 2e 31 c0 45 31 e4 4d 85 ff 75 89 eb 7c e8 ad fb 00 fd 90 <0f> 0b 90 e9 cf fe ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 22 ff > RSP: 0018:ffffc900032cf938 EFLAGS: 00010293 > RAX: ffffffff84bfcba3 RBX: dffffc0000000000 RCX: ffff888107f61cc0 > RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000000000000 > RBP: ffff8881119a8008 R08: ffff888110bb69c7 R09: 1ffff11022176d38 > R10: dffffc0000000000 R11: ffffed1022176d39 R12: ffff8881119a8000 > R13: ffff888108441e90 R14: ffff888107f61cc0 R15: 0000000000000000 > FS: 00007f81f25716c0(0000) GS:ffff8881a39f5000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000001b31b63fff CR3: 000000010f24c000 CR4: 00000000000006f0 > Call Trace: > <TASK> > tctx_task_work_run+0x99/0x370 io_uring/io_uring.c:1223 > io_sq_tw io_uring/sqpoll.c:244 [inline] > io_sq_thread+0xed1/0x1e50 io_uring/sqpoll.c:327 > ret_from_fork+0x47f/0x820 arch/x86/kernel/process.c:148 > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 > </TASK> Probably the sanest thing to do here is to clear IORING_SETUP_SINGLE_ISSUER if it's set with IORING_SETUP_SQPOLL. If we allow it, it'll be impossible to uphold the locking criteria on both the issue and register side. -- Jens Axboe
On Wed, Sep 3, 2025 at 4:30 PM Jens Axboe <axboe@kernel.dk> wrote: > > On 9/3/25 3:55 PM, syzbot ci wrote: > > syzbot ci has tested the following series > > > > [v1] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > https://lore.kernel.org/all/20250903032656.2012337-1-csander@purestorage.com > > * [PATCH 1/4] io_uring: don't include filetable.h in io_uring.h > > * [PATCH 2/4] io_uring/rsrc: respect submitter_task in io_register_clone_buffers() > > * [PATCH 3/4] io_uring: factor out uring_lock helpers > > * [PATCH 4/4] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > > > and found the following issue: > > WARNING in io_handle_tw_list > > > > Full report is available here: > > https://ci.syzbot.org/series/54ae0eae-5e47-4cfe-9ae7-9eaaf959b5ae > > > > *** > > > > WARNING in io_handle_tw_list > > > > tree: linux-next > > URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next > > base: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 > > arch: amd64 > > compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8 > > config: https://ci.syzbot.org/builds/1de646dd-4ee2-418d-9c62-617d88ed4fd2/config > > syz repro: https://ci.syzbot.org/findings/e229a878-375f-4286-89fe-b6724c23addd/syz_repro > > > > ------------[ cut here ]------------ > > WARNING: io_uring/io_uring.h:127 at io_ring_ctx_lock io_uring/io_uring.h:127 [inline], CPU#1: iou-sqp-6294/6297 > > WARNING: io_uring/io_uring.h:127 at io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155, CPU#1: iou-sqp-6294/6297 > > Modules linked in: > > CPU: 1 UID: 0 PID: 6297 Comm: iou-sqp-6294 Not tainted syzkaller #0 PREEMPT(full) > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > > RIP: 0010:io_ring_ctx_lock io_uring/io_uring.h:127 [inline] > > RIP: 0010:io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155 > > Code: 00 00 48 c7 c7 e0 90 02 8c be 8e 04 00 00 31 d2 e8 01 e5 d2 fc 2e 2e 2e 31 c0 45 31 e4 4d 85 ff 75 89 eb 7c e8 ad fb 00 fd 90 <0f> 0b 90 e9 cf fe ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 22 ff > > RSP: 0018:ffffc900032cf938 EFLAGS: 00010293 > > RAX: ffffffff84bfcba3 RBX: dffffc0000000000 RCX: ffff888107f61cc0 > > RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000000000000 > > RBP: ffff8881119a8008 R08: ffff888110bb69c7 R09: 1ffff11022176d38 > > R10: dffffc0000000000 R11: ffffed1022176d39 R12: ffff8881119a8000 > > R13: ffff888108441e90 R14: ffff888107f61cc0 R15: 0000000000000000 > > FS: 00007f81f25716c0(0000) GS:ffff8881a39f5000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 0000001b31b63fff CR3: 000000010f24c000 CR4: 00000000000006f0 > > Call Trace: > > <TASK> > > tctx_task_work_run+0x99/0x370 io_uring/io_uring.c:1223 > > io_sq_tw io_uring/sqpoll.c:244 [inline] > > io_sq_thread+0xed1/0x1e50 io_uring/sqpoll.c:327 > > ret_from_fork+0x47f/0x820 arch/x86/kernel/process.c:148 > > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 > > </TASK> > > Probably the sanest thing to do here is to clear > IORING_SETUP_SINGLE_ISSUER if it's set with IORING_SETUP_SQPOLL. If we > allow it, it'll be impossible to uphold the locking criteria on both the > issue and register side. Yup, I was thinking the same thing. Thanks for taking a look. Best, Caleb
On Thu, Sep 4, 2025 at 7:52 AM Caleb Sander Mateos <csander@purestorage.com> wrote: > > On Wed, Sep 3, 2025 at 4:30 PM Jens Axboe <axboe@kernel.dk> wrote: > > > > On 9/3/25 3:55 PM, syzbot ci wrote: > > > syzbot ci has tested the following series > > > > > > [v1] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > > https://lore.kernel.org/all/20250903032656.2012337-1-csander@purestorage.com > > > * [PATCH 1/4] io_uring: don't include filetable.h in io_uring.h > > > * [PATCH 2/4] io_uring/rsrc: respect submitter_task in io_register_clone_buffers() > > > * [PATCH 3/4] io_uring: factor out uring_lock helpers > > > * [PATCH 4/4] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > > > > > and found the following issue: > > > WARNING in io_handle_tw_list > > > > > > Full report is available here: > > > https://ci.syzbot.org/series/54ae0eae-5e47-4cfe-9ae7-9eaaf959b5ae > > > > > > *** > > > > > > WARNING in io_handle_tw_list > > > > > > tree: linux-next > > > URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next > > > base: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 > > > arch: amd64 > > > compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8 > > > config: https://ci.syzbot.org/builds/1de646dd-4ee2-418d-9c62-617d88ed4fd2/config > > > syz repro: https://ci.syzbot.org/findings/e229a878-375f-4286-89fe-b6724c23addd/syz_repro > > > > > > ------------[ cut here ]------------ > > > WARNING: io_uring/io_uring.h:127 at io_ring_ctx_lock io_uring/io_uring.h:127 [inline], CPU#1: iou-sqp-6294/6297 > > > WARNING: io_uring/io_uring.h:127 at io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155, CPU#1: iou-sqp-6294/6297 > > > Modules linked in: > > > CPU: 1 UID: 0 PID: 6297 Comm: iou-sqp-6294 Not tainted syzkaller #0 PREEMPT(full) > > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > > > RIP: 0010:io_ring_ctx_lock io_uring/io_uring.h:127 [inline] > > > RIP: 0010:io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155 > > > Code: 00 00 48 c7 c7 e0 90 02 8c be 8e 04 00 00 31 d2 e8 01 e5 d2 fc 2e 2e 2e 31 c0 45 31 e4 4d 85 ff 75 89 eb 7c e8 ad fb 00 fd 90 <0f> 0b 90 e9 cf fe ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 22 ff > > > RSP: 0018:ffffc900032cf938 EFLAGS: 00010293 > > > RAX: ffffffff84bfcba3 RBX: dffffc0000000000 RCX: ffff888107f61cc0 > > > RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000000000000 > > > RBP: ffff8881119a8008 R08: ffff888110bb69c7 R09: 1ffff11022176d38 > > > R10: dffffc0000000000 R11: ffffed1022176d39 R12: ffff8881119a8000 > > > R13: ffff888108441e90 R14: ffff888107f61cc0 R15: 0000000000000000 > > > FS: 00007f81f25716c0(0000) GS:ffff8881a39f5000(0000) knlGS:0000000000000000 > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > CR2: 0000001b31b63fff CR3: 000000010f24c000 CR4: 00000000000006f0 > > > Call Trace: > > > <TASK> > > > tctx_task_work_run+0x99/0x370 io_uring/io_uring.c:1223 > > > io_sq_tw io_uring/sqpoll.c:244 [inline] > > > io_sq_thread+0xed1/0x1e50 io_uring/sqpoll.c:327 > > > ret_from_fork+0x47f/0x820 arch/x86/kernel/process.c:148 > > > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 > > > </TASK> > > > > Probably the sanest thing to do here is to clear > > IORING_SETUP_SINGLE_ISSUER if it's set with IORING_SETUP_SQPOLL. If we > > allow it, it'll be impossible to uphold the locking criteria on both the > > issue and register side. > > Yup, I was thinking the same thing. Thanks for taking a look. On further thought, IORING_SETUP_SQPOLL actually does guarantee a single issuer. io_uring_enter() already avoids taking the uring_lock in the IORING_SETUP_SQPOLL case because it doesn't issue any SQEs itself. Only the SQ thread does that, so it *is* the single issuer. The assertions I added in io_ring_ctx_lock()/io_ring_ctx_unlock() is just unnecessarily strict. It should expect current == ctx->sq_data->thread in the IORING_SETUP_SQPOLL case. Best, Caleb
On Thu, Sep 4, 2025 at 9:46 AM Caleb Sander Mateos <csander@purestorage.com> wrote: > > On Thu, Sep 4, 2025 at 7:52 AM Caleb Sander Mateos > <csander@purestorage.com> wrote: > > > > On Wed, Sep 3, 2025 at 4:30 PM Jens Axboe <axboe@kernel.dk> wrote: > > > > > > On 9/3/25 3:55 PM, syzbot ci wrote: > > > > syzbot ci has tested the following series > > > > > > > > [v1] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > > > https://lore.kernel.org/all/20250903032656.2012337-1-csander@purestorage.com > > > > * [PATCH 1/4] io_uring: don't include filetable.h in io_uring.h > > > > * [PATCH 2/4] io_uring/rsrc: respect submitter_task in io_register_clone_buffers() > > > > * [PATCH 3/4] io_uring: factor out uring_lock helpers > > > > * [PATCH 4/4] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER > > > > > > > > and found the following issue: > > > > WARNING in io_handle_tw_list > > > > > > > > Full report is available here: > > > > https://ci.syzbot.org/series/54ae0eae-5e47-4cfe-9ae7-9eaaf959b5ae > > > > > > > > *** > > > > > > > > WARNING in io_handle_tw_list > > > > > > > > tree: linux-next > > > > URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next > > > > base: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 > > > > arch: amd64 > > > > compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8 > > > > config: https://ci.syzbot.org/builds/1de646dd-4ee2-418d-9c62-617d88ed4fd2/config > > > > syz repro: https://ci.syzbot.org/findings/e229a878-375f-4286-89fe-b6724c23addd/syz_repro > > > > > > > > ------------[ cut here ]------------ > > > > WARNING: io_uring/io_uring.h:127 at io_ring_ctx_lock io_uring/io_uring.h:127 [inline], CPU#1: iou-sqp-6294/6297 > > > > WARNING: io_uring/io_uring.h:127 at io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155, CPU#1: iou-sqp-6294/6297 > > > > Modules linked in: > > > > CPU: 1 UID: 0 PID: 6297 Comm: iou-sqp-6294 Not tainted syzkaller #0 PREEMPT(full) > > > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > > > > RIP: 0010:io_ring_ctx_lock io_uring/io_uring.h:127 [inline] > > > > RIP: 0010:io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155 > > > > Code: 00 00 48 c7 c7 e0 90 02 8c be 8e 04 00 00 31 d2 e8 01 e5 d2 fc 2e 2e 2e 31 c0 45 31 e4 4d 85 ff 75 89 eb 7c e8 ad fb 00 fd 90 <0f> 0b 90 e9 cf fe ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 22 ff > > > > RSP: 0018:ffffc900032cf938 EFLAGS: 00010293 > > > > RAX: ffffffff84bfcba3 RBX: dffffc0000000000 RCX: ffff888107f61cc0 > > > > RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000000000000 > > > > RBP: ffff8881119a8008 R08: ffff888110bb69c7 R09: 1ffff11022176d38 > > > > R10: dffffc0000000000 R11: ffffed1022176d39 R12: ffff8881119a8000 > > > > R13: ffff888108441e90 R14: ffff888107f61cc0 R15: 0000000000000000 > > > > FS: 00007f81f25716c0(0000) GS:ffff8881a39f5000(0000) knlGS:0000000000000000 > > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > CR2: 0000001b31b63fff CR3: 000000010f24c000 CR4: 00000000000006f0 > > > > Call Trace: > > > > <TASK> > > > > tctx_task_work_run+0x99/0x370 io_uring/io_uring.c:1223 > > > > io_sq_tw io_uring/sqpoll.c:244 [inline] > > > > io_sq_thread+0xed1/0x1e50 io_uring/sqpoll.c:327 > > > > ret_from_fork+0x47f/0x820 arch/x86/kernel/process.c:148 > > > > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 > > > > </TASK> > > > > > > Probably the sanest thing to do here is to clear > > > IORING_SETUP_SINGLE_ISSUER if it's set with IORING_SETUP_SQPOLL. If we > > > allow it, it'll be impossible to uphold the locking criteria on both the > > > issue and register side. > > > > Yup, I was thinking the same thing. Thanks for taking a look. > > On further thought, IORING_SETUP_SQPOLL actually does guarantee a > single issuer. io_uring_enter() already avoids taking the uring_lock > in the IORING_SETUP_SQPOLL case because it doesn't issue any SQEs > itself. Only the SQ thread does that, so it *is* the single issuer. > The assertions I added in io_ring_ctx_lock()/io_ring_ctx_unlock() is > just unnecessarily strict. It should expect current == > ctx->sq_data->thread in the IORING_SETUP_SQPOLL case. Oh, but you are totally correct about needing the mutex to synchronize between issue on the SQ thread and io_uring_register() on other threads. Yeah, I don't see an easy way to avoid taking the mutex on the SQ thread unless we disallowed io_uring_register() completely. Clearing IORING_SETUP_SINGLE_ISSUER seems like the best option for now. Best, Caleb
On 9/4/25 10:50 AM, Caleb Sander Mateos wrote: > On Thu, Sep 4, 2025 at 9:46?AM Caleb Sander Mateos > <csander@purestorage.com> wrote: >> >> On Thu, Sep 4, 2025 at 7:52?AM Caleb Sander Mateos >> <csander@purestorage.com> wrote: >>> >>> On Wed, Sep 3, 2025 at 4:30?PM Jens Axboe <axboe@kernel.dk> wrote: >>>> >>>> On 9/3/25 3:55 PM, syzbot ci wrote: >>>>> syzbot ci has tested the following series >>>>> >>>>> [v1] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER >>>>> https://lore.kernel.org/all/20250903032656.2012337-1-csander@purestorage.com >>>>> * [PATCH 1/4] io_uring: don't include filetable.h in io_uring.h >>>>> * [PATCH 2/4] io_uring/rsrc: respect submitter_task in io_register_clone_buffers() >>>>> * [PATCH 3/4] io_uring: factor out uring_lock helpers >>>>> * [PATCH 4/4] io_uring: avoid uring_lock for IORING_SETUP_SINGLE_ISSUER >>>>> >>>>> and found the following issue: >>>>> WARNING in io_handle_tw_list >>>>> >>>>> Full report is available here: >>>>> https://ci.syzbot.org/series/54ae0eae-5e47-4cfe-9ae7-9eaaf959b5ae >>>>> >>>>> *** >>>>> >>>>> WARNING in io_handle_tw_list >>>>> >>>>> tree: linux-next >>>>> URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next >>>>> base: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 >>>>> arch: amd64 >>>>> compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8 >>>>> config: https://ci.syzbot.org/builds/1de646dd-4ee2-418d-9c62-617d88ed4fd2/config >>>>> syz repro: https://ci.syzbot.org/findings/e229a878-375f-4286-89fe-b6724c23addd/syz_repro >>>>> >>>>> ------------[ cut here ]------------ >>>>> WARNING: io_uring/io_uring.h:127 at io_ring_ctx_lock io_uring/io_uring.h:127 [inline], CPU#1: iou-sqp-6294/6297 >>>>> WARNING: io_uring/io_uring.h:127 at io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155, CPU#1: iou-sqp-6294/6297 >>>>> Modules linked in: >>>>> CPU: 1 UID: 0 PID: 6297 Comm: iou-sqp-6294 Not tainted syzkaller #0 PREEMPT(full) >>>>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 >>>>> RIP: 0010:io_ring_ctx_lock io_uring/io_uring.h:127 [inline] >>>>> RIP: 0010:io_handle_tw_list+0x234/0x2e0 io_uring/io_uring.c:1155 >>>>> Code: 00 00 48 c7 c7 e0 90 02 8c be 8e 04 00 00 31 d2 e8 01 e5 d2 fc 2e 2e 2e 31 c0 45 31 e4 4d 85 ff 75 89 eb 7c e8 ad fb 00 fd 90 <0f> 0b 90 e9 cf fe ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 22 ff >>>>> RSP: 0018:ffffc900032cf938 EFLAGS: 00010293 >>>>> RAX: ffffffff84bfcba3 RBX: dffffc0000000000 RCX: ffff888107f61cc0 >>>>> RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000000000000 >>>>> RBP: ffff8881119a8008 R08: ffff888110bb69c7 R09: 1ffff11022176d38 >>>>> R10: dffffc0000000000 R11: ffffed1022176d39 R12: ffff8881119a8000 >>>>> R13: ffff888108441e90 R14: ffff888107f61cc0 R15: 0000000000000000 >>>>> FS: 00007f81f25716c0(0000) GS:ffff8881a39f5000(0000) knlGS:0000000000000000 >>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>>>> CR2: 0000001b31b63fff CR3: 000000010f24c000 CR4: 00000000000006f0 >>>>> Call Trace: >>>>> <TASK> >>>>> tctx_task_work_run+0x99/0x370 io_uring/io_uring.c:1223 >>>>> io_sq_tw io_uring/sqpoll.c:244 [inline] >>>>> io_sq_thread+0xed1/0x1e50 io_uring/sqpoll.c:327 >>>>> ret_from_fork+0x47f/0x820 arch/x86/kernel/process.c:148 >>>>> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 >>>>> </TASK> >>>> >>>> Probably the sanest thing to do here is to clear >>>> IORING_SETUP_SINGLE_ISSUER if it's set with IORING_SETUP_SQPOLL. If we >>>> allow it, it'll be impossible to uphold the locking criteria on both the >>>> issue and register side. >>> >>> Yup, I was thinking the same thing. Thanks for taking a look. >> >> On further thought, IORING_SETUP_SQPOLL actually does guarantee a >> single issuer. io_uring_enter() already avoids taking the uring_lock >> in the IORING_SETUP_SQPOLL case because it doesn't issue any SQEs >> itself. Only the SQ thread does that, so it *is* the single issuer. >> The assertions I added in io_ring_ctx_lock()/io_ring_ctx_unlock() is >> just unnecessarily strict. It should expect current == >> ctx->sq_data->thread in the IORING_SETUP_SQPOLL case. > > Oh, but you are totally correct about needing the mutex to synchronize > between issue on the SQ thread and io_uring_register() on other > threads. Yeah, I don't see an easy way to avoid taking the mutex on > the SQ thread unless we disallowed io_uring_register() completely. > Clearing IORING_SETUP_SINGLE_ISSUER seems like the best option for > now. Right - I don't disagree that SQPOLL is the very definition of "single issuer", but it'll still have to contend with the creating task doing other operations that they would need mutual exclusion for. I don't think clearing SINGLE_ISSUER on SQPOLL is a big deal, it's not like it's worse off than before. It's just not getting the same optimizations that the !SQPOLL single issuer path would get. -- Jens Axboe
© 2016 - 2025 Red Hat, Inc.