Re: [PATCH v6 3/3] iomap: add simple dio path for small direct I/O
From: Joanne Koong
Date: Tue Jul 07 2026 - 22:08:19 EST
On Mon, Jul 6, 2026 at 11:57 PM changfengnan <changfengnan@xxxxxxxxxxxxx> wrote:
>
>
> > From: "Joanne Koong"<joannelkoong@xxxxxxxxx>
> > Date: Tue, Jul 7, 2026, 03:27
> > Subject: Re: [PATCH v6 3/3] iomap: add simple dio path for small direct I/O
> > To: "changfengnan"<changfengnan@xxxxxxxxxxxxx>
> > Cc: "Christoph Hellwig"<hch@xxxxxxxxxxxxx>, "brauner"<brauner@xxxxxxxxxx>, "djwong"<djwong@xxxxxxxxxx>, "ojaswin"<ojaswin@xxxxxxxxxxxxx>, "dgc"<dgc@xxxxxxxxxx>, "linux-xfs"<linux-xfs@xxxxxxxxxxxxxxx>, "linux-fsdevel"<linux-fsdevel@xxxxxxxxxxxxxxx>, "linux-ext4"<linux-ext4@xxxxxxxxxxxxxxx>, "linux-kernel"<linux-kernel@xxxxxxxxxxxxxxx>, <lidiangang@xxxxxxxxxxxxx>, "pankaj.raghav"<pankaj.raghav@xxxxxxxxx>, "Brian Foster"<bfoster@xxxxxxxxxx>
> > On Mon, Jul 6, 2026 at 4:10 AM changfengnan <changfengnan@xxxxxxxxxxxxx> wrote:
> > >
> > > > From: "Christoph Hellwig"<hch@xxxxxxxxxxxxx>
> > > > Date: Fri, Jul 3, 2026, 20:40
> > > > Subject: Re: [PATCH v6 3/3] iomap: add simple dio path for small direct I/O
> > > > To: "changfengnan"<changfengnan@xxxxxxxxxxxxx>
> > > > Cc: "Joanne Koong"<joannelkoong@xxxxxxxxx>, "Christoph Hellwig"<hch@xxxxxxxxxxxxx>, "brauner"<brauner@xxxxxxxxxx>, "djwong"<djwong@xxxxxxxxxx>, "ojaswin"<ojaswin@xxxxxxxxxxxxx>, "dgc"<dgc@xxxxxxxxxx>, "linux-xfs"<linux-xfs@xxxxxxxxxxxxxxx>, "linux-fsdevel"<linux-fsdevel@xxxxxxxxxxxxxxx>, "linux-ext4"<linux-ext4@xxxxxxxxxxxxxxx>, "linux-kernel"<linux-kernel@xxxxxxxxxxxxxxx>, <lidiangang@xxxxxxxxxxxxx>, "pankaj.raghav"<pankaj.raghav@xxxxxxxxx>, "Brian Foster"<bfoster@xxxxxxxxxx>
> > > > On Fri, Jul 03, 2026 at 05:13:12PM +0800, changfengnan wrote:
> > > > > taken, which introduced some overhead.
> > > > > I implemented some optimizations based on Christoph’s suggestions and
> > > > > made some modifications to `iomap_process`;
> > > > > Now, XFS performance is essentially unchanged, while ext4 still shows a
> > > > > 1.5% drop; I will continue to investigate the cause.. I’ve attached the patch.
> > > > > Actually, I had expected that switching to `iomap_next` would improve
> > > > > performance, since it avoids an indirect call.
> > > >
> > > > I don't think it will for you - the direct I/O readers don't have
> > > > iomap_end methods, so you still have the same number of indirect
> > > > calls. What could help is to inline the iterator as seen in Joannes'
> > > > fist demo series. We could even look into doing that only for simple
> > > > dio in a first step, shifting the burden to use the simple method
> > > > to the callers (at least for the POC).
> > >
> > > I'm not sure if this is what you had in mind, but I made some
> > > adjustments referencing Joannes' first patch, and using this approach,
> > > there was no performance degradation.
> >
> > Thanks, Fengnan. This is awesome that it got the degradation down to zero.
> >
> > Would you be able to test how this change performs on your benchmarks?:
>
> XFS basically has no performance degradation, while ext4 experiences
> a 1-1.5% performance degradation.
>
> Round 1:
> d67 xfs : 2.04M, 2.05M, 2.04M, 2.04M, 2.04M avg=2.042M
> d67 ext4: 2.12M, 2.12M, 2.12M, 2.12M, 2.12M avg=2.120M
> joanne86 xfs : 2.13M, 2.05M, 2.04M, 2.05M, 2.05M avg=2.064M
> joanne86 ext4: 2.11M, 2.11M, 2.11M, 2.11M, 2.11M avg=2.110M
>
> Round 2:
> d67 xfs : 2.05M, 2.05M, 2.05M, 2.05M, 2.05M avg=2.050M
> d67 ext4: 2.12M, 2.12M, 2.12M, 2.12M, 2.12M avg=2.120M
> joanne86 xfs : 2.04M, 2.04M, 2.04M, 2.04M, 2.05M avg=2.042M
> joanne86 ext4: 2.09M, 2.08M, 2.09M, 2.10M, 2.09M avg=2.090M
>
> Round 3:
> d67 xfs : 2.04M, 2.04M, 2.04M, 2.04M, 2.05M avg=2.042M
> d67 ext4: 2.12M, 2.12M, 2.11M, 2.12M, 2.12M avg=2.118M
> joanne86 xfs : 2.04M, 2.05M, 2.04M, 2.04M, 2.04M avg=2.042M
> joanne86 ext4: 2.09M, 2.09M, 2.08M, 2.09M, 2.09M avg=2.088M
>
Thanks. I'm not sure why it makes ext4 worse. Maybe it being inlined
bloats it too much so it leads to more frontend stalls/icache misses
than what the devirtualization helps with? Looking at the begin
functions, ext4's is around 2.5x larger than xfs's. It also looks like
ext4's read_iomap_begin function has 2 callers, so I think that may
not get automatically inlined whereas xfs's will since
xfs_read_iomap_begin just has 1 caller, though I don't know if that
ends up mattering.
I'm seeing that ext4_iomap_begin is ~1570 bytes vs.
xfs_read_iomap_begin is ~635 bytes. It looks like though
ext4_iomap_begin is a combined read + write begin, whereas xfs's is a
dedicated read-only begin. If we separate ext4_iomap_begin into
dedicated read vs write begins, then I think this gets
ext4_read_iomap_begin down to ~230 bytes.
Would you be able to test if this improves the ext4 performance you're seeing?:
change #1: separate out ext4_iomap_begin into ext4_read/write_iomap_begin
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 4c30dd8dbec7..54cdecbc7539 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3771,8 +3771,46 @@ static int ext4_iomap_alloc(struct inode
*inode, struct ext4_map_blocks *map,
}
-static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
- unsigned flags, struct iomap *iomap, struct iomap *srcmap)
+static int ext4_read_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags, struct iomap *iomap,
+ struct iomap *srcmap)
+{
+ int ret;
+ struct ext4_map_blocks map;
+ u8 blkbits = inode->i_blkbits;
+
+ if ((offset >> blkbits) > EXT4_MAX_LOGICAL_BLOCK)
+ return -EINVAL;
+
+ if (WARN_ON_ONCE(ext4_has_inline_data(inode)))
+ return -ERANGE;
+
+ /*
+ * Calculate the first and last logical blocks respectively.
+ */
+ map.m_lblk = offset >> blkbits;
+ map.m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
+ EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
+
+ ret = ext4_map_blocks(NULL, inode, &map, 0);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * When inline encryption is enabled, sometimes I/O to an encrypted file
+ * has to be broken up to guarantee DUN contiguity. Handle this by
+ * limiting the length of the mapping returned.
+ */
+ map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
+
+ ext4_set_iomap(inode, iomap, &map, offset, length, flags);
+
+ return 0;
+}
+
+static int ext4_write_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags, struct iomap *iomap,
+ struct iomap *srcmap)
{
int ret;
struct ext4_map_blocks map;
@@ -3793,37 +3831,33 @@ static int ext4_iomap_begin(struct inode
*inode, loff_t offset, loff_t length,
EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
orig_mlen = map.m_len;
- if (flags & IOMAP_WRITE) {
+ /*
+ * We check here if the blocks are already allocated, then we
+ * don't need to start a journal txn and we can directly return
+ * the mapping information. This could boost performance
+ * especially in multi-threaded overwrite requests.
+ */
+ if (offset + length <= i_size_read(inode)) {
+ ret = ext4_map_blocks(NULL, inode, &map, 0);
/*
- * We check here if the blocks are already allocated, then we
- * don't need to start a journal txn and we can directly return
- * the mapping information. This could boost performance
- * especially in multi-threaded overwrite requests.
+ * For DAX we convert extents to initialized ones before
+ * copying the data, otherwise we do it after I/O so
+ * there's no need to call into ext4_iomap_alloc().
*/
- if (offset + length <= i_size_read(inode)) {
- ret = ext4_map_blocks(NULL, inode, &map, 0);
+ if ((map.m_flags & EXT4_MAP_MAPPED) ||
+ (!(flags & IOMAP_DAX) &&
+ (map.m_flags & EXT4_MAP_UNWRITTEN))) {
/*
- * For DAX we convert extents to initialized ones before
- * copying the data, otherwise we do it after I/O so
- * there's no need to call into ext4_iomap_alloc().
+ * For atomic writes the entire requested
+ * length should be mapped.
*/
- if ((map.m_flags & EXT4_MAP_MAPPED) ||
- (!(flags & IOMAP_DAX) &&
- (map.m_flags & EXT4_MAP_UNWRITTEN))) {
- /*
- * For atomic writes the entire requested
- * length should be mapped.
- */
- if (ret == orig_mlen ||
- (!(flags & IOMAP_ATOMIC) && ret > 0))
- goto out;
- }
- map.m_len = orig_mlen;
+ if (ret == orig_mlen ||
+ (!(flags & IOMAP_ATOMIC) && ret > 0))
+ goto out;
}
- ret = ext4_iomap_alloc(inode, &map, flags);
- } else {
- ret = ext4_map_blocks(NULL, inode, &map, 0);
+ map.m_len = orig_mlen;
}
+ ret = ext4_iomap_alloc(inode, &map, flags);
if (ret < 0)
return ret;
@@ -3850,6 +3884,16 @@ static int ext4_iomap_begin(struct inode
*inode, loff_t offset, loff_t length,
return 0;
}
+static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+ unsigned flags, struct iomap *iomap, struct iomap *srcmap)
+{
+ if (flags & IOMAP_WRITE)
+ return ext4_write_iomap_begin(inode, offset, length, flags,
+ iomap, srcmap);
+ return ext4_read_iomap_begin(inode, offset, length, flags, iomap,
+ srcmap);
+}
+
int ext4_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
struct iomap *srcmap)
{
change #2: use ext4_read_iomap_next in iomap_dio_rw
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 755fde1baf03..bef2851d5466 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4006,6 +4006,8 @@ static inline void
ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
int ext4_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
struct iomap *srcmap);
+int ext4_read_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
+ struct iomap *srcmap);
int ext4_iomap_next_report(const struct iomap_iter *iter, struct iomap *iomap,
struct iomap *srcmap);
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index bf0c18cf1017..48c6a76a16a1 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -91,7 +91,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb
*iocb, struct iov_iter *to)
return generic_file_read_iter(iocb, to);
}
- ret = iomap_dio_rw(iocb, to, ext4_iomap_next, NULL,
+ ret = iomap_dio_rw(iocb, to, ext4_read_iomap_next, NULL,
IOMAP_DIO_NO_IOMAP_END, NULL, 0);
inode_unlock_shared(inode);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 54cdecbc7539..6e7c77276d58 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3771,9 +3771,9 @@ static int ext4_iomap_alloc(struct inode *inode,
struct ext4_map_blocks *map,
}
-static int ext4_read_iomap_begin(struct inode *inode, loff_t offset,
- loff_t length, unsigned int flags, struct iomap *iomap,
- struct iomap *srcmap)
+static __always_inline int ext4_read_iomap_begin(struct inode *inode,
+ loff_t offset, loff_t length, unsigned int flags,
+ struct iomap *iomap, struct iomap *srcmap)
{
int ret;
struct ext4_map_blocks map;
@@ -3900,6 +3900,12 @@ int ext4_iomap_next(const struct iomap_iter
*iter, struct iomap *iomap,
return iomap_process(iter, iomap, srcmap, ext4_iomap_begin, NULL);
}
+int ext4_read_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
+ struct iomap *srcmap)
+{
+ return iomap_process(iter, iomap, srcmap, ext4_read_iomap_begin, NULL);
+}
+
static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
loff_t length, unsigned int flags,
struct iomap *iomap, struct iomap *srcmap)
The github link for the changes are in [1].
Thanks for your patience with this!
[1] https://github.com/joannekoong/linux/commits/iomap_simple_dio_iomap_next_ext4/