| Message ID | 7dbf1733c917f37122c630d392622d70021cdbdb.1576195673.git.dennis@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: async discard support | expand |
On Fri, Dec 13, 2019 at 04:22:27PM -0800, Dennis Zhou wrote: > As mentioned earlier, discarding data can be done either by issuing an > explicit discard or implicitly by reusing the LBA. Metadata chunks see > much more frequent reuse due to well it being metadata. So instead of > explicitly discarding metadata blocks, just leave them be and let the > latter implicit discarding be done for them. > > Signed-off-by: Dennis Zhou <dennis@kernel.org> > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/block-group.h | 6 ++++++ > fs/btrfs/discard.c | 11 +++++++++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h > index 601e1d217e22..ee8441439a56 100644 > --- a/fs/btrfs/block-group.h > +++ b/fs/btrfs/block-group.h > @@ -182,6 +182,12 @@ static inline u64 btrfs_block_group_end(struct btrfs_block_group *block_group) > return (block_group->start + block_group->length); > } > > +static inline bool btrfs_is_block_group_data( > + struct btrfs_block_group *block_group) > +{ > + return (block_group->flags & BTRFS_BLOCK_GROUP_DATA); What happens for mixed data and metadata block groups? As this is a special mode that will likely lead to fragmented block groups I think that async discard won't be able to help much. I'd suggest to make the test explicit only for the separate block group types and comment that mixed bg's are not supported.
On Mon, Dec 30, 2019 at 06:39:54PM +0100, David Sterba wrote: > On Fri, Dec 13, 2019 at 04:22:27PM -0800, Dennis Zhou wrote: > > As mentioned earlier, discarding data can be done either by issuing an > > explicit discard or implicitly by reusing the LBA. Metadata chunks see > > much more frequent reuse due to well it being metadata. So instead of > > explicitly discarding metadata blocks, just leave them be and let the > > latter implicit discarding be done for them. > > > > Signed-off-by: Dennis Zhou <dennis@kernel.org> > > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > > --- > > fs/btrfs/block-group.h | 6 ++++++ > > fs/btrfs/discard.c | 11 +++++++++-- > > 2 files changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h > > index 601e1d217e22..ee8441439a56 100644 > > --- a/fs/btrfs/block-group.h > > +++ b/fs/btrfs/block-group.h > > @@ -182,6 +182,12 @@ static inline u64 btrfs_block_group_end(struct btrfs_block_group *block_group) > > return (block_group->start + block_group->length); > > } > > > > +static inline bool btrfs_is_block_group_data( > > + struct btrfs_block_group *block_group) > > +{ > > + return (block_group->flags & BTRFS_BLOCK_GROUP_DATA); > > What happens for mixed data and metadata block groups? As this is a > special mode that will likely lead to fragmented block groups I think > that async discard won't be able to help much. > > I'd suggest to make the test explicit only for the separate block group > types and comment that mixed bg's are not supported. Yeah it probably wouldn't do the system much good. I've renamed the test to btrfs_is_block_group_data_only() and added a comment in the async discard header patch to state that mixed block_groups aren't supported.
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h index 601e1d217e22..ee8441439a56 100644 --- a/fs/btrfs/block-group.h +++ b/fs/btrfs/block-group.h @@ -182,6 +182,12 @@ static inline u64 btrfs_block_group_end(struct btrfs_block_group *block_group) return (block_group->start + block_group->length); } +static inline bool btrfs_is_block_group_data( + struct btrfs_block_group *block_group) +{ + return (block_group->flags & BTRFS_BLOCK_GROUP_DATA); +} + #ifdef CONFIG_BTRFS_DEBUG static inline int btrfs_should_fragment_free_space( struct btrfs_block_group *block_group) diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c index 884dffd28596..55ad357e65f3 100644 --- a/fs/btrfs/discard.c +++ b/fs/btrfs/discard.c @@ -53,6 +53,9 @@ static void __add_to_discard_list(struct btrfs_discard_ctl *discard_ctl, static void add_to_discard_list(struct btrfs_discard_ctl *discard_ctl, struct btrfs_block_group *block_group) { + if (!btrfs_is_block_group_data(block_group)) + return; + spin_lock(&discard_ctl->lock); __add_to_discard_list(discard_ctl, block_group); @@ -168,7 +171,10 @@ static struct btrfs_block_group *peek_discard_list( if (block_group && now > block_group->discard_eligible_time) { if (block_group->discard_index == BTRFS_DISCARD_INDEX_UNUSED && block_group->used != 0) { - __add_to_discard_list(discard_ctl, block_group); + if (btrfs_is_block_group_data(block_group)) + __add_to_discard_list(discard_ctl, block_group); + else + list_del_init(&block_group->discard_list); goto again; } if (block_group->discard_state == BTRFS_DISCARD_RESET_CURSOR) { @@ -508,7 +514,8 @@ void btrfs_discard_update_discardable(struct btrfs_block_group *block_group, s64 bytes_delta; if (!block_group || - !btrfs_test_opt(block_group->fs_info, DISCARD_ASYNC)) + !btrfs_test_opt(block_group->fs_info, DISCARD_ASYNC) || + !btrfs_is_block_group_data(block_group)) return; discard_ctl = &block_group->fs_info->discard_ctl;