| Message ID | 20180921064521.13102-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] btrfs: relocation: Cleanup while() loop using for() | expand |
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); >
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); >>
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);
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(-)