[PATCH] iomap: allocate s_dio_done_wq for async reads as well

[PATCH] iomap: allocate s_dio_done_wq for async reads as well

[Christoph Hellwig]

Since commit 222f2c7c6d14 ("iomap: always run error completions in user
context"), read error completions are deferred to s_dio_done_wq.  This
means the workqueue also needs to be allocated for async reads.

Fixes: 222f2c7c6d14 ("iomap: always run error completions in user context")
Reported-by: syzbot+a2b9a4ed0d61b1efb3f5@syzkaller.appspotmail.com
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: syzbot+a2b9a4ed0d61b1efb3f5@syzkaller.appspotmail.com
---
 fs/iomap/direct-io.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index d4e2e328d893..8e273408453a 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -738,12 +738,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 			}
 			goto out_free_dio;
 		}
+	}
 
-		if (!wait_for_completion && !inode->i_sb->s_dio_done_wq) {
-			ret = sb_init_dio_done_wq(inode->i_sb);
-			if (ret < 0)
-				goto out_free_dio;
-		}
+	if (!wait_for_completion && !inode->i_sb->s_dio_done_wq) {
+		ret = sb_init_dio_done_wq(inode->i_sb);
+		if (ret < 0)
+			goto out_free_dio;
 	}
 
 	inode_dio_begin(inode);
-- 
2.47.3

Re: [PATCH] iomap: allocate s_dio_done_wq for async reads as well

[Darrick J. Wong]

On Mon, Nov 24, 2025 at 03:00:13PM +0100, Christoph Hellwig wrote:
> Since commit 222f2c7c6d14 ("iomap: always run error completions in user
> context"), read error completions are deferred to s_dio_done_wq.  This
> means the workqueue also needs to be allocated for async reads.
> 
> Fixes: 222f2c7c6d14 ("iomap: always run error completions in user context")
> Reported-by: syzbot+a2b9a4ed0d61b1efb3f5@syzkaller.appspotmail.com
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: syzbot+a2b9a4ed0d61b1efb3f5@syzkaller.appspotmail.com

Heh, ooops. :/

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index d4e2e328d893..8e273408453a 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -738,12 +738,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  			}
>  			goto out_free_dio;
>  		}
> +	}
>  
> -		if (!wait_for_completion && !inode->i_sb->s_dio_done_wq) {
> -			ret = sb_init_dio_done_wq(inode->i_sb);
> -			if (ret < 0)
> -				goto out_free_dio;
> -		}
> +	if (!wait_for_completion && !inode->i_sb->s_dio_done_wq) {
> +		ret = sb_init_dio_done_wq(inode->i_sb);
> +		if (ret < 0)
> +			goto out_free_dio;
>  	}
>  
>  	inode_dio_begin(inode);
> -- 
> 2.47.3
> 
> 

Re: [PATCH] iomap: allocate s_dio_done_wq for async reads as well

[Dave Chinner]

On Mon, Nov 24, 2025 at 03:00:13PM +0100, Christoph Hellwig wrote:
> Since commit 222f2c7c6d14 ("iomap: always run error completions in user
> context"), read error completions are deferred to s_dio_done_wq.  This
> means the workqueue also needs to be allocated for async reads.

The change LGTM, so:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

But I can't help but wonder about putting this in the fast path....

i.e. on the first io_uring/AIO DIO read or write, we will need
allocate this work queue. I think that's getting quite common in
applications and utilities these days, so filesystems are
increasingly likely to need this wq.

Maybe we should make this wq init unconditional and move it to fs
superblock initialisation?  That would remove this "only needed once
for init" check that is made on every call through the the IO fast
path....

-Dave.

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] iomap: allocate s_dio_done_wq for async reads as well

[Christoph Hellwig]

On Tue, Nov 25, 2025 at 08:45:00AM +1100, Dave Chinner wrote:
> But I can't help but wonder about putting this in the fast path....
> 
> i.e. on the first io_uring/AIO DIO read or write, we will need
> allocate this work queue. I think that's getting quite common in
> applications and utilities these days, so filesystems are
> increasingly likely to need this wq.
> 
> Maybe we should make this wq init unconditional and move it to fs
> superblock initialisation?  That would remove this "only needed once
> for init" check that is made on every call through the the IO fast
> path....

I agree, and I originally did that, and it caused a long bikeshed.
So not feeling like reopening that can of worms right now even if I
agree with the sentiment.

Re: [PATCH] iomap: allocate s_dio_done_wq for async reads as well

[Christian Brauner]

On Mon, 24 Nov 2025 15:00:13 +0100, Christoph Hellwig wrote:
> Since commit 222f2c7c6d14 ("iomap: always run error completions in user
> context"), read error completions are deferred to s_dio_done_wq.  This
> means the workqueue also needs to be allocated for async reads.
> 
> 

Applied to the vfs-6.19.iomap branch of the vfs/vfs.git tree.
Patches in the vfs-6.19.iomap branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.19.iomap

[1/1] iomap: allocate s_dio_done_wq for async reads as well
      https://git.kernel.org/vfs/vfs/c/d2560991c66f