Re: Regression in: [PATCH 4/4] btrfs: qgroup: account shared subtree during snapshot delete

[Stefan Priebe <s.priebe@profihost.ag>]

Am 03.11.2015 um 20:26 schrieb Mark Fasheh:
> On Mon, Nov 02, 2015 at 09:34:24AM +0800, Qu Wenruo wrote:
>>
>>
>> Stefan Priebe wrote on 2015/11/01 21:49 +0100:
>>> Hi,
>>>
>>> this one: http://www.spinics.net/lists/linux-btrfs/msg47377.html
>>>
>>> adds a regression to my test systems with very large disks (30tb and 50tb).
>>>
>>> btrfs balance is super slow afterwards while heavily making use of cp
>>> --reflink=always on big files (200gb - 500gb).
>>>
>>> Sorry didn't know how to correctly reply to that "old" message.
>>>
>>> Greets,
>>> Stefan
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> Thanks for the testing.
>>
>> Are you using qgroup or just doing normal balance with qgroup disabled?
>>
>> For the latter case, that's should be optimized to skip the dirty
>> extent insert in qgroup disabled case.
>>
>> For qgroup enabled case, I'm afraid that's the design.
>> As relocation will drop a subtree to relocate, and to ensure qgroup
>> consistent, we must walk down all the tree blocks and mark them
>> dirty for later qgroup accounting.
>
> Qu, we're always going to have to walk the tree when deleting it, this is
> part of removing a subvolume. We've walked shared subtrees in this code for
> numerous kernel releases without incident before it was removed in 4.2.
>
> Do you have any actual evidence that this is a major performance regression?
>  From our previous conversations you seemed convinced of this, without even
> having a working subtree walk to test. I remember the hand wringing
> about an individual commit being too heavy with the qgroup code (even though
> I pointed out that tree walk is a restartable transaction).
>
> It seems that you are confused still about how we handle removing a volume
> wrt qgroups.
>
> If you have questions or concerns I would be happy to explain them but
> IMHO your statements there are opinion and not based in fact.
>
> Yes btw, we might have to do more work for the uncommon case of a
> qgroup being referenced by higher level groups but that is clearly not
> happening here (and honestly it's not a common case at all).
> 	--Mark

Sorry don't know much about the btrfs internals.

I just can reproduce this. Switching to a kernel with this patch and 
without. With it takes ages - without it's super fast. I prooved this 
several times by just rebooting to the other kernel.

Stefan