Re: [PATCH v2] btrfs: report filemap_fdata<write|wait>_range error

[Anand Jain <anand.jain@oracle.com>]

On 4/18/24 14:30, Qu Wenruo wrote:
> 
> 
> 在 2024/4/18 15:44, Anand Jain 写道:
>> In the function btrfs_write_marked_extents() and in 
>> __btrfs_wait_marked_extents()
>> return the actual error if when filemap_fdata<write|wait>_range() fails.
>>
>> Suggested-by: Josef Bacik <josef@toxicpanda.com>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> Looks fine for the patch, although I have a small question.
> 
> If we failed to write some metadata extents, we break out, meaning there
> would be dirty metadata still hanging there.
> 
> Would it be a problem?
> 

I had the exact same question, but what I discovered is that the
error originated here will lead to our handle errors and to readonly
state. So it should be fine. Further, if submit layer write/wait is
failing there is nothing much we can do as of now. However, in the
long run we probably should provide an option to fail-safe.

Thanks, Anand

> Thanks,
> Qu
>> ---
>> v2: add __btrfs_wait_marked_extents() as well.
>>
>>   fs/btrfs/transaction.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index 3e3bcc5f64c6..8c3b3cda1390 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -1156,6 +1156,8 @@ int btrfs_write_marked_extents(struct 
>> btrfs_fs_info *fs_info,
>>           else if (wait_writeback)
>>               werr = filemap_fdatawait_range(mapping, start, end);
>>           free_extent_state(cached_state);
>> +        if (werr)
>> +            break;
>>           cached_state = NULL;
>>           cond_resched();
>>           start = end + 1;
>> @@ -1198,6 +1200,8 @@ static int __btrfs_wait_marked_extents(struct 
>> btrfs_fs_info *fs_info,
>>           if (err)
>>               werr = err;
>>           free_extent_state(cached_state);
>> +        if (werr)
>> +            break;
>>           cached_state = NULL;
>>           cond_resched();
>>           start = end + 1;