* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 5:05 [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO Qu Wenruo
@ 2026-05-25 7:05 ` Christoph Hellwig
2026-05-25 7:16 ` Qu Wenruo
2026-05-25 9:14 ` Qu Wenruo
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-25 7:05 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, May 25, 2026 at 02:35:33PM +0930, Qu Wenruo wrote:
> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
> struct btrfs_dio_data data = { 0 };
> + const u64 data_profile = btrfs_data_alloc_profile(inode->root->fs_info) &
> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
> + unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
> +
> + /*
> + * Userspace may modify the buffer while DIO is in flight. With
> + * data checksumming this would produce a checksum that doesn't
> + * match the persisted data; with duplicated profiles the mirrors
> + * would diverge. Bounce in those cases so writeback sees stable
> + * content.
> + */
> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
Should the 0 here be BTRFS_RAID_SINGLE for better documentation?
Also the old code only did this for !BTRFS_INODE_NODATASUM and not
raid, right?
> + dio_flags |= IOMAP_DIO_BOUNCE;
Splitting the data_profile check into a helper would improve readabilty
here. Maybe even as a prep patch before the actual change, then a
patch to add IOMAP_DIO_BOUNCE, and the extension to raid as another
stand-alone well documented patch?
Also while you touch this, you should probably also check the
bdev_stable_writes similar to XFS (probably yet another patch).
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 7:05 ` Christoph Hellwig
@ 2026-05-25 7:16 ` Qu Wenruo
2026-05-25 7:23 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2026-05-25 7:16 UTC (permalink / raw)
To: Christoph Hellwig, Qu Wenruo; +Cc: linux-btrfs
在 2026/5/25 16:35, Christoph Hellwig 写道:
> On Mon, May 25, 2026 at 02:35:33PM +0930, Qu Wenruo wrote:
>> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
>> struct btrfs_dio_data data = { 0 };
>> + const u64 data_profile = btrfs_data_alloc_profile(inode->root->fs_info) &
>> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
>> + unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
>> +
>> + /*
>> + * Userspace may modify the buffer while DIO is in flight. With
>> + * data checksumming this would produce a checksum that doesn't
>> + * match the persisted data; with duplicated profiles the mirrors
>> + * would diverge. Bounce in those cases so writeback sees stable
>> + * content.
>> + */
>> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
>> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
>
> Should the 0 here be BTRFS_RAID_SINGLE for better documentation?
That BTRFS_RAID_SINGLE is a special enum for iterating btrfs_raid_array[].
Meanwhile for bit flags we do not use that enum index, even if the
values match.
>
> Also the old code only did this for !BTRFS_INODE_NODATASUM and not
> raid, right?
The old code also does the fallback to buffered for raid too.
That's done in commit 7c2830f00c3e ("btrfs: fallback to buffered IO if
the data profile has duplication").
>
>
>> + dio_flags |= IOMAP_DIO_BOUNCE;
>
> Splitting the data_profile check into a helper would improve readabilty
> here.
Sure, will go a dedicated helper for that.
> Maybe even as a prep patch before the actual change, then a
> patch to add IOMAP_DIO_BOUNCE, and the extension to raid as another
> stand-alone well documented patch?
>
> Also while you touch this, you should probably also check the
> bdev_stable_writes similar to XFS (probably yet another patch).
For btrfs stable writes == data csum so far. But we do not have any bdev
stable writes checks, thus it will be a pretty huge change if we want to
take per-dev stable writes into consideration.
Thanks,
Qu
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 7:16 ` Qu Wenruo
@ 2026-05-25 7:23 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-25 7:23 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Christoph Hellwig, Qu Wenruo, linux-btrfs
On Mon, May 25, 2026 at 04:46:16PM +0930, Qu Wenruo wrote:
> > > + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
> > > + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
> >
> > Should the 0 here be BTRFS_RAID_SINGLE for better documentation?
>
> That BTRFS_RAID_SINGLE is a special enum for iterating btrfs_raid_array[].
>
> Meanwhile for bit flags we do not use that enum index, even if the values
> match.
Ok.
> >
> > Also the old code only did this for !BTRFS_INODE_NODATASUM and not
> > raid, right?
>
> The old code also does the fallback to buffered for raid too.
>
> That's done in commit 7c2830f00c3e ("btrfs: fallback to buffered IO if the
> data profile has duplication").
Ah, right.
> > Also while you touch this, you should probably also check the
> > bdev_stable_writes similar to XFS (probably yet another patch).
>
> For btrfs stable writes == data csum so far. But we do not have any bdev
> stable writes checks, thus it will be a pretty huge change if we want to
> take per-dev stable writes into consideration.
I tink you need to. Various network storage protocols like iSCSI, NVMe
over fabrics or DRBD, as well as T10 data protection for SCSI and NVMe
require stable pages, so you have to take them into account. My
approach would be to do it globally if any such device is part of the
pool.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 5:05 [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO Qu Wenruo
2026-05-25 7:05 ` Christoph Hellwig
@ 2026-05-25 9:14 ` Qu Wenruo
2026-05-26 0:38 ` Qu Wenruo
2026-05-26 6:38 ` Christoph Hellwig
2026-05-25 23:55 ` Wang Yugui
2026-05-26 17:59 ` Boris Burkov
3 siblings, 2 replies; 10+ messages in thread
From: Qu Wenruo @ 2026-05-25 9:14 UTC (permalink / raw)
To: linux-btrfs, Christoph Hellwig, Filipe Manana
在 2026/5/25 14:35, Qu Wenruo 写道:
> Previously btrfs forces direct writes to fall back to buffered ones if the
> inode has data checksum or the profile has duplication.
>
> That fallback is to avoid the content being modified that the final
> content may mismatch with the checksum or the other mirrors.
>
> That brings a pretty huge performance cost, which already caused some
> concern at that time.
>
> But later upstream commit c9d114846b38 ("iomap: add a flag to bounce
> buffer direct I/O") introduced a new method by copying the content into
> new pages, and do all the operations based on the newly allocated pages.
>
> So let btrfs to utilize the new flag for direct writes if we require
> stable folios.
>
> There is a quick benchmark, using the following fio setup:
>
> fio --name=randwrite --filename $mnt/foobar --ioengine=libaio --size=4G \
> --rw=randwrite --iodepth=64 --runtime=60 --time_based --direct=1 \
> --bs=$blocksize
>
> Unit is MiB/s.
>
> Blocksize | Zero-copy (*) | Buffered | Bounce
> -----------+---------------+----------+-----------
> 4K | 35.1 | 17.1 | 33.8
> 64K | 522 | 251 | 492
>
> *: This is done by reverting the commit 968f19c5b1b7 ("btrfs: always
> fallback to buffered write if the inode requires checksum")
>
> Although with page bouncing the performance is only around 95% of
> true-zero copy, it's still almost double the performance of buffered
> fallback.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Rework the comment in btrfs_dio_write()
> ---
> fs/btrfs/direct-io.c | 45 ++++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> index 57167d56dc72..173fe065fc38 100644
> --- a/fs/btrfs/direct-io.c
> +++ b/fs/btrfs/direct-io.c
> @@ -768,10 +768,25 @@ static ssize_t btrfs_dio_read(struct kiocb *iocb, struct iov_iter *iter,
> static struct iomap_dio *btrfs_dio_write(struct kiocb *iocb, struct iov_iter *iter,
> size_t done_before)
> {
> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
> struct btrfs_dio_data data = { 0 };
> + const u64 data_profile = btrfs_data_alloc_profile(inode->root->fs_info) &
> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
> + unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
> +
> + /*
> + * Userspace may modify the buffer while DIO is in flight. With
> + * data checksumming this would produce a checksum that doesn't
> + * match the persisted data; with duplicated profiles the mirrors
> + * would diverge. Bounce in those cases so writeback sees stable
> + * content.
> + */
> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
> + dio_flags |= IOMAP_DIO_BOUNCE;
Unfortunately this will deadlock at generic/647.
Currently btrfs avoids the deadlock by disabling page fault for the
@from iov_iter.
But that iov_iter->nofault is not respected during
bio_iov_iter_bounce_write() -> copy_from_iter(), thus we will hit a
deadlock at exactly the situation described in the comment just before
btrfs_dio_write() call.
I tried to check how XFS handles this, and XFS seems to go a completely
different way using different flags for xfs_ilock().
And it doesn't look like it's even possible to make copy_from_iter() to
properly respect the nofault flag.
I'm wondering if there is any good idea to handle such situation.
Or we should add some extra checks inside btrfs? E.g. if we found out
that the folio we're reading belongs to a direct write, instead of
waiting for the OE to finish, returning -EFAULT?
The blocked call traces looks like the following:
task:mmap-rw-fault state:D stack:0 pid:1157 tgid:1157 ppid:981
task_flags:0x440100 flags:0x00080000
Call Trace:
<TASK>
__schedule+0x408/0x1870
? __blk_flush_plug+0xea/0x140
schedule+0x27/0xd0
btrfs_start_ordered_extent_nowriteback+0x16d/0x200 [btrfs
8565e31bdc0fd7f3ea5ed5d7c25129d026ba3f05]
? wake_up_bit+0xc0/0xc0
lock_extents_for_read.constprop.0+0x1d6/0x2a0 [btrfs
8565e31bdc0fd7f3ea5ed5d7c25129d026ba3f05]
btrfs_readahead+0x91/0x1c0 [btrfs
8565e31bdc0fd7f3ea5ed5d7c25129d026ba3f05]
read_pages+0x72/0x210
page_cache_ra_order+0x232/0x390
filemap_fault+0x630/0x1220
__do_fault+0x2e/0x180
do_fault+0x300/0x570
? __pte_offset_map+0x1b/0x100
__handle_mm_fault+0x94e/0xf50
? btrfs_clear_extent_bit_changeset+0x316/0x770 [btrfs
8565e31bdc0fd7f3ea5ed5d7c25129d026ba3f05]
handle_mm_fault+0xf2/0x300
do_user_addr_fault+0x15b/0x680
exc_page_fault+0x82/0x1c0
asm_exc_page_fault+0x26/0x30
RIP: 0010:_copy_from_iter+0x9f/0x620
Code: 8b 6d 08 e8 33 ab 0c 00 84 c0 75 60 48 b8 00 f0 ff ff ff 7f 00
00 4b 8d 34 2e 48 39 c6 48 0f 47 f0 0f 01 cb 48 89 d9 4c 89 e7 <f3> a4
0f 1f 00 0f 01 ca 49 89 dd 49 29 cd 48 03 4d 18 4c 01 6d 08
RSP: 0018:ffffd3644303f778 EFLAGS: 00050287
RAX: 00007ffffffff000 RBX: 0000000000001000 RCX: 0000000000001000
RDX: ffff8cff8f578000 RSI: 00007f601a7b4000 RDI: ffff8cff85565000
RBP: ffffd3644303fb58 R08: 0000000000000000 R09: 000000000000010f
R10: ffffffffa37aee20 R11: ffff8d00fffd73c0 R12: ffff8cff85565000
R13: 0000000000000000 R14: 00007f601a7b4000 R15: fffffbdb04155940
? _copy_from_iter+0x7d/0x620
? alloc_pages_mpol+0xb6/0x170
bio_iov_iter_bounce+0x1a9/0x2c0
iomap_dio_bio_iter+0x20e/0x620
__iomap_dio_rw+0x4e5/0x8f0
btrfs_direct_write+0x282/0x4b0 [btrfs
8565e31bdc0fd7f3ea5ed5d7c25129d026ba3f05]
btrfs_do_write_iter+0x19a/0x220 [btrfs
8565e31bdc0fd7f3ea5ed5d7c25129d026ba3f05]
vfs_write+0x253/0x480
__x64_sys_pwrite64+0x98/0xd0
do_syscall_64+0xe1/0x7c0
? vm_mmap_pgoff+0x15b/0x200
? ksys_mmap_pgoff+0x168/0x200
? do_syscall_64+0xe1/0x7c0
? switch_fpu_return+0x52/0xe0
? do_syscall_64+0x26e/0x7c0
? do_syscall_64+0x26e/0x7c0
? __x64_sys_openat+0x61/0xa0
? do_syscall_64+0xe1/0x7c0
? __x64_sys_close+0x3d/0x80
? do_syscall_64+0xe1/0x7c0
? do_syscall_64+0x98/0x7c0
? exc_page_fault+0x82/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x7f601a49318e
RSP: 002b:00007fffbfd38e80 EFLAGS: 00000202 ORIG_RAX: 0000000000000012
RAX: ffffffffffffffda RBX: 0000000000001000 RCX: 00007f601a49318e
RDX: 0000000000001000 RSI: 00007f601a7b4000 RDI: 0000000000000003
RBP: 00007fffbfd38e90 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
R13: 0000000000000003 R14: 0000000000000000 R15: 0000559f60b73d58
</TASK>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 9:14 ` Qu Wenruo
@ 2026-05-26 0:38 ` Qu Wenruo
2026-05-26 6:38 ` Christoph Hellwig
1 sibling, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2026-05-26 0:38 UTC (permalink / raw)
To: linux-btrfs, Christoph Hellwig, Filipe Manana
在 2026/5/25 18:44, Qu Wenruo 写道:
>
>
> 在 2026/5/25 14:35, Qu Wenruo 写道:
>> Previously btrfs forces direct writes to fall back to buffered ones if
>> the
>> inode has data checksum or the profile has duplication.
>>
>> That fallback is to avoid the content being modified that the final
>> content may mismatch with the checksum or the other mirrors.
>>
>> That brings a pretty huge performance cost, which already caused some
>> concern at that time.
>>
>> But later upstream commit c9d114846b38 ("iomap: add a flag to bounce
>> buffer direct I/O") introduced a new method by copying the content into
>> new pages, and do all the operations based on the newly allocated pages.
>>
>> So let btrfs to utilize the new flag for direct writes if we require
>> stable folios.
>>
>> There is a quick benchmark, using the following fio setup:
>>
>> fio --name=randwrite --filename $mnt/foobar --ioengine=libaio --
>> size=4G \
>> --rw=randwrite --iodepth=64 --runtime=60 --time_based --direct=1 \
>> --bs=$blocksize
>>
>> Unit is MiB/s.
>>
>> Blocksize | Zero-copy (*) | Buffered | Bounce
>> -----------+---------------+----------+-----------
>> 4K | 35.1 | 17.1 | 33.8
>> 64K | 522 | 251 | 492
>>
>> *: This is done by reverting the commit 968f19c5b1b7 ("btrfs: always
>> fallback to buffered write if the inode requires checksum")
>>
>> Although with page bouncing the performance is only around 95% of
>> true-zero copy, it's still almost double the performance of buffered
>> fallback.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> Changelog:
>> v2:
>> - Rework the comment in btrfs_dio_write()
>> ---
>> fs/btrfs/direct-io.c | 45 ++++++++++++++++----------------------------
>> 1 file changed, 16 insertions(+), 29 deletions(-)
>>
>> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
>> index 57167d56dc72..173fe065fc38 100644
>> --- a/fs/btrfs/direct-io.c
>> +++ b/fs/btrfs/direct-io.c
>> @@ -768,10 +768,25 @@ static ssize_t btrfs_dio_read(struct kiocb
>> *iocb, struct iov_iter *iter,
>> static struct iomap_dio *btrfs_dio_write(struct kiocb *iocb, struct
>> iov_iter *iter,
>> size_t done_before)
>> {
>> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
>> struct btrfs_dio_data data = { 0 };
>> + const u64 data_profile = btrfs_data_alloc_profile(inode->root-
>> >fs_info) &
>> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
>> + unsigned int dio_flags = IOMAP_DIO_PARTIAL |
>> IOMAP_DIO_FSBLOCK_ALIGNED;
>> +
>> + /*
>> + * Userspace may modify the buffer while DIO is in flight. With
>> + * data checksumming this would produce a checksum that doesn't
>> + * match the persisted data; with duplicated profiles the mirrors
>> + * would diverge. Bounce in those cases so writeback sees stable
>> + * content.
>> + */
>> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
>> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
>> + dio_flags |= IOMAP_DIO_BOUNCE;
>
> Unfortunately this will deadlock at generic/647.
>
> Currently btrfs avoids the deadlock by disabling page fault for the
> @from iov_iter.
>
> But that iov_iter->nofault is not respected during
> bio_iov_iter_bounce_write() -> copy_from_iter(), thus we will hit a
> deadlock at exactly the situation described in the comment just before
> btrfs_dio_write() call.
It turns out that we can simply disable page faulting during
btrfs_dio_write().
But this will come with new problems.
With page fault disabled, we will not hit the deadlock, but we will hit
another case where we have already allocated a new OE, then page
bouncing failed, resulting no real dio bio being submitted.
Then we go into btrfs_dio_iomap_end() which will mark the allocated OE
as error.
Later even if we fall back to buffered write, we got to
fdatawait_range(), and since we got an OE marked as error, it returned
error, failing the buffered write fallback.
So this new BOUNCE flag indeed exposed several btrfs error handling
problems.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 9:14 ` Qu Wenruo
2026-05-26 0:38 ` Qu Wenruo
@ 2026-05-26 6:38 ` Christoph Hellwig
1 sibling, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-05-26 6:38 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Christoph Hellwig, Filipe Manana
On Mon, May 25, 2026 at 06:44:48PM +0930, Qu Wenruo wrote:
> Currently btrfs avoids the deadlock by disabling page fault for the @from
> iov_iter.
>
> But that iov_iter->nofault is not respected during
> bio_iov_iter_bounce_write() -> copy_from_iter(), thus we will hit a deadlock
> at exactly the situation described in the comment just before
> btrfs_dio_write() call.
Yeah. We could try to propagate it, but ...
> I tried to check how XFS handles this, and XFS seems to go a completely
> different way using different flags for xfs_ilock().
Note that the flag is just messaging. The important part is that XFS
uses a different lock for protecting the internal state in the inode
like the extent mapping (xfs_inode.i_lock) vs the highlevel VFS lock
protecting I/O (inode.i_rwsem), which is really important to avoid
all kinds of locking pitfalls. I thought btrfs was doing the same,
but maybe this is some other lock you are seeing the recursion on?
Sorting out the locking is good way to avoid these problems and also
integrate better with common VFS helpers and/or iomap.
> Or we should add some extra checks inside btrfs? E.g. if we found out that
> the folio we're reading belongs to a direct write, instead of waiting for
> the OE to finish, returning -EFAULT?
Oh, this isn't actually a lock, but the ordered_extent wait? My memory
is a little fuzzy, but I think this is the same problem with the
ordered_extent semantics that is also causing so many other problems:
btrfs completes the pagecache level writeback (that is clearing the
folio writeback bit) before the ordered_extent has completed and thus
the metadata has been recorded. Which breaks the assumptions of all
the common VFS code, and requires reads into the pagecache to lock
the extent. If you'd manage to fix the writeback code to only clear
the folio writeback bit once all metadata is recorded in the right place
all this would go away. I.e. call btrfs_folio_clear_writeback only
from the finish ordered extent context and not directly from
end_bbio_data_write. I tried this a long time ago and was running
into problems, but you cleaned up a lot of the mess in this area
since, so it might be easier now.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 5:05 [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO Qu Wenruo
2026-05-25 7:05 ` Christoph Hellwig
2026-05-25 9:14 ` Qu Wenruo
@ 2026-05-25 23:55 ` Wang Yugui
2026-05-26 17:59 ` Boris Burkov
3 siblings, 0 replies; 10+ messages in thread
From: Wang Yugui @ 2026-05-25 23:55 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
Hi,
> Previously btrfs forces direct writes to fall back to buffered ones if the
> inode has data checksum or the profile has duplication.
>
> That fallback is to avoid the content being modified that the final
> content may mismatch with the checksum or the other mirrors.
>
> That brings a pretty huge performance cost, which already caused some
> concern at that time.
>
> But later upstream commit c9d114846b38 ("iomap: add a flag to bounce
> buffer direct I/O") introduced a new method by copying the content into
> new pages, and do all the operations based on the newly allocated pages.
>
> So let btrfs to utilize the new flag for direct writes if we require
> stable folios.
>
> There is a quick benchmark, using the following fio setup:
>
> fio --name=randwrite --filename $mnt/foobar --ioengine=libaio --size=4G \
> --rw=randwrite --iodepth=64 --runtime=60 --time_based --direct=1 \
> --bs=$blocksize
>
> Unit is MiB/s.
>
> Blocksize | Zero-copy (*) | Buffered | Bounce
> -----------+---------------+----------+-----------
> 4K | 35.1 | 17.1 | 33.8
> 64K | 522 | 251 | 492
>
> *: This is done by reverting the commit 968f19c5b1b7 ("btrfs: always
> fallback to buffered write if the inode requires checksum")
>
> Although with page bouncing the performance is only around 95% of
> true-zero copy, it's still almost double the performance of buffered
> fallback.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Rework the comment in btrfs_dio_write()
> ---
> fs/btrfs/direct-io.c | 45 ++++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> index 57167d56dc72..173fe065fc38 100644
> --- a/fs/btrfs/direct-io.c
> +++ b/fs/btrfs/direct-io.c
> @@ -768,10 +768,25 @@ static ssize_t btrfs_dio_read(struct kiocb *iocb, struct iov_iter *iter,
> static struct iomap_dio *btrfs_dio_write(struct kiocb *iocb, struct iov_iter *iter,
> size_t done_before)
> {
> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
> struct btrfs_dio_data data = { 0 };
> + const u64 data_profile = btrfs_data_alloc_profile(inode->root->fs_info) &
> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
> + unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
> +
> + /*
> + * Userspace may modify the buffer while DIO is in flight. With
> + * data checksumming this would produce a checksum that doesn't
> + * match the persisted data; with duplicated profiles the mirrors
> + * would diverge. Bounce in those cases so writeback sees stable
> + * content.
> + */
> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
> + 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 ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
> @@ -800,8 +815,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> ssize_t ret;
> unsigned int ilock_flags = 0;
> struct iomap_dio *dio;
> - const u64 data_profile = btrfs_data_alloc_profile(fs_info) &
> - BTRFS_BLOCK_GROUP_PROFILE_MASK;
>
> if (iocb->ki_flags & IOCB_NOWAIT)
> ilock_flags |= BTRFS_ILOCK_TRY;
> @@ -815,16 +828,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode) && IS_NOSEC(inode))
> ilock_flags |= BTRFS_ILOCK_SHARED;
>
> - /*
> - * If our data profile has duplication (either extra mirrors or RAID56),
> - * we can not trust the direct IO buffer, the content may change during
> - * writeback and cause different contents written to different mirrors.
> - *
> - * Thus only RAID0 and SINGLE can go true zero-copy direct IO.
> - */
> - if (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0)
> - goto buffered;
> -
> relock:
> ret = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
> if (ret < 0)
> @@ -865,22 +868,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
> goto buffered;
> }
> - /*
> - * We can't control the folios being passed in, applications can write
> - * to them while a direct IO write is in progress. This means the
> - * content might change after we calculated the data checksum.
> - * Therefore we can end up storing a checksum that doesn't match the
> - * persisted data.
> - *
> - * To be extra safe and avoid false data checksum mismatch, if the
> - * inode requires data checksum, just fallback to buffered IO.
> - * For buffered IO we have full control of page cache and can ensure
> - * no one is modifying the content during writeback.
> - */
> - if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> - btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
> - goto buffered;
> - }
>
> /*
> * The iov_iter can be mapped to the same file range we are writing to.
Do we need this too just like xfs?
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fa82def46e39..64eae7417242 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3843,7 +3843,7 @@ const struct file_operations btrfs_file_operations = {
#endif
.remap_file_range = btrfs_remap_file_range,
.uring_cmd = btrfs_uring_cmd,
- .fop_flags = FOP_BUFFER_RASYNC | FOP_BUFFER_WASYNC,
+ .fop_flags = FOP_BUFFER_RASYNC | FOP_BUFFER_WASYNC | FOP_DONTCACHE,
};
int btrfs_fdatawrite_range(struct btrfs_inode *inode, loff_t start, loff_t end)
Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2026/05/26
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-25 5:05 [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO Qu Wenruo
` (2 preceding siblings ...)
2026-05-25 23:55 ` Wang Yugui
@ 2026-05-26 17:59 ` Boris Burkov
2026-05-26 21:42 ` Qu Wenruo
3 siblings, 1 reply; 10+ messages in thread
From: Boris Burkov @ 2026-05-26 17:59 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, May 25, 2026 at 02:35:33PM +0930, Qu Wenruo wrote:
> Previously btrfs forces direct writes to fall back to buffered ones if the
> inode has data checksum or the profile has duplication.
>
> That fallback is to avoid the content being modified that the final
> content may mismatch with the checksum or the other mirrors.
>
> That brings a pretty huge performance cost, which already caused some
> concern at that time.
>
> But later upstream commit c9d114846b38 ("iomap: add a flag to bounce
> buffer direct I/O") introduced a new method by copying the content into
> new pages, and do all the operations based on the newly allocated pages.
>
> So let btrfs to utilize the new flag for direct writes if we require
> stable folios.
>
> There is a quick benchmark, using the following fio setup:
>
> fio --name=randwrite --filename $mnt/foobar --ioengine=libaio --size=4G \
> --rw=randwrite --iodepth=64 --runtime=60 --time_based --direct=1 \
> --bs=$blocksize
>
> Unit is MiB/s.
>
> Blocksize | Zero-copy (*) | Buffered | Bounce
> -----------+---------------+----------+-----------
> 4K | 35.1 | 17.1 | 33.8
> 64K | 522 | 251 | 492
This is really exciting!
Dumb question:
Do you have a sense (or reference to previous discussion?) that would
explain why bounce buffering so much faster?
Thinking out loud about possible overhead:
- copying overhead : bounce vs buffered both need to copy the user data
- memory allocation overhead: they both need a folio (to copy into or to
dirty). It might already be present for buffered.
- btrfs architecture overhead: They both should be doing all the same
cow-ing/extent_map manipulation.
- synch-ness: The buffered fallback immediately writes back and waits as
far as I can tell, so it shouldn't be some kind of "writeback doesn't
get triggered when we want"
- balance_dirty_pages: we do call into it in the buffered fallback so we
could be made to wait. But I would sort of not expect that to happen
on the test you are running, unless you are doing lots of other
dirtying at the same time? Since each fallback pass does trigger
writeback so it shouldn't build up too much.
- libaio/iodepth true async: buffered fallback is synchronous per go
while maybe bounce can be more properly async? Haven't thought through
this too carefully.
- generic page cache overhead: we just have to do a lot more work for
the same operations. managing folio state, xarray, locks, balance
dirty pages, writeback xarray, etc etc
I would guess that it is the "generic overhead" in the benchmark.
Curious if you have any clearer thoughts on it.
Thanks,
Boris
>
> *: This is done by reverting the commit 968f19c5b1b7 ("btrfs: always
> fallback to buffered write if the inode requires checksum")
>
> Although with page bouncing the performance is only around 95% of
> true-zero copy, it's still almost double the performance of buffered
> fallback.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Rework the comment in btrfs_dio_write()
> ---
> fs/btrfs/direct-io.c | 45 ++++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> index 57167d56dc72..173fe065fc38 100644
> --- a/fs/btrfs/direct-io.c
> +++ b/fs/btrfs/direct-io.c
> @@ -768,10 +768,25 @@ static ssize_t btrfs_dio_read(struct kiocb *iocb, struct iov_iter *iter,
> static struct iomap_dio *btrfs_dio_write(struct kiocb *iocb, struct iov_iter *iter,
> size_t done_before)
> {
> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
> struct btrfs_dio_data data = { 0 };
> + const u64 data_profile = btrfs_data_alloc_profile(inode->root->fs_info) &
> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
> + unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
> +
> + /*
> + * Userspace may modify the buffer while DIO is in flight. With
> + * data checksumming this would produce a checksum that doesn't
> + * match the persisted data; with duplicated profiles the mirrors
> + * would diverge. Bounce in those cases so writeback sees stable
> + * content.
> + */
> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
> + 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 ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
> @@ -800,8 +815,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> ssize_t ret;
> unsigned int ilock_flags = 0;
> struct iomap_dio *dio;
> - const u64 data_profile = btrfs_data_alloc_profile(fs_info) &
> - BTRFS_BLOCK_GROUP_PROFILE_MASK;
>
> if (iocb->ki_flags & IOCB_NOWAIT)
> ilock_flags |= BTRFS_ILOCK_TRY;
> @@ -815,16 +828,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode) && IS_NOSEC(inode))
> ilock_flags |= BTRFS_ILOCK_SHARED;
>
> - /*
> - * If our data profile has duplication (either extra mirrors or RAID56),
> - * we can not trust the direct IO buffer, the content may change during
> - * writeback and cause different contents written to different mirrors.
> - *
> - * Thus only RAID0 and SINGLE can go true zero-copy direct IO.
> - */
> - if (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0)
> - goto buffered;
> -
> relock:
> ret = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
> if (ret < 0)
> @@ -865,22 +868,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
> goto buffered;
> }
> - /*
> - * We can't control the folios being passed in, applications can write
> - * to them while a direct IO write is in progress. This means the
> - * content might change after we calculated the data checksum.
> - * Therefore we can end up storing a checksum that doesn't match the
> - * persisted data.
> - *
> - * To be extra safe and avoid false data checksum mismatch, if the
> - * inode requires data checksum, just fallback to buffered IO.
> - * For buffered IO we have full control of page cache and can ensure
> - * no one is modifying the content during writeback.
> - */
> - if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> - btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
> - goto buffered;
> - }
>
> /*
> * The iov_iter can be mapped to the same file range we are writing to.
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO
2026-05-26 17:59 ` Boris Burkov
@ 2026-05-26 21:42 ` Qu Wenruo
0 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2026-05-26 21:42 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs
在 2026/5/27 03:29, Boris Burkov 写道:
> On Mon, May 25, 2026 at 02:35:33PM +0930, Qu Wenruo wrote:
>> Previously btrfs forces direct writes to fall back to buffered ones if the
>> inode has data checksum or the profile has duplication.
>>
>> That fallback is to avoid the content being modified that the final
>> content may mismatch with the checksum or the other mirrors.
>>
>> That brings a pretty huge performance cost, which already caused some
>> concern at that time.
>>
>> But later upstream commit c9d114846b38 ("iomap: add a flag to bounce
>> buffer direct I/O") introduced a new method by copying the content into
>> new pages, and do all the operations based on the newly allocated pages.
>>
>> So let btrfs to utilize the new flag for direct writes if we require
>> stable folios.
>>
>> There is a quick benchmark, using the following fio setup:
>>
>> fio --name=randwrite --filename $mnt/foobar --ioengine=libaio --size=4G \
>> --rw=randwrite --iodepth=64 --runtime=60 --time_based --direct=1 \
>> --bs=$blocksize
>>
>> Unit is MiB/s.
>>
>> Blocksize | Zero-copy (*) | Buffered | Bounce
>> -----------+---------------+----------+-----------
>> 4K | 35.1 | 17.1 | 33.8
>> 64K | 522 | 251 | 492
>
> This is really exciting!
>
> Dumb question:
> Do you have a sense (or reference to previous discussion?) that would
> explain why bounce buffering so much faster?
>
> Thinking out loud about possible overhead:
>
> - copying overhead : bounce vs buffered both need to copy the user data
> - memory allocation overhead: they both need a folio (to copy into or to
> dirty). It might already be present for buffered.
> - btrfs architecture overhead: They both should be doing all the same
> cow-ing/extent_map manipulation.
> - synch-ness: The buffered fallback immediately writes back and waits as
> far as I can tell, so it shouldn't be some kind of "writeback doesn't
> get triggered when we want"
> - balance_dirty_pages: we do call into it in the buffered fallback so we
> could be made to wait. But I would sort of not expect that to happen
> on the test you are running, unless you are doing lots of other
> dirtying at the same time? Since each fallback pass does trigger
> writeback so it shouldn't build up too much.
> - libaio/iodepth true async: buffered fallback is synchronous per go
> while maybe bounce can be more properly async? Haven't thought through
> this too carefully.
> - generic page cache overhead: we just have to do a lot more work for
> the same operations. managing folio state, xarray, locks, balance
> dirty pages, writeback xarray, etc etc
>
> I would guess that it is the "generic overhead" in the benchmark.
> Curious if you have any clearer thoughts on it.
I guess it's mostly due to the page cache synchronization.
But please do not be too excited for now, this new bounce behavior
exposes several problems, mostly related to the page fault during write.
It exposed a bug that even without this patch, for nodatasum mount
option that generic/362 (*) can still fail due to failed page fault-in
and handling of later buffered fallback.
*: Needs this patch to reliably trigger the failure, or it will only
fail with newly formatted TEST_DEV:
https://lore.kernel.org/linux-btrfs/20260526070055.60193-1-wqu@suse.com/T/#u
Thanks,
Qu
>
> Thanks,
> Boris
>
>>
>> *: This is done by reverting the commit 968f19c5b1b7 ("btrfs: always
>> fallback to buffered write if the inode requires checksum")
>>
>> Although with page bouncing the performance is only around 95% of
>> true-zero copy, it's still almost double the performance of buffered
>> fallback.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> Changelog:
>> v2:
>> - Rework the comment in btrfs_dio_write()
>> ---
>> fs/btrfs/direct-io.c | 45 ++++++++++++++++----------------------------
>> 1 file changed, 16 insertions(+), 29 deletions(-)
>>
>> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
>> index 57167d56dc72..173fe065fc38 100644
>> --- a/fs/btrfs/direct-io.c
>> +++ b/fs/btrfs/direct-io.c
>> @@ -768,10 +768,25 @@ static ssize_t btrfs_dio_read(struct kiocb *iocb, struct iov_iter *iter,
>> static struct iomap_dio *btrfs_dio_write(struct kiocb *iocb, struct iov_iter *iter,
>> size_t done_before)
>> {
>> + struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
>> struct btrfs_dio_data data = { 0 };
>> + const u64 data_profile = btrfs_data_alloc_profile(inode->root->fs_info) &
>> + BTRFS_BLOCK_GROUP_PROFILE_MASK;
>> + unsigned int dio_flags = IOMAP_DIO_PARTIAL | IOMAP_DIO_FSBLOCK_ALIGNED;
>> +
>> + /*
>> + * Userspace may modify the buffer while DIO is in flight. With
>> + * data checksumming this would produce a checksum that doesn't
>> + * match the persisted data; with duplicated profiles the mirrors
>> + * would diverge. Bounce in those cases so writeback sees stable
>> + * content.
>> + */
>> + if (!(inode->flags & BTRFS_INODE_NODATASUM) ||
>> + (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0))
>> + 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 ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
>> @@ -800,8 +815,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>> ssize_t ret;
>> unsigned int ilock_flags = 0;
>> struct iomap_dio *dio;
>> - const u64 data_profile = btrfs_data_alloc_profile(fs_info) &
>> - BTRFS_BLOCK_GROUP_PROFILE_MASK;
>>
>> if (iocb->ki_flags & IOCB_NOWAIT)
>> ilock_flags |= BTRFS_ILOCK_TRY;
>> @@ -815,16 +828,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>> if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode) && IS_NOSEC(inode))
>> ilock_flags |= BTRFS_ILOCK_SHARED;
>>
>> - /*
>> - * If our data profile has duplication (either extra mirrors or RAID56),
>> - * we can not trust the direct IO buffer, the content may change during
>> - * writeback and cause different contents written to different mirrors.
>> - *
>> - * Thus only RAID0 and SINGLE can go true zero-copy direct IO.
>> - */
>> - if (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0)
>> - goto buffered;
>> -
>> relock:
>> ret = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
>> if (ret < 0)
>> @@ -865,22 +868,6 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>> btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
>> goto buffered;
>> }
>> - /*
>> - * We can't control the folios being passed in, applications can write
>> - * to them while a direct IO write is in progress. This means the
>> - * content might change after we calculated the data checksum.
>> - * Therefore we can end up storing a checksum that doesn't match the
>> - * persisted data.
>> - *
>> - * To be extra safe and avoid false data checksum mismatch, if the
>> - * inode requires data checksum, just fallback to buffered IO.
>> - * For buffered IO we have full control of page cache and can ensure
>> - * no one is modifying the content during writeback.
>> - */
>> - if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
>> - btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
>> - goto buffered;
>> - }
>>
>> /*
>> * The iov_iter can be mapped to the same file range we are writing to.
>> --
>> 2.54.0
>>
^ permalink raw reply [flat|nested] 10+ messages in thread