* [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for()
@ 2018-09-21 6:45 Qu Wenruo
2018-09-21 6:45 ` [PATCH 2/2] btrfs: relocation: Remove redundant tree level check Qu Wenruo
2018-09-21 6:53 ` [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Nikolay Borisov
0 siblings, 2 replies; 5+ messages in thread
From: Qu Wenruo @ 2018-09-21 6:45 UTC (permalink / raw)
To: linux-btrfs
And add one line comment explaining what we're doing for each loop.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/relocation.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 8783a1776540..d7f5a11dde20 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2997,27 +2997,25 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
goto out_free_blocks;
}
- rb_node = rb_first(blocks);
- while (rb_node) {
+ /* Kick in readahead for tree blocks with missing keys */
+ for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
block = rb_entry(rb_node, struct tree_block, rb_node);
if (!block->key_ready)
readahead_tree_block(fs_info, block->bytenr);
- rb_node = rb_next(rb_node);
}
- rb_node = rb_first(blocks);
- while (rb_node) {
+ /* Get first keys */
+ for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
block = rb_entry(rb_node, struct tree_block, rb_node);
if (!block->key_ready) {
err = get_tree_block_key(fs_info, block);
if (err)
goto out_free_path;
}
- rb_node = rb_next(rb_node);
}
- rb_node = rb_first(blocks);
- while (rb_node) {
+ /* Do tree relocation */
+ for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
block = rb_entry(rb_node, struct tree_block, rb_node);
node = build_backref_tree(rc, &block->key,
@@ -3034,7 +3032,6 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
err = ret;
goto out;
}
- rb_node = rb_next(rb_node);
}
out:
err = finish_pending_nodes(trans, rc, path, err);
--
2.19.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] btrfs: relocation: Remove redundant tree level check
2018-09-21 6:45 [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Qu Wenruo
@ 2018-09-21 6:45 ` Qu Wenruo
2018-09-21 6:55 ` Nikolay Borisov
2018-09-21 6:53 ` [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Nikolay Borisov
1 sibling, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2018-09-21 6:45 UTC (permalink / raw)
To: linux-btrfs
Commit 581c1760415c ("btrfs: Validate child tree block's level and first
key") has made tree block level check mandatory.
So if tree block level doesn't match, we won't get a valid extent
buffer.
The extra WARN_ON() check can be removed completely.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/relocation.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d7f5a11dde20..56e2fab2926d 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2911,7 +2911,6 @@ static int get_tree_block_key(struct btrfs_fs_info *fs_info,
free_extent_buffer(eb);
return -EIO;
}
- WARN_ON(btrfs_header_level(eb) != block->level);
if (block->level == 0)
btrfs_item_key_to_cpu(eb, &block->key, 0);
else
--
2.19.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] btrfs: relocation: Remove redundant tree level check
2018-09-21 6:45 ` [PATCH 2/2] btrfs: relocation: Remove redundant tree level check Qu Wenruo
@ 2018-09-21 6:55 ` Nikolay Borisov
0 siblings, 0 replies; 5+ messages in thread
From: Nikolay Borisov @ 2018-09-21 6:55 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 21.09.2018 09:45, Qu Wenruo wrote:
> Commit 581c1760415c ("btrfs: Validate child tree block's level and first
> key") has made tree block level check mandatory.
>
> So if tree block level doesn't match, we won't get a valid extent
> buffer.
> The extra WARN_ON() check can be removed completely.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/relocation.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index d7f5a11dde20..56e2fab2926d 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2911,7 +2911,6 @@ static int get_tree_block_key(struct btrfs_fs_info *fs_info,
> free_extent_buffer(eb);
> return -EIO;
> }
> - WARN_ON(btrfs_header_level(eb) != block->level);
> if (block->level == 0)
> btrfs_item_key_to_cpu(eb, &block->key, 0);
> else
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for()
2018-09-21 6:45 [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Qu Wenruo
2018-09-21 6:45 ` [PATCH 2/2] btrfs: relocation: Remove redundant tree level check Qu Wenruo
@ 2018-09-21 6:53 ` Nikolay Borisov
2018-09-21 6:56 ` Qu Wenruo
1 sibling, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2018-09-21 6:53 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 21.09.2018 09:45, Qu Wenruo wrote:
> And add one line comment explaining what we're doing for each loop.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/relocation.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 8783a1776540..d7f5a11dde20 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2997,27 +2997,25 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
> goto out_free_blocks;
> }
>
> - rb_node = rb_first(blocks);
> - while (rb_node) {
> + /* Kick in readahead for tree blocks with missing keys */
> + for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
FYI there is already a rbtree_postorder_for_each_entry_safe functino
which allows to iterate the rb tree in post order. It provides stronger
guarantees than you actually need (namely the _safe ) but in any case it
will allows you to get rid of the "block = " line as well as the
"rb_node =" line. This will remove all superfluous code that's needed to
handle the iteration.
> block = rb_entry(rb_node, struct tree_block, rb_node);
> if (!block->key_ready)
> readahead_tree_block(fs_info, block->bytenr);
> - rb_node = rb_next(rb_node);
> }
>
> - rb_node = rb_first(blocks);
> - while (rb_node) {
> + /* Get first keys */
> + for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
> block = rb_entry(rb_node, struct tree_block, rb_node);
> if (!block->key_ready) {
> err = get_tree_block_key(fs_info, block);
> if (err)
> goto out_free_path;
> }
> - rb_node = rb_next(rb_node);
> }
>
> - rb_node = rb_first(blocks);
> - while (rb_node) {
> + /* Do tree relocation */
> + for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
> block = rb_entry(rb_node, struct tree_block, rb_node);
>
> node = build_backref_tree(rc, &block->key,
> @@ -3034,7 +3032,6 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
> err = ret;
> goto out;
> }
> - rb_node = rb_next(rb_node);
> }
> out:
> err = finish_pending_nodes(trans, rc, path, err);
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for()
2018-09-21 6:53 ` [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Nikolay Borisov
@ 2018-09-21 6:56 ` Qu Wenruo
0 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2018-09-21 6:56 UTC (permalink / raw)
To: Nikolay Borisov, Qu Wenruo, linux-btrfs
On 2018/9/21 下午2:53, Nikolay Borisov wrote:
>
>
> On 21.09.2018 09:45, Qu Wenruo wrote:
>> And add one line comment explaining what we're doing for each loop.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/relocation.c | 15 ++++++---------
>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 8783a1776540..d7f5a11dde20 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -2997,27 +2997,25 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
>> goto out_free_blocks;
>> }
>>
>> - rb_node = rb_first(blocks);
>> - while (rb_node) {
>> + /* Kick in readahead for tree blocks with missing keys */
>> + for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
>
> FYI there is already a rbtree_postorder_for_each_entry_safe functino
> which allows to iterate the rb tree in post order. It provides stronger
> guarantees than you actually need (namely the _safe ) but in any case it
> will allows you to get rid of the "block = " line as well as the
> "rb_node =" line. This will remove all superfluous code that's needed to
> handle the iteration.
Great thanks!
This is much better solution, I'll use them in next version.
Thanks,
Qu
>
>> block = rb_entry(rb_node, struct tree_block, rb_node);
>> if (!block->key_ready)
>> readahead_tree_block(fs_info, block->bytenr);
>> - rb_node = rb_next(rb_node);
>> }
>>
>> - rb_node = rb_first(blocks);
>> - while (rb_node) {
>> + /* Get first keys */
>> + for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
>> block = rb_entry(rb_node, struct tree_block, rb_node);
>> if (!block->key_ready) {
>> err = get_tree_block_key(fs_info, block);
>> if (err)
>> goto out_free_path;
>> }
>> - rb_node = rb_next(rb_node);
>> }
>>
>> - rb_node = rb_first(blocks);
>> - while (rb_node) {
>> + /* Do tree relocation */
>> + for (rb_node = rb_first(blocks); rb_node; rb_node = rb_next(rb_node)) {
>> block = rb_entry(rb_node, struct tree_block, rb_node);
>>
>> node = build_backref_tree(rc, &block->key,
>> @@ -3034,7 +3032,6 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
>> err = ret;
>> goto out;
>> }
>> - rb_node = rb_next(rb_node);
>> }
>> out:
>> err = finish_pending_nodes(trans, rc, path, err);
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-21 12:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-21 6:45 [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Qu Wenruo
2018-09-21 6:45 ` [PATCH 2/2] btrfs: relocation: Remove redundant tree level check Qu Wenruo
2018-09-21 6:55 ` Nikolay Borisov
2018-09-21 6:53 ` [PATCH 1/2] btrfs: relocation: Cleanup while() loop using for() Nikolay Borisov
2018-09-21 6:56 ` Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).