From: Christoph Hellwig <hch@infradead.org>
To: Fengnan Chang <changfengnan@bytedance.com>
Cc: brauner@kernel.org, djwong@kernel.org, hch@infradead.org,
ojaswin@linux.ibm.com, dgc@kernel.org, linux-xfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-kernel@vger.kernel.org, lidiangang@bytedance.com,
pankaj.raghav@linux.dev, Joanne Koong <joannelkoong@gmail.com>
Subject: Re: [PATCH v6 3/3] iomap: add simple dio path for small direct I/O
Date: Wed, 1 Jul 2026 04:23:32 -0700 [thread overview]
Message-ID: <akT4tJzFSfrBxqdq@infradead.org> (raw)
In-Reply-To: <20260701033253.46420-4-changfengnan@bytedance.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
We'll need to sort out the interaction with the iomap_next work, though.
Joanne, is it ok to get this in first and have Fengnan help your with
the interactions due to the open coded iomap_begin/end calls?
On Wed, Jul 01, 2026 at 11:32:53AM +0800, Fengnan Chang wrote:
> When running 4K random read workloads on high-performance Gen5 NVMe
> SSDs, the software overhead in the iomap direct I/O path
> (__iomap_dio_rw) becomes a significant bottleneck.
>
> Using io_uring with poll mode for a 4K randread test on a raw block
> device:
> taskset -c 30 ./t/io_uring -p1 -d512 -b4096 -s32 -c32 -F1 -B1 -R1 -X1
> -n1 -P1 /dev/nvme10n1
> Result: ~3.2M IOPS
>
> Running the exact same workload on ext4 and XFS:
> taskset -c 30 ./t/io_uring -p1 -d512 -b4096 -s32 -c32 -F1 -B1 -R1 -X1
> -n1 -P1 /mnt/testfile
> Result: ~1.92M IOPS
>
> Profiling the ext4 workload reveals that a significant portion of CPU
> time is spent on memory allocation and the iomap state machine
> iteration:
> 5.33% [kernel] [k] __iomap_dio_rw
> 3.26% [kernel] [k] iomap_iter
> 2.37% [kernel] [k] iomap_dio_bio_iter
> 2.35% [kernel] [k] kfree
> 1.33% [kernel] [k] iomap_dio_complete
>
> Introduce a simple dio path to reduce the overhead of iomap. It is
> triggered when the request satisfies all of:
> - a READ request whose I/O size is <= inode blocksize (fits in a single
> block, no splits);
> - no custom iomap_dio_ops (dops) registered by the filesystem;
> - no caller-accumulated residual (done_before == 0);
> - none of IOMAP_DIO_FORCE_WAIT / IOMAP_DIO_PARTIAL / IOMAP_DIO_BOUNCE
> set, the range is within i_size, and the inode is not encrypted.
>
> The bio is allocated from a dedicated bioset whose front_pad embeds
> struct iomap_dio_simple, so the whole request lives in a single
> cacheline-aligned allocation and no separate struct iomap_dio is
> needed. Completion is handled inline from ->bi_end_io for the common
> success case, and only punted to the s_dio_done_wq workqueue on error.
>
> After this optimization, the heavy generic functions disappear from the
> profile, replaced by a single streamlined execution path:
> 4.83% [kernel] [k] iomap_dio_simple
>
> With this patch, 4K random read IOPS on ext4 increases from 1.92M to
> 2.19M in the original single-core io_uring poll-mode workload.
>
> Below are the test results using fio:
>
> fs workload qd simple=0 simple=1 gain
> ext4 libaio 1 18,740 18,761 +0.11%
> ext4 libaio 64 462,850 480,587 +3.83%
> ext4 libaio 128 459,498 478,824 +4.21%
> ext4 libaio 256 459,938 480,156 +4.40%
> ext4 io_uring 1 18,836 18,880 +0.24%
> ext4 io_uring 64 568,193 600,625 +5.71%
> ext4 io_uring 128 570,998 602,148 +5.46%
> ext4 io_uring 256 572,052 602,536 +5.33%
> ext4 io_uring_poll 1 19,283 19,272 -0.06%
> ext4 io_uring_poll 64 989,735 1,013,342 +2.39%
> ext4 io_uring_poll 128 1,467,336 1,538,444 +4.85%
> ext4 io_uring_poll 256 1,663,498 1,830,842 +10.06%
> xfs libaio 1 18,764 18,776 +0.06%
> xfs libaio 64 462,408 480,860 +3.99%
> xfs libaio 128 461,280 480,819 +4.24%
> xfs libaio 256 461,626 480,190 +4.02%
> xfs io_uring 1 18,871 18,903 +0.17%
> xfs io_uring 64 570,383 597,399 +4.74%
> xfs io_uring 128 568,290 597,370 +5.12%
> xfs io_uring 256 570,616 598,775 +4.93%
> xfs io_uring_poll 1 19,211 19,315 +0.54%
> xfs io_uring_poll 64 989,726 1,008,455 +1.89%
> xfs io_uring_poll 128 1,430,426 1,513,064 +5.78%
> xfs io_uring_poll 256 1,587,339 1,742,220 +9.76%
>
> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> ---
> fs/iomap/direct-io.c | 274 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 274 insertions(+)
>
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 1b9abdd831d0b..ca790239e5eb3 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -10,6 +10,7 @@
> #include <linux/iomap.h>
> #include <linux/task_io_accounting_ops.h>
> #include <linux/fserror.h>
> +#include <linux/init.h>
> #include "internal.h"
> #include "trace.h"
>
> @@ -893,12 +894,277 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> }
> EXPORT_SYMBOL_GPL(__iomap_dio_rw);
>
> +struct iomap_dio_simple {
> + struct kiocb *iocb;
> + size_t size;
> + unsigned int dio_flags;
> + struct work_struct work;
> + /*
> + * Align @bio to a cacheline boundary so that, combined with the
> + * front_pad passed to bioset_init(), the bio sits at the start of
> + * a cacheline in memory returned by the (HWCACHE-aligned) bio
> + * slab. This keeps the hot fields block layer touches on submit
> + * and completion (bi_iter, bi_status, ...) within a single line.
> + */
> + struct bio bio ____cacheline_aligned_in_smp;
> +};
> +
> +static struct bio_set iomap_dio_simple_pool;
> +
> +static ssize_t iomap_dio_simple_complete(struct iomap_dio_simple *sr)
> +{
> + struct bio *bio = &sr->bio;
> + struct kiocb *iocb = sr->iocb;
> + struct inode *inode = file_inode(iocb->ki_filp);
> + ssize_t ret;
> +
> + if (unlikely(bio->bi_status)) {
> + ret = blk_status_to_errno(bio->bi_status);
> + if (should_report_dio_fserror(ret))
> + fserror_report_io(inode, FSERR_DIRECTIO_READ,
> + iocb->ki_pos, sr->size, ret,
> + GFP_NOFS);
> + } else {
> + ret = sr->size;
> + iocb->ki_pos += ret;
> + }
> +
> + if (sr->dio_flags & IOMAP_DIO_USER_BACKED) {
> + bio_check_pages_dirty(bio);
> + } else {
> + bio_release_pages(bio, false);
> + bio_put(bio);
> + }
> + inode_dio_end(inode);
> + trace_iomap_dio_complete(iocb, ret < 0 ? ret : 0, ret);
> + return ret;
> +}
> +
> +static void iomap_dio_simple_complete_work(struct work_struct *work)
> +{
> + struct iomap_dio_simple *sr =
> + container_of(work, struct iomap_dio_simple, work);
> + struct kiocb *iocb = sr->iocb;
> +
> + WRITE_ONCE(iocb->private, NULL);
> + iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
> +}
> +
> +static void iomap_dio_simple_end_io(struct bio *bio)
> +{
> + struct iomap_dio_simple *sr =
> + container_of(bio, struct iomap_dio_simple, bio);
> + struct kiocb *iocb = sr->iocb;
> +
> + if (unlikely(sr->bio.bi_status)) {
> + struct inode *inode = file_inode(iocb->ki_filp);
> +
> + INIT_WORK(&sr->work, iomap_dio_simple_complete_work);
> + queue_work(inode->i_sb->s_dio_done_wq, &sr->work);
> + return;
> + }
> +
> + WRITE_ONCE(iocb->private, NULL);
> + iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
> +}
> +
> +static inline bool
> +iomap_dio_simple_supported(struct kiocb *iocb, struct iov_iter *iter,
> + const struct iomap_dio_ops *dops,
> + unsigned int dio_flags, size_t done_before)
> +{
> + struct inode *inode = file_inode(iocb->ki_filp);
> + size_t count = iov_iter_count(iter);
> +
> + if (dops || done_before)
> + return false;
> + if (iov_iter_rw(iter) != READ)
> + return false;
> + if (!count)
> + return false;
> + /*
> + * Simple dio is an optimization for small IO. Filter out large IO
> + * early as it's the most common case to fail for typical direct IO
> + * workloads.
> + */
> + if (count > inode->i_sb->s_blocksize)
> + return false;
> + if (dio_flags & (IOMAP_DIO_FORCE_WAIT | IOMAP_DIO_PARTIAL |
> + IOMAP_DIO_BOUNCE))
> + return false;
> + if (iocb->ki_pos + count > i_size_read(inode))
> + return false;
> + if (IS_ENCRYPTED(inode))
> + return false;
> +
> + return true;
> +}
> +
> +/*
> + * Fast path for small, block-aligned direct I/Os that map to a single
> + * contiguous on-disk extent.
> + *
> + * iomap_dio_simple_supported() enforces the cheap up-front constraints before
> + * entering this path.
> + *
> + * @dops must be NULL: a non-NULL @dops means the caller wants its
> + * ->end_io / ->submit_io hooks invoked, and in particular wants its bios to be
> + * allocated from the filesystem-private @dops->bio_set (whose front_pad sizes a
> + * filesystem-private wrapper around the bio). The fast path instead allocates
> + * from the shared iomap_dio_simple_pool, whose front_pad matches struct
> + * iomap_dio_simple; the two wrappers are not interchangeable, so we must fall
> + * back to __iomap_dio_rw() in that case.
> + *
> + * @done_before must be zero: a non-zero caller-accumulated residual cannot be
> + * carried through a single-bio inline completion.
> + *
> + * @iter must describe a non-empty READ no larger than the inode block size:
> + * writes, zero-length I/O, and larger requests need the generic iomap direct
> + * I/O path.
> + *
> + * @dio_flags must not request IOMAP_DIO_FORCE_WAIT, IOMAP_DIO_PARTIAL, or
> + * IOMAP_DIO_BOUNCE: this path does not support forced waiting, partial direct
> + * I/O, or bouncing. The range must also stay within i_size and encrypted
> + * inodes must use the generic iomap direct I/O path.
> + *
> + * -ENOTBLK is the private sentinel returned by iomap_dio_simple() when it
> + * decides the request does not fit the fast path. In that case we proceed to
> + * the generic __iomap_dio_rw() slow path. Any other errno is a real result and
> + * is propagated as-is, in particular -EAGAIN for IOCB_NOWAIT must reach the
> + * caller.
> + */
> +static ssize_t
> +iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
> + const struct iomap_ops *ops, void *private,
> + unsigned int dio_flags)
> +{
> + struct inode *inode = file_inode(iocb->ki_filp);
> + size_t count = iov_iter_count(iter);
> + bool wait_for_completion = is_sync_kiocb(iocb);
> + struct iomap_iter iomi = {
> + .inode = inode,
> + .pos = iocb->ki_pos,
> + .len = count,
> + .flags = IOMAP_DIRECT,
> + .private = private,
> + };
> + struct iomap_dio_simple *sr;
> + unsigned int alignment;
> + struct bio *bio;
> + ssize_t ret;
> +
> + if (iocb->ki_flags & IOCB_NOWAIT)
> + iomi.flags |= IOMAP_NOWAIT;
> +
> + ret = kiocb_write_and_wait(iocb, count);
> + if (ret)
> + return ret;
> +
> + inode_dio_begin(inode);
> +
> + ret = ops->iomap_begin(inode, iomi.pos, count, iomi.flags,
> + &iomi.iomap, &iomi.srcmap);
> + if (ret) {
> + inode_dio_end(inode);
> + return ret;
> + }
> +
> + if (iomi.iomap.type != IOMAP_MAPPED ||
> + iomi.iomap.offset + iomi.iomap.length < iomi.pos + count ||
> + (iomi.iomap.flags & IOMAP_F_INTEGRITY)) {
> + ret = -ENOTBLK;
> + goto out_iomap_end;
> + }
> +
> + alignment = iomap_dio_alignment(inode, iomi.iomap.bdev, dio_flags);
> + if ((iomi.pos | count) & (alignment - 1)) {
> + ret = -EINVAL;
> + goto out_iomap_end;
> + }
> +
> + if (!wait_for_completion && unlikely(!inode->i_sb->s_dio_done_wq)) {
> + ret = sb_init_dio_done_wq(inode->i_sb);
> + if (ret < 0)
> + goto out_iomap_end;
> + }
> +
> + trace_iomap_dio_rw_begin(iocb, iter, dio_flags, 0);
> +
> + if (user_backed_iter(iter))
> + dio_flags |= IOMAP_DIO_USER_BACKED;
> +
> + bio = bio_alloc_bioset(iomi.iomap.bdev,
> + bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS),
> + REQ_OP_READ, GFP_KERNEL, &iomap_dio_simple_pool);
> + sr = container_of(bio, struct iomap_dio_simple, bio);
> + sr->iocb = iocb;
> + sr->dio_flags = dio_flags;
> +
> + bio->bi_iter.bi_sector = iomap_sector(&iomi.iomap, iomi.pos);
> + bio->bi_ioprio = iocb->ki_ioprio;
> +
> + ret = bio_iov_iter_get_pages(bio, iter, alignment - 1);
> + if (unlikely(ret))
> + goto out_bio_put;
> +
> + if (bio->bi_iter.bi_size != count) {
> + iov_iter_revert(iter, bio->bi_iter.bi_size);
> + ret = -ENOTBLK;
> + goto out_bio_release_pages;
> + }
> +
> + sr->size = bio->bi_iter.bi_size;
> +
> + if (dio_flags & IOMAP_DIO_USER_BACKED)
> + bio_set_pages_dirty(bio);
> +
> + if (iocb->ki_flags & IOCB_NOWAIT)
> + bio->bi_opf |= REQ_NOWAIT;
> + if ((iocb->ki_flags & IOCB_HIPRI) && !wait_for_completion) {
> + bio->bi_opf |= REQ_POLLED;
> + WRITE_ONCE(iocb->private, bio);
> + }
> +
> + if (ops->iomap_end)
> + ops->iomap_end(inode, iomi.pos, count, count, iomi.flags,
> + &iomi.iomap);
> +
> + if (!wait_for_completion) {
> + bio->bi_end_io = iomap_dio_simple_end_io;
> + submit_bio(bio);
> + trace_iomap_dio_rw_queued(inode, iomi.pos, count);
> + return -EIOCBQUEUED;
> + }
> +
> + submit_bio_wait(bio);
> + return iomap_dio_simple_complete(sr);
> +
> +out_bio_release_pages:
> + bio_release_pages(bio, false);
> +out_bio_put:
> + bio_put(bio);
> +out_iomap_end:
> + if (ops->iomap_end)
> + ops->iomap_end(inode, iomi.pos, count, 0, iomi.flags,
> + &iomi.iomap);
> + inode_dio_end(inode);
> + return ret;
> +}
> +
> ssize_t
> iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
> unsigned int dio_flags, void *private, size_t done_before)
> {
> struct iomap_dio *dio;
> + ssize_t ret;
> +
> + if (iomap_dio_simple_supported(iocb, iter, dops, dio_flags,
> + done_before)) {
> + ret = iomap_dio_simple(iocb, iter, ops, private, dio_flags);
> + if (ret != -ENOTBLK)
> + return ret;
> + }
>
> dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, private,
> done_before);
> @@ -907,3 +1173,11 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> return iomap_dio_complete(dio);
> }
> EXPORT_SYMBOL_GPL(iomap_dio_rw);
> +
> +static int __init iomap_dio_init(void)
> +{
> + return bioset_init(&iomap_dio_simple_pool, 4,
> + offsetof(struct iomap_dio_simple, bio),
> + BIOSET_NEED_BVECS | BIOSET_PERCPU_CACHE);
> +}
> +fs_initcall(iomap_dio_init);
> --
> 2.39.5 (Apple Git-154)
---end quoted text---
next prev parent reply other threads:[~2026-07-01 11:23 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 3:32 [PATCH v6 0/3] iomap: add simple dio path for small direct I/O Fengnan Chang
2026-07-01 3:32 ` [PATCH v6 1/3] iomap: factor out iomap_dio_alignment helper Fengnan Chang
2026-07-01 3:32 ` [PATCH v6 2/3] iomap: pass error code to should_report_dio_fserror directly Fengnan Chang
2026-07-01 3:32 ` [PATCH v6 3/3] iomap: add simple dio path for small direct I/O Fengnan Chang
2026-07-01 11:23 ` Christoph Hellwig [this message]
2026-07-01 19:35 ` Joanne Koong
2026-07-02 14:35 ` Christoph Hellwig
2026-07-03 3:05 ` Joanne Koong
2026-07-03 9:13 ` changfengnan
2026-07-03 12:40 ` Christoph Hellwig
2026-07-06 11:09 ` changfengnan
2026-07-06 19:27 ` Joanne Koong
2026-07-03 9:22 ` changfengnan
2026-07-01 12:44 ` [PATCH v6 0/3] " Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=akT4tJzFSfrBxqdq@infradead.org \
--to=hch@infradead.org \
--cc=brauner@kernel.org \
--cc=changfengnan@bytedance.com \
--cc=dgc@kernel.org \
--cc=djwong@kernel.org \
--cc=joannelkoong@gmail.com \
--cc=lidiangang@bytedance.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=ojaswin@linux.ibm.com \
--cc=pankaj.raghav@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox