Re: [PATCH 2/7] btrfs: unset reloc control if we fail to recover

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

[-- Attachment #1.1: Type: text/plain, Size: 1733 bytes --]



On 2020/3/3 上午9:03, Josef Bacik wrote:
> On 3/2/20 7:58 PM, Qu Wenruo wrote:
>>
>>
>> On 2020/3/3 上午2:47, Josef Bacik wrote:
>>> If we fail to load an fs root, or fail to start a transaction we can
>>> bail without unsetting the reloc control, which leads to problems later
>>> when we free the reloc control but still have it attached to the file
>>> system.
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> ---
>>>   fs/btrfs/relocation.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>>> index 507361e99316..173fc7628235 100644
>>> --- a/fs/btrfs/relocation.c
>>> +++ b/fs/btrfs/relocation.c
>>> @@ -4678,6 +4678,7 @@ int btrfs_recover_relocation(struct btrfs_root
>>> *root)
>>>           fs_root = read_fs_root(fs_info, reloc_root->root_key.offset);
>>>           if (IS_ERR(fs_root)) {
>>>               err = PTR_ERR(fs_root);
>>> +            unset_reloc_control(rc);
>>>               list_add_tail(&reloc_root->root_list, &reloc_roots);
>>>               goto out_free;
>>>           }
>>
>>
>> Shouldn't the unset_reloc_control() also get called for all related
>> errors after set_reloc_control()?
>>
>> That includes btrfs_join_transaction() (the one after
>> set_reloc_contrl(), which looks missing), the read_fs_root() and the
>> commit transaction error you're addressing.
>>
> 
> It's already doing unset in the join_transaction right after calling
> set.  These are the only two missing paths.  Thanks,
> 
> Josef

My bad.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]