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

[Qu Wenruo <wqu@suse.com>]

在 2024/4/18 16:48, Anand Jain 写道:
> 
> 
> 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.

Yep, I know we would mark the fs error, but would things like 
close_ctree() report other warnings as we still have dirty pages for 
metadata inode?

Thanks,
Qu

> 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;
>