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

[Qu Wenruo <wqu@suse.com>]

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

[...]

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

Thanks,
Qu