[PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[Qu Wenruo]

Similar to direct writes, if the inode requires data checksum, we should
use a stable buffer for IO, or even for direct reads an unstable buffer
(e.g. the buffer is being modified during the direct read) can lead to
checksum mismatch and even cause read failure.

To address the potential problems of unstable dio read buffers, use
IOMAP_DIO_BOUNCE for dio reads if the inode requires data checksum.

This will bring a small performance drop, around 7% for my benchmark,
which is definitely observable on modern NVME SSDs, but the overhead is
still much smaller compared to data checksum:

 Fio command line:

 # fio --name=randread --filename $mnt/foobar --ioengine=libaio --size=4G \
    --rw=randread --bs=64k --iodepth=64 --runtime=60 --time_based --direct=1

 The VM setup:

 10 vCPUs (host-passthrough), 8G RAM, target storage has cache=none
 option, is backed by a cheap DRAM-less 1T mainstream PCIe4 SSD.

                                 | Bandwidth (MiB/s) |   Diff
 --------------------------------+-------------------+-----------
 Nodatasum, zero-copy (Baseline) | 988               |     0
 Datasum, zero-copy (unpatched)  | 853               | -13.7%
 Datasum, bounce (patched)       | 794               | -19.6%

The difference between data csum bounce and zero-copy is only 7%, which
is much more acceptable than falling back to buffered IO.

Meanwhile just enabling data checksum, even for the fastest CRC32C, the
read performance drops around 13.7%.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/direct-io.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
index ed1779ccb4de..0c9386f1922b 100644
--- a/fs/btrfs/direct-io.c
+++ b/fs/btrfs/direct-io.c
@@ -818,9 +818,23 @@ static ssize_t btrfs_dio_read(struct kiocb *iocb, struct iov_iter *iter,
 			      size_t done_before)
 {
 	struct btrfs_dio_data data = { 0 };
+	unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
+
+	/*
+	 * Data checksum requires stable buffer.
+	 *
+	 * And unlike dio writes, reads are always from one single
+	 * mirror, so there is no need to consider mirror/parity profiles.
+	 */
+	if (!(BTRFS_I(file_inode(iocb->ki_filp))->flags & BTRFS_INODE_NODATASUM)) {
+		/* Bounce will allocate memory, breaking NOWAIT. */
+		if (iocb->ki_flags & IOCB_NOWAIT)
+			return -EAGAIN;
+		dio_flags |= IOMAP_DIO_BOUNCE;
+	}
 
 	return iomap_dio_rw(iocb, iter, &btrfs_dio_iomap_ops, &btrfs_dio_ops,
-			    IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED, &data, done_before);
+			    dio_flags, &data, done_before);
 }
 
 static bool need_stable_write(struct btrfs_inode *inode)
-- 
2.55.0

