[09/11] btrfs: Mark pinned log extents as excluded
diff mbox series

Message ID 20200120140918.15647-10-nborisov@suse.com
State New
Headers show
Series
  • Make pinned extents tracking per-transaction
Related show

Commit Message

Nikolay Borisov Jan. 20, 2020, 2:09 p.m. UTC
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(-)

Comments

Josef Bacik Jan. 22, 2020, 8:12 p.m. UTC | #1
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
David Sterba Jan. 30, 2020, 1:53 p.m. UTC | #2
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
Nikolay Borisov Jan. 30, 2020, 2:03 p.m. UTC | #3
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
Nikolay Borisov Feb. 5, 2020, 8:51 a.m. UTC | #4
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>

Patch
diff mbox series

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.
  */