Re: [PATCH v2] btrfs: use IOMAP_DIO_BOUNCE flag instead of falling back to buffered IO

[Qu Wenruo <wqu@suse.com>]

在 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.