| Message ID | 20200120140918.15647-10-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make pinned extents tracking per-transaction | expand |
On 1/20/20 9:09 AM, Nikolay Borisov wrote: > In preparation to making pinned extents per-transaction ensure that > log such extents are always excluded from caching. To achieve this in > addition to marking them via btrfs_pin_extent_for_log_replay they also > need to be marked with btrfs_add_excluded_extent to prevent log tree > extent buffer being loaded by the free space caching thread. That's > required since log treeblocks are not recorded in the extent tree, hence > they always look free. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent-tree.c | 8 ++++++++ > fs/btrfs/tree-log.c | 2 +- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 7dcf9217a622..d680f2ac336b 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2634,6 +2634,8 @@ int btrfs_pin_extent_for_log_replay(struct btrfs_trans_handle *trans, > struct btrfs_block_group *cache; > int ret; > > + btrfs_add_excluded_extent(trans->fs_info, bytenr, num_bytes); > + > cache = btrfs_lookup_block_group(trans->fs_info, bytenr); > if (!cache) > return -EINVAL; > @@ -2920,6 +2922,12 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) > mutex_unlock(&fs_info->unused_bg_unpin_mutex); > break; > } > + if (test_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags)) { > + clear_extent_bits(&fs_info->freed_extents[0], start, > + end, EXTENT_UPTODATE); > + clear_extent_bits(&fs_info->freed_extents[1], start, > + end, EXTENT_UPTODATE); > + } > > if (btrfs_test_opt(fs_info, DISCARD_SYNC)) > ret = btrfs_discard_extent(fs_info, start, > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index b535d409a728..f89de24838d5 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -2994,7 +2994,7 @@ static inline void btrfs_remove_log_ctx(struct btrfs_root *root, > mutex_unlock(&root->log_mutex); > } > > -/* > +/* nit: this part needs to be dropped. Other than that Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On Mon, Jan 20, 2020 at 04:09:16PM +0200, Nikolay Borisov wrote: > In preparation to making pinned extents per-transaction ensure that > log such extents are always excluded from caching. To achieve this in > addition to marking them via btrfs_pin_extent_for_log_replay they also > need to be marked with btrfs_add_excluded_extent to prevent log tree > extent buffer being loaded by the free space caching thread. That's > required since log treeblocks are not recorded in the extent tree, hence > they always look free. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent-tree.c | 8 ++++++++ > fs/btrfs/tree-log.c | 2 +- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 7dcf9217a622..d680f2ac336b 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2634,6 +2634,8 @@ int btrfs_pin_extent_for_log_replay(struct btrfs_trans_handle *trans, > struct btrfs_block_group *cache; > int ret; > > + btrfs_add_excluded_extent(trans->fs_info, bytenr, num_bytes); Again lack of error handling, I thought that untangling and cleaning up the extent pinning was motivated to actually handle the errors. > + > cache = btrfs_lookup_block_group(trans->fs_info, bytenr); > if (!cache) > return -EINVAL; > @@ -2920,6 +2922,12 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) > mutex_unlock(&fs_info->unused_bg_unpin_mutex); > break; > } > + if (test_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags)) { > + clear_extent_bits(&fs_info->freed_extents[0], start, > + end, EXTENT_UPTODATE); > + clear_extent_bits(&fs_info->freed_extents[1], start, > + end, EXTENT_UPTODATE); > + } > > if (btrfs_test_opt(fs_info, DISCARD_SYNC)) > ret = btrfs_discard_extent(fs_info, start, > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index b535d409a728..f89de24838d5 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -2994,7 +2994,7 @@ static inline void btrfs_remove_log_ctx(struct btrfs_root *root, > mutex_unlock(&root->log_mutex); > } > > -/* > +/* > * Invoked in log mutex context, or be sure there is no other task which > * can access the list. > */ > -- > 2.17.1
On 30.01.20 г. 15:53 ч., David Sterba wrote: > On Mon, Jan 20, 2020 at 04:09:16PM +0200, Nikolay Borisov wrote: >> In preparation to making pinned extents per-transaction ensure that >> log such extents are always excluded from caching. To achieve this in >> addition to marking them via btrfs_pin_extent_for_log_replay they also >> need to be marked with btrfs_add_excluded_extent to prevent log tree >> extent buffer being loaded by the free space caching thread. That's >> required since log treeblocks are not recorded in the extent tree, hence >> they always look free. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> --- >> fs/btrfs/extent-tree.c | 8 ++++++++ >> fs/btrfs/tree-log.c | 2 +- >> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 7dcf9217a622..d680f2ac336b 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -2634,6 +2634,8 @@ int btrfs_pin_extent_for_log_replay(struct btrfs_trans_handle *trans, >> struct btrfs_block_group *cache; >> int ret; >> >> + btrfs_add_excluded_extent(trans->fs_info, bytenr, num_bytes); > > Again lack of error handling, I thought that untangling and cleaning up > the extent pinning was motivated to actually handle the errors. It is but it will be done in a follow up series. For now I want to have the pinned extents be per-transaction and not be more broken (e.g. no error handling) than we currently are. > >> + >> cache = btrfs_lookup_block_group(trans->fs_info, bytenr); >> if (!cache) >> return -EINVAL; >> @@ -2920,6 +2922,12 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) >> mutex_unlock(&fs_info->unused_bg_unpin_mutex); >> break; >> } >> + if (test_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags)) { >> + clear_extent_bits(&fs_info->freed_extents[0], start, >> + end, EXTENT_UPTODATE); >> + clear_extent_bits(&fs_info->freed_extents[1], start, >> + end, EXTENT_UPTODATE); >> + } >> >> if (btrfs_test_opt(fs_info, DISCARD_SYNC)) >> ret = btrfs_discard_extent(fs_info, start, >> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c >> index b535d409a728..f89de24838d5 100644 >> --- a/fs/btrfs/tree-log.c >> +++ b/fs/btrfs/tree-log.c >> @@ -2994,7 +2994,7 @@ static inline void btrfs_remove_log_ctx(struct btrfs_root *root, >> mutex_unlock(&root->log_mutex); >> } >> >> -/* >> +/* >> * Invoked in log mutex context, or be sure there is no other task which >> * can access the list. >> */ >> -- >> 2.17.1
On 30.01.20 г. 15:53 ч., David Sterba wrote: > On Mon, Jan 20, 2020 at 04:09:16PM +0200, Nikolay Borisov wrote: >> In preparation to making pinned extents per-transaction ensure that >> log such extents are always excluded from caching. To achieve this in >> addition to marking them via btrfs_pin_extent_for_log_replay they also >> need to be marked with btrfs_add_excluded_extent to prevent log tree >> extent buffer being loaded by the free space caching thread. That's >> required since log treeblocks are not recorded in the extent tree, hence >> they always look free. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> --- >> fs/btrfs/extent-tree.c | 8 ++++++++ >> fs/btrfs/tree-log.c | 2 +- >> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 7dcf9217a622..d680f2ac336b 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -2634,6 +2634,8 @@ int btrfs_pin_extent_for_log_replay(struct btrfs_trans_handle *trans, >> struct btrfs_block_group *cache; >> int ret; >> >> + btrfs_add_excluded_extent(trans->fs_info, bytenr, num_bytes); > > Again lack of error handling, I thought that untangling and cleaning up > the extent pinning was motivated to actually handle the errors. Currently btrfs_add_excluded_extent always returns 0. The possible err values from set_extent_bit are not propagated. I think this is a cleanup for another series. > <snip>
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 7dcf9217a622..d680f2ac336b 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -2634,6 +2634,8 @@ int btrfs_pin_extent_for_log_replay(struct btrfs_trans_handle *trans, struct btrfs_block_group *cache; int ret; + btrfs_add_excluded_extent(trans->fs_info, bytenr, num_bytes); + cache = btrfs_lookup_block_group(trans->fs_info, bytenr); if (!cache) return -EINVAL; @@ -2920,6 +2922,12 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) mutex_unlock(&fs_info->unused_bg_unpin_mutex); break; } + if (test_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags)) { + clear_extent_bits(&fs_info->freed_extents[0], start, + end, EXTENT_UPTODATE); + clear_extent_bits(&fs_info->freed_extents[1], start, + end, EXTENT_UPTODATE); + } if (btrfs_test_opt(fs_info, DISCARD_SYNC)) ret = btrfs_discard_extent(fs_info, start, diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index b535d409a728..f89de24838d5 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -2994,7 +2994,7 @@ static inline void btrfs_remove_log_ctx(struct btrfs_root *root, mutex_unlock(&root->log_mutex); } -/* +/* * Invoked in log mutex context, or be sure there is no other task which * can access the list. */
In preparation to making pinned extents per-transaction ensure that log such extents are always excluded from caching. To achieve this in addition to marking them via btrfs_pin_extent_for_log_replay they also need to be marked with btrfs_add_excluded_extent to prevent log tree extent buffer being loaded by the free space caching thread. That's required since log treeblocks are not recorded in the extent tree, hence they always look free. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent-tree.c | 8 ++++++++ fs/btrfs/tree-log.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)