* [PATCH] Btrfs: do not set log for full commit when creating non-data block groups @ 2018-11-08 13:17 fdmanana 2018-11-08 14:33 ` Qu Wenruo 0 siblings, 1 reply; 9+ messages in thread From: fdmanana @ 2018-11-08 13:17 UTC (permalink / raw) To: linux-btrfs 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. 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) -- 2.11.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-08 13:17 [PATCH] Btrfs: do not set log for full commit when creating non-data block groups fdmanana @ 2018-11-08 14:33 ` Qu Wenruo 2018-11-08 14:37 ` Filipe Manana 0 siblings, 1 reply; 9+ messages in thread From: Qu Wenruo @ 2018-11-08 14:33 UTC (permalink / raw) To: fdmanana, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 1284 bytes --] 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? 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-08 14:33 ` Qu Wenruo @ 2018-11-08 14:37 ` Filipe Manana 2018-11-08 14:48 ` Filipe Manana 0 siblings, 1 reply; 9+ messages in thread From: Filipe Manana @ 2018-11-08 14:37 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs 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. > > 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) > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-08 14:37 ` Filipe Manana @ 2018-11-08 14:48 ` Filipe Manana 2018-11-09 0:27 ` Qu Wenruo 2018-11-13 14:31 ` David Sterba 0 siblings, 2 replies; 9+ messages in thread From: Filipe Manana @ 2018-11-08 14:48 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs 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. > > > > > 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) > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-08 14:48 ` Filipe Manana @ 2018-11-09 0:27 ` Qu Wenruo 2018-11-09 10:21 ` Filipe Manana 2018-11-13 14:31 ` David Sterba 1 sibling, 1 reply; 9+ messages in thread From: Qu Wenruo @ 2018-11-09 0:27 UTC (permalink / raw) To: Filipe Manana; +Cc: linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 2095 bytes --] 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. 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-09 0:27 ` Qu Wenruo @ 2018-11-09 10:21 ` Filipe Manana 2018-11-09 12:02 ` Qu Wenruo 0 siblings, 1 reply; 9+ messages in thread From: Filipe Manana @ 2018-11-09 10:21 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs 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) > >>>> > >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-09 10:21 ` Filipe Manana @ 2018-11-09 12:02 ` Qu Wenruo 0 siblings, 0 replies; 9+ messages in thread From: Qu Wenruo @ 2018-11-09 12:02 UTC (permalink / raw) To: Filipe Manana; +Cc: linux-btrfs [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-08 14:48 ` Filipe Manana 2018-11-09 0:27 ` Qu Wenruo @ 2018-11-13 14:31 ` David Sterba 2018-11-13 14:44 ` Filipe Manana 1 sibling, 1 reply; 9+ messages in thread From: David Sterba @ 2018-11-13 14:31 UTC (permalink / raw) To: Filipe Manana; +Cc: Qu Wenruo, linux-btrfs On Thu, Nov 08, 2018 at 02:48:29PM +0000, 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. Is this patch ok for for-next or does it need more work? Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Btrfs: do not set log for full commit when creating non-data block groups 2018-11-13 14:31 ` David Sterba @ 2018-11-13 14:44 ` Filipe Manana 0 siblings, 0 replies; 9+ messages in thread From: Filipe Manana @ 2018-11-13 14:44 UTC (permalink / raw) To: dsterba, Qu Wenruo, linux-btrfs On Tue, Nov 13, 2018 at 2:31 PM David Sterba <dsterba@suse.cz> wrote: > > On Thu, Nov 08, 2018 at 02:48:29PM +0000, 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. > > Is this patch ok for for-next or does it need more work? Thanks. Nop, it's no good (despite not triggering problems initially), due to Qu's first question. So just drop it and forget it. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-13 14:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-08 13:17 [PATCH] Btrfs: do not set log for full commit when creating non-data block groups fdmanana 2018-11-08 14:33 ` Qu Wenruo 2018-11-08 14:37 ` Filipe Manana 2018-11-08 14:48 ` Filipe Manana 2018-11-09 0:27 ` Qu Wenruo 2018-11-09 10:21 ` Filipe Manana 2018-11-09 12:02 ` Qu Wenruo 2018-11-13 14:31 ` David Sterba 2018-11-13 14:44 ` Filipe Manana
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.