Re: [PATCH v2] btrfs: Limit size of bios submitted from writeback

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

在 2026/4/27 18:33, Jan Kara 写道:
> On Thu 23-04-26 19:24:02, Qu Wenruo wrote:
>>
>>
>> 在 2026/4/23 19:00, Jan Kara 写道:
>>> Currently btrfs_writepages() just accumulates as large bio as possible
>>> (within writeback_control constraints) and then submits it. This can
>>> however lead to significant latency in writeback IO submission (I have
>>> observed tens of miliseconds) because the submitted bio easily has over
>>> hundred of megabytes. Consequently this leads to IO pipeline stalls and
>>> reduced throughput.
>>>
>>> At the same time beyond certain size submitting so large bio provides
>>> diminishing returns because the bio is split by the block layer
>>> immediately anyway. So compute (estimate of) bio size beyond which we
>>> are unlikely to improve performance and just submit the bio for
>>> writeback once we accumulate that much to keep the IO pipeline busy.
>>> This improves writeback throughput for sequential writes by about 15% on
>>> the test machine I was using.
>>>
>>> Signed-off-by: Jan Kara <jack@suse.cz>
>>
>> Looks great!
>>
>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>
>> Just one minor question inlined below.
> 
> Thanks!
> 
>>> +int btrfs_init_writeback_bio_size(struct btrfs_fs_info *fs_info)
>>> +{
>>> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>>> +	struct btrfs_device *device;
>>> +	u32 writeback_bio_size = fs_info->sectorsize;
>>> +
>>> +	mutex_lock(&fs_devices->device_list_mutex);
>>> +	/*
>>> +	 * Let's take maximum over optimal request sizes for all devices. For
>>> +	 * RAID profiles writeback will submit stripe (64k) sized bios anyway
>>> +	 * so our value doesn't matter and for simple profiles this is a good
>>> +	 * approximation of sensible IO chunking.
>>> +	 */
>>> +	list_for_each_entry(device, &fs_devices->devices, dev_list) {
>>> +		struct request_queue *queue;
>>> +		unsigned int io_opt;
>>> +
>>> +		queue = bdev_get_queue(device->bdev);
>>> +		io_opt = queue_io_opt(queue) ? :
>>> +				queue_max_sectors(queue) << SECTOR_SHIFT;
>>> +		writeback_bio_size = max(writeback_bio_size, io_opt);
>>> +	}
>>> +	mutex_unlock(&fs_devices->device_list_mutex);
>>> +
>>> +	fs_info->writeback_bio_size = writeback_bio_size;
>>
>> With this simplified version of optimal io size detection, do we want to
>> hook dev add/removal/replace to update the calculation?
>>
>> I guess in the real world, the added/removed/replaced disks should have all
>> the same performance parameter for server usages, so no difference there.
>>
>> And for personal/pro users, I doubt if the original performance problem is
>> even noticeable for most end users.
>>
>> So overall I'm fine either way.
> 
> Yeah, at this point I'm not sure the complexity is worth it. Normally added
> disks have very similar parameters as existing ones, also even if they are
> somewhat different, it will cost you a few percent of writeback speed at
> worst which doesn't seem too bad and it will "fix" itself on next mount.

Thanks, we're on the same page.

Now the patch is pushed to for-next branch, with one typo "miliseconds" 
fixed.

Thanks,
Qu

> 
> 								Honza