Re: [PATCH 8/9] btrfs: make btrfs_submit_compressed_read() to determine stripe boundary at bio allocation time

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

On 2021/6/11 下午3:49, Johannes Thumshirn wrote:
> On 11/06/2021 03:32, Qu Wenruo wrote:
>> +		/* Allocate new bio if not allocated or already submitted */
>> +		if (!bio) {
>> +			bio = alloc_compressed_bio(cb, cur_disk_bytenr,
>> +				bio_op | write_flags,
>> +				end_compressed_bio_write,
>> +				&next_stripe_start);
>> +			if (IS_ERR(bio)) {
>> +				ret = errno_to_blk_status(PTR_ERR(bio));
>> +				bio = NULL;
>> +				goto finish_cb;
>> +			}
>> +		}
>> +		/*
>> +		 * We should never reach next_stripe_start, as if we reach the
>> +		 * boundary we will submit the bio immediately.
>> +		 */
>> +		ASSERT(cur_disk_bytenr != next_stripe_start);
>> +
>> +		/*
>> +		 * We have various limit on the real read size:
>> +		 * - stripe boundary
>> +		 * - page boundary
>> +		 * - compressed length boundary
>> +		 */
>> +		real_size = min_t(u64, U32_MAX,
>> +				  next_stripe_start - cur_disk_bytenr);
>> +		real_size = min_t(u64, real_size,
>> +				  PAGE_SIZE - offset_in_page(offset));
>> +		real_size = min_t(u64, real_size,
>> +				  compressed_len - offset);
>> +		ASSERT(IS_ALIGNED(real_size, fs_info->sectorsize));
>>
>> -	/* create and submit bios for the compressed pages */
>> -	bytes_left = compressed_len;
>> -	for (pg_index = 0; pg_index < cb->nr_pages; pg_index++) {
>> -		int submit = 0;
>> -		int len;
>> +		added = bio_add_page(bio, page, real_size,
>> +				     offset_in_page(offset));
>> +		/*
>> +		 * Maximum compressed extent size is 128K, we should never
>> +		 * reach bio size limit.
>> +		 */
>> +		ASSERT(added == real_size);
>>
>> -		page = compressed_pages[pg_index];
>> -		page->mapping = inode->vfs_inode.i_mapping;
>> -		if (bio->bi_iter.bi_size)
>> -			submit = btrfs_bio_fits_in_stripe(page, PAGE_SIZE, bio,
>> -							  0);
>> +		cur_disk_bytenr += added;
>>
>> -		if (pg_index == 0 && use_append)
>> -			len = bio_add_zone_append_page(bio, page, PAGE_SIZE, 0);
>> -		else
>> -			len = bio_add_page(bio, page, PAGE_SIZE, 0);
>
> I think you still need to distinguish between normal write and zone append here,
> as you adding pages to an already created bio.

Oh, my bad, forgot to handle the zoned append differently.

> Adding one page to an empty bio
> will always succeed but when adding more than one page to a zone append bio, you
> have to take the device's maximum zone append limit into account, as zone append
> bios can't be split. This is also the reason why we do the device
> lookup/bio_set_dev() for the zone append bios, so bio_add_zone_append_page() can
> look at the device's limitations when adding the pages.

Did you mean that for the bio_add_zone_append_page(), it may return less
bytes than we expected?
Even if our compressed write is ensured to be smaller than 128K?

Thanks,
Qu