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

[Filipe Manana <fdmanana@kernel.org>]

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.

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