Re: [PATCH 12/29] btrfs: btrfs_write_marked_extents rename werr to ret err to ret2

Anand Jain
2024-04-16T02:40:18Z

On 3/20/24 01:53, Josef Bacik wrote:
> On Tue, Mar 19, 2024 at 08:25:20PM +0530, Anand Jain wrote:
>> Rename the function's local variable werr to ret and err to ret2, then
>> move ret2 to the local variable of the while loop. Drop the initialization
>> there since it's immediately assigned below.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> fs/btrfs/transaction.c | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index feffff91c6fe..167893457b58 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -1119,8 +1119,7 @@ int btrfs_end_transaction_throttle(struct btrfs_trans_handle *trans)
>> int btrfs_write_marked_extents(struct btrfs_fs_info *fs_info,
>> struct extent_io_tree *dirty_pages, int mark)
>> {
>> - int err = 0;
>> - int werr = 0;
>> + int ret = 0;
>> struct address_space *mapping = fs_info->btree_inode->i_mapping;
>> struct extent_state *cached_state = NULL;
>> u64 start = 0;
>> @@ -1128,9 +1127,10 @@ int btrfs_write_marked_extents(struct btrfs_fs_info *fs_info,
>>
>> while (find_first_extent_bit(dirty_pages, start, &start, &end,
>> mark, &cached_state)) {
>> + int ret2;
>> bool wait_writeback = false;
>>
>> - err = convert_extent_bit(dirty_pages, start, end,
>> + ret2 = convert_extent_bit(dirty_pages, start, end,
>> EXTENT_NEED_WAIT,
>> mark, &cached_state);
>> /*
>> @@ -1146,22 +1146,22 @@ int btrfs_write_marked_extents(struct btrfs_fs_info *fs_info,
>> * We cleanup any entries left in the io tree when committing
>> * the transaction (through extent_io_tree_release()).
>> */
>> - if (err == -ENOMEM) {
>> - err = 0;
>> + if (ret2 == -ENOMEM) {
>> + ret2 = 0;
>> wait_writeback = true;
>> }
>> - if (!err)
>> - err = filemap_fdatawrite_range(mapping, start, end);
>> - if (err)
>> - werr = err;
>> + if (!ret2)
>> + ret2 = filemap_fdatawrite_range(mapping, start, end);
>> + if (ret2)
>> + ret = ret2;
>> else if (wait_writeback)
>> - werr = filemap_fdatawait_range(mapping, start, end);
>> + ret = filemap_fdatawait_range(mapping, start, end);
>
> Ok so this is a correct conversion, but we'll lose "ret" here. Can you follow
> up with a different series to fix this? I think we just say
>

> free_extent_state(cached_state);
> if (ret)
> break;
>

Sure. Checked the function's stack will propagate the error here back
to the parent functions.

Thanks, Anand

> otherwise this patch looks fine, you can add
>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
>
> Thanks,
>
> Josef