Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups

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

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



On 2018/11/9 下午6:21, Filipe Manana wrote:
> On Fri, Nov 9, 2018 at 12:27 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>>
>> On 2018/11/8 下午10:48, Filipe Manana wrote:
>>> On Thu, Nov 8, 2018 at 2:37 PM Filipe Manana <fdmanana@kernel.org> wrote:
>>>>
>>>> On Thu, Nov 8, 2018 at 2:35 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 2018/11/8 下午9:17, fdmanana@kernel.org wrote:
>>>>>> From: Filipe Manana <fdmanana@suse.com>
>>>>>>
>>>>>> When creating a block group we don't need to set the log for full commit
>>>>>> if the new block group is not used for data. Logged items can only point
>>>>>> to logical addresses of data block groups (through file extent items) so
>>>>>> there is no need to for the next fsync to fallback to a transaction commit
>>>>>> if the new block group is for metadata.
>>>>>
>>>>> Is it possible for the log tree blocks to be allocated in that new block
>>>>> group?
>>>>
>>>> Yes.
>>>
>>> Now I realize what might be your concern, and this would cause trouble.
>>> Surprised this didn't trigger any problem and I had this (together
>>> with other changes) running tests for some weeks already.
>>
>> Maybe it's related metadata chunk pre-allocation so it will be super
>> hard to hit in normal case, or some extent allocation policy preventing
>> us from allocating tree block of newly created bg.
> 
> No, I don't think we have such kind of policy, would have noticed it
> over the years if we did have one.
> Metadata chunk allocation just happens much less frequently for
> workloads the tests exercised.

Then smaller fs along with 4K nodesize would help, but I'm still not
sure if we could hit such case.

Thanks,
Qu

> 
>>
>> Thanks,
>> Qu
>>
>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> Qu
>>>>>
>>>>>>
>>>>>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>>>>>> ---
>>>>>>  fs/btrfs/extent-tree.c | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>>>>> index 577878324799..588fbd1606fb 100644
>>>>>> --- a/fs/btrfs/extent-tree.c
>>>>>> +++ b/fs/btrfs/extent-tree.c
>>>>>> @@ -10112,7 +10112,8 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used,
>>>>>>       struct btrfs_block_group_cache *cache;
>>>>>>       int ret;
>>>>>>
>>>>>> -     btrfs_set_log_full_commit(fs_info, trans);
>>>>>> +     if (type & BTRFS_BLOCK_GROUP_DATA)
>>>>>> +             btrfs_set_log_full_commit(fs_info, trans);
>>>>>>
>>>>>>       cache = btrfs_create_block_group_cache(fs_info, chunk_offset, size);
>>>>>>       if (!cache)
>>>>>>
>>>>>
>>


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