Re: [PATCH] btrfs: fix check_shared for fiemap ioctl

From: luke <lufq.fnst@cn.fujitsu.com>
To: <dsterba@suse.cz>
Cc: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH] btrfs: fix check_shared for fiemap ioctl
Date: Wed, 1 Jun 2016 13:43:37 +0800	[thread overview]
Message-ID: <6b57e774-27f5-d9a7-8bf0-d83b8da6f209@cn.fujitsu.com> (raw)
In-Reply-To: <20160531162325.GJ29147@suse.cz>


At 06/01/2016 12:23 AM, David Sterba wrote:
> On Tue, May 31, 2016 at 03:27:42PM +0800, luke wrote:
>>>> +static void ref_root_fini(struct ref_root *ref_tree)
>>>> +{
>>>> +	struct ref_node *node;
>>>> +	struct rb_node *next;
>>>> +
>>>> +	while ((next = rb_first(&ref_tree->rb_root)) != NULL) {
>>>> +		node = rb_entry(next, struct ref_node, rb_node);
>>>> +		rb_erase(next, &ref_tree->rb_root);
>>>> +		kfree(node);
>>> This could be slow as rb_erase has to do the rb-tree rotations. Can we
>>> do a post-order traversal and just free the nodes?
>> Excuse me, this seems to be standard way to release the rb-tree. For
>> example,  "btrfs_free_block_groups" this function is the same way. So
>> should i refactor this function?
> So for first version it's ok to keep it the simple way, but refactoring
> to the post-order traversal should help performance. And as there are
> more places it would be better to do it separately.
>
>
Yes, you're right. I have updated this patch to fix other problems. And, 
I'll refactor this function with another patch soon.

Thanks,
Lu