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@xxxxxxxx>
>> ---
>> 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);
>>
