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

[Boris Burkov <boris@bur.io>]

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
>