Re: [PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[Wang Yugui]

Hi,

> Similar to direct writes, if the inode requires data checksum, we should
> use a stable buffer for IO, or even for direct reads an unstable buffer
> (e.g. the buffer is being modified during the direct read) can lead to
> checksum mismatch and even cause read failure.
> 
> To address the potential problems of unstable dio read buffers, use
> IOMAP_DIO_BOUNCE for dio reads if the inode requires data checksum.
> 
> This will bring a small performance drop, around 7% for my benchmark,
> which is definitely observable on modern NVME SSDs, but the overhead is
> still much smaller compared to data checksum:
> 
>  Fio command line:
> 
>  # fio --name=randread --filename $mnt/foobar --ioengine=libaio --size=4G \
>     --rw=randread --bs=64k --iodepth=64 --runtime=60 --time_based --direct=1
> 
>  The VM setup:
> 
>  10 vCPUs (host-passthrough), 8G RAM, target storage has cache=none
>  option, is backed by a cheap DRAM-less 1T mainstream PCIe4 SSD.
> 
>                                  | Bandwidth (MiB/s) |   Diff
>  --------------------------------+-------------------+-----------
>  Nodatasum, zero-copy (Baseline) | 988               |     0
>  Datasum, zero-copy (unpatched)  | 853               | -13.7%
>  Datasum, bounce (patched)       | 794               | -19.6%
> 
> The difference between data csum bounce and zero-copy is only 7%, which
> is much more acceptable than falling back to buffered IO.

for direct write, there is some process of 'falling back to buffered IO' in
btrfs_direct_write().

for direct read, there is no process of 'falling back to buffered IO' ?
Or I missed something?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2026/08/28

Re: [PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[Christoph Hellwig]

On Fri, Aug 28, 2026 at 06:58:16PM +0930, Qu Wenruo wrote:
> Similar to direct writes, if the inode requires data checksum, we should
> use a stable buffer for IO, or even for direct reads an unstable buffer
> (e.g. the buffer is being modified during the direct read) can lead to
> checksum mismatch and even cause read failure.
> 
> To address the potential problems of unstable dio read buffers, use
> IOMAP_DIO_BOUNCE for dio reads if the inode requires data checksum.
> 
> This will bring a small performance drop, around 7% for my benchmark,
> which is definitely observable on modern NVME SSDs, but the overhead is
> still much smaller compared to data checksum:

Please take a look at the "lazy bounce buffering for checksummed reads"
series I just sent v2 of.

For one this removes the read-side IOMAP_DIO_BOUNCE support you are
using here, because it had problems.  The alternative is iomap-based
and better at building large, aligned read bios which matter for HDD.
It also introduces the concept of lazy bouncing where it only bounce
buffers after a checksum failure happens by default, which means you
only pay an overhead for misbehaving applications.  And unlike for
writes where they do exist in real life, I don't know any that modify
the buffer they read into.

Re: [PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[Qu Wenruo]

在 2026/8/31 16:45, Christoph Hellwig 写道:
> On Fri, Aug 28, 2026 at 06:58:16PM +0930, Qu Wenruo wrote:
>> Similar to direct writes, if the inode requires data checksum, we should
>> use a stable buffer for IO, or even for direct reads an unstable buffer
>> (e.g. the buffer is being modified during the direct read) can lead to
>> checksum mismatch and even cause read failure.
>>
>> To address the potential problems of unstable dio read buffers, use
>> IOMAP_DIO_BOUNCE for dio reads if the inode requires data checksum.
>>
>> This will bring a small performance drop, around 7% for my benchmark,
>> which is definitely observable on modern NVME SSDs, but the overhead is
>> still much smaller compared to data checksum:
> 
> Please take a look at the "lazy bounce buffering for checksummed reads"
> series I just sent v2 of.

Thanks for pointing to that series.

However it only shows up in lore, but not in my inbox at all, and I have 
no idea why.

And lore is never a good way to review patches, nor easy to reply.

> 
> For one this removes the read-side IOMAP_DIO_BOUNCE support you are
> using here, because it had problems.  The alternative is iomap-based
> and better at building large, aligned read bios which matter for HDD.
> It also introduces the concept of lazy bouncing where it only bounce
> buffers after a checksum failure happens by default, which means you
> only pay an overhead for misbehaving applications.

In that case I'm afraid btrfs may need some way to integrate the 
checksum handling into iomap.
As the btrfs has a very internal handling for repairing each block.

>  And unlike for
> writes where they do exist in real life, I don't know any that modify
> the buffer they read into.

Yep, and that's why we didn't put too much attention to dio reads until
the dio writes bounce is implemented.

Thanks,
Qu

Re: [PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[Johannes Thumshirn]

On 8/31/26 9:31 AM, Qu Wenruo wrote:
>
> Thanks for pointing to that series.
>
> However it only shows up in lore, but not in my inbox at all, and I 
> have no idea why.
>
> And lore is never a good way to review patches, nor easy to reply. 

JFYI: You can use the "download mbox.gz" link and import into your MUA.

Re: [PATCH] btrfs: use IOMAP_DIO_BOUNCE flag for direct reads

[Christoph Hellwig]

On Mon, Aug 31, 2026 at 05:01:21PM +0930, Qu Wenruo wrote:
> However it only shows up in lore, but not in my inbox at all, and I have no
> idea why.
> 
> And lore is never a good way to review patches, nor easy to reply.

I got it brack through all the usual lists, so this must be some kind
of filtering on your side.

> > For one this removes the read-side IOMAP_DIO_BOUNCE support you are
> > using here, because it had problems.  The alternative is iomap-based
> > and better at building large, aligned read bios which matter for HDD.
> > It also introduces the concept of lazy bouncing where it only bounce
> > buffers after a checksum failure happens by default, which means you
> > only pay an overhead for misbehaving applications.
> 
> In that case I'm afraid btrfs may need some way to integrate the checksum
> handling into iomap.
> As the btrfs has a very internal handling for repairing each block.

The only iomap path is the bounce buffering, the checksum handling is
still left to the file systems.  The code will probably need some
minor tweaks for btrfs, like passing in a bio_set to use for the clones,
but it shouldn't be too bad.