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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 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