Re: [PATCH 2/2] btrfs: simplify regions mark and keep start unchanged in err handling

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

在 2024/10/25 17:24, iamhswang@gmail.com 写道:
> From: Haisu Wang <haisuwang@tencent.com>
>
> Simplify the regions mark by using cur_alloc_size only to present
> the reserved but may failed to alloced extent. Remove the ram_size
> as well since it is always consistent to the cur_alloc_size in the
> context. Advanced the start mark in normal path until extent succeed
> alloced and keep the start unchanged in error handling path.
>
> PASSed the fstest generic/475 test for a hundred times with quota
> enabled. And a modified generic/475 test by removing the sleep time
> for a hundred times. About one tenth of the tests do enter the error
> handling path due to fail to reserve extent.
>

Although this patch is already merged into for-next, it looks like the
next patch will again change the error handling, mostly render the this
one useless:

https://lore.kernel.org/linux-btrfs/2a0925f0264daf90741ed0a7ba7ed4b4888cf778.1728725060.git.wqu@suse.com/

The newer patch will change the error handling to a simpler one, so
instead of 3 regions, there will be only 2.

There will be no change needed from your side, I will update my patches
to solve the conflicts, just in case if you find the error handling is
different in the future.

Thanks,
Qu

> Suggested-by: Qu Wenruo <wqu@suse.com>
> Signed-off-by: Haisu Wang <haisuwang@tencent.com>
> ---
>   fs/btrfs/inode.c | 32 ++++++++++++++------------------
>   1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3646734a7e59..7e67a6d50be2 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1359,7 +1359,6 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   	u64 alloc_hint = 0;
>   	u64 orig_start = start;
>   	u64 num_bytes;
> -	unsigned long ram_size;
>   	u64 cur_alloc_size = 0;
>   	u64 min_alloc_size;
>   	u64 blocksize = fs_info->sectorsize;
> @@ -1367,7 +1366,6 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   	struct extent_map *em;
>   	unsigned clear_bits;
>   	unsigned long page_ops;
> -	bool extent_reserved = false;
>   	int ret = 0;
>
>   	if (btrfs_is_free_space_inode(inode)) {
> @@ -1421,8 +1419,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   		struct btrfs_ordered_extent *ordered;
>   		struct btrfs_file_extent file_extent;
>
> -		cur_alloc_size = num_bytes;
> -		ret = btrfs_reserve_extent(root, cur_alloc_size, cur_alloc_size,
> +		ret = btrfs_reserve_extent(root, num_bytes, num_bytes,
>   					   min_alloc_size, 0, alloc_hint,
>   					   &ins, 1, 1);
>   		if (ret == -EAGAIN) {
> @@ -1453,9 +1450,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   		if (ret < 0)
>   			goto out_unlock;
>   		cur_alloc_size = ins.offset;
> -		extent_reserved = true;
>
> -		ram_size = ins.offset;
>   		file_extent.disk_bytenr = ins.objectid;
>   		file_extent.disk_num_bytes = ins.offset;
>   		file_extent.num_bytes = ins.offset;
> @@ -1463,14 +1458,14 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   		file_extent.offset = 0;
>   		file_extent.compression = BTRFS_COMPRESS_NONE;
>
> -		lock_extent(&inode->io_tree, start, start + ram_size - 1,
> +		lock_extent(&inode->io_tree, start, start + cur_alloc_size - 1,
>   			    &cached);
>
>   		em = btrfs_create_io_em(inode, start, &file_extent,
>   					BTRFS_ORDERED_REGULAR);
>   		if (IS_ERR(em)) {
>   			unlock_extent(&inode->io_tree, start,
> -				      start + ram_size - 1, &cached);
> +				      start + cur_alloc_size - 1, &cached);
>   			ret = PTR_ERR(em);
>   			goto out_reserve;
>   		}
> @@ -1480,7 +1475,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   						     1 << BTRFS_ORDERED_REGULAR);
>   		if (IS_ERR(ordered)) {
>   			unlock_extent(&inode->io_tree, start,
> -				      start + ram_size - 1, &cached);
> +				      start + cur_alloc_size - 1, &cached);
>   			ret = PTR_ERR(ordered);
>   			goto out_drop_extent_cache;
>   		}
> @@ -1501,7 +1496,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   			 */
>   			if (ret)
>   				btrfs_drop_extent_map_range(inode, start,
> -							    start + ram_size - 1,
> +							    start + cur_alloc_size - 1,
>   							    false);
>   		}
>   		btrfs_put_ordered_extent(ordered);
> @@ -1519,7 +1514,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   		page_ops = (keep_locked ? 0 : PAGE_UNLOCK);
>   		page_ops |= PAGE_SET_ORDERED;
>
> -		extent_clear_unlock_delalloc(inode, start, start + ram_size - 1,
> +		extent_clear_unlock_delalloc(inode, start, start + cur_alloc_size - 1,
>   					     locked_folio, &cached,
>   					     EXTENT_LOCKED | EXTENT_DELALLOC,
>   					     page_ops);
> @@ -1529,7 +1524,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   			num_bytes -= cur_alloc_size;
>   		alloc_hint = ins.objectid + ins.offset;
>   		start += cur_alloc_size;
> -		extent_reserved = false;
> +		cur_alloc_size = 0;
>
>   		/*
>   		 * btrfs_reloc_clone_csums() error, since start is increased
> @@ -1545,7 +1540,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   	return ret;
>
>   out_drop_extent_cache:
> -	btrfs_drop_extent_map_range(inode, start, start + ram_size - 1, false);
> +	btrfs_drop_extent_map_range(inode, start, start + cur_alloc_size - 1, false);
>   out_reserve:
>   	btrfs_dec_block_group_reservations(fs_info, ins.objectid);
>   	btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 1);
> @@ -1599,13 +1594,12 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   	 * to decrement again the data space_info's bytes_may_use counter,
>   	 * therefore we do not pass it the flag EXTENT_CLEAR_DATA_RESV.
>   	 */
> -	if (extent_reserved) {
> +	if (cur_alloc_size) {
>   		extent_clear_unlock_delalloc(inode, start,
>   					     start + cur_alloc_size - 1,
>   					     locked_folio, &cached, clear_bits,
>   					     page_ops);
>   		btrfs_qgroup_free_data(inode, NULL, start, cur_alloc_size, NULL);
> -		start += cur_alloc_size;
>   	}
>
>   	/*
> @@ -1614,11 +1608,13 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>   	 * space_info's bytes_may_use counter, reserved in
>   	 * btrfs_check_data_free_space().
>   	 */
> -	if (start < end) {
> +	if (start + cur_alloc_size < end) {
>   		clear_bits |= EXTENT_CLEAR_DATA_RESV;
> -		extent_clear_unlock_delalloc(inode, start, end, locked_folio,
> +		extent_clear_unlock_delalloc(inode, start + cur_alloc_size,
> +					     end, locked_folio,
>   					     &cached, clear_bits, page_ops);
> -		btrfs_qgroup_free_data(inode, NULL, start, end - start + 1, NULL);
> +		btrfs_qgroup_free_data(inode, NULL, start + cur_alloc_size,
> +				end - start - cur_alloc_size + 1, NULL);
>   	}
>   	return ret;
>   